Fix wrong "Makefile*" pattern in find.

Fix wrong "Makefile*" pattern in find.

[Luigi Semenzato, 2010-06-25 00:19:01]

Thanks!

[vb, 2010-06-25 00:27:09]

LGTM

I was going to do it, but then noticed your email, I'll leave this fix
up to you then.

 Also, we might be better off using git to find all files which are
tracked under this tree - this would cover situations when there are
some leftover files  present in the directory.

cheers,
/v


On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
> Reviewers: vb,
>
> Message:
> Thanks!
>
> Description:
> Fix wrong "Makefile*" pattern in find.
>
> Please review this at http://codereview.chromium.org/2868035/show
>
> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>
> Affected files:
>  M src/tlcl/Makefile
>  M src/tlcl/version.h
>
>
> Index: src/tlcl/Makefile
> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
> index
>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
> 100644
> --- a/src/tlcl/Makefile
> +++ b/src/tlcl/Makefile
> @@ -28,7 +28,7 @@ update-structures: generator
>                  cp structures.tmp structures.h )
>
>  update-version:
> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name version.h
> \
> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name version.h
> \
>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>        cmp -s version.tmp version.h || \
> Index: src/tlcl/version.h
> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
> index
>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
> 100644
> --- a/src/tlcl/version.h
> +++ b/src/tlcl/version.h
> @@ -1 +1 @@
> -char* TlclVersion = "TLCLv=5538f324";
> +char* TlclVersion = "TLCLv=a8e5e857";
>
>
>

[Luigi Semenzato, 2010-06-25 00:55:38]

Interesting idea.

As Gaurav was saying, the build should be agnostic of the VCS.  But
we've already broken this separation by the mere existence of
version.c, whose only point is to automate version identification of a
binary.

Actually---I just remembered that it doesn't matter if there are extra
files.  The hash could be a random number and this would still work,
because version.c is committed to the repository.  The advantage of a
hash versus a random number is that the hash doesn't change if nothing
else does, so we're not annoyed by a continuously changing version.c

On Thu, Jun 24, 2010 at 5:26 PM, Vadim Bendebury <vbendeb@chromium.org> wrote:
> LGTM
>
> I was going to do it, but then noticed your email, I'll leave this fix
> up to you then.
>
>  Also, we might be better off using git to find all files which are
> tracked under this tree - this would cover situations when there are
> some leftover files  present in the directory.
>
> cheers,
> /v
>
>
> On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
>> Reviewers: vb,
>>
>> Message:
>> Thanks!
>>
>> Description:
>> Fix wrong "Makefile*" pattern in find.
>>
>> Please review this at http://codereview.chromium.org/2868035/show
>>
>> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>>
>> Affected files:
>>  M src/tlcl/Makefile
>>  M src/tlcl/version.h
>>
>>
>> Index: src/tlcl/Makefile
>> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
>> index
>>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
>> 100644
>> --- a/src/tlcl/Makefile
>> +++ b/src/tlcl/Makefile
>> @@ -28,7 +28,7 @@ update-structures: generator
>>                  cp structures.tmp structures.h )
>>
>>  update-version:
>> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name version.h
>> \
>> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name version.h
>> \
>>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>>        cmp -s version.tmp version.h || \
>> Index: src/tlcl/version.h
>> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
>> index
>>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
>> 100644
>> --- a/src/tlcl/version.h
>> +++ b/src/tlcl/version.h
>> @@ -1 +1 @@
>> -char* TlclVersion = "TLCLv=5538f324";
>> +char* TlclVersion = "TLCLv=a8e5e857";
>>
>>
>>
>

[Luigi Semenzato, 2010-06-25 01:01:18]

To clarify: it doesn't matter because the firmware build doesn't
change version.c.

On Thu, Jun 24, 2010 at 5:55 PM, Luigi Semenzato <semenzato@chromium.org> wrote:
> Interesting idea.
>
> As Gaurav was saying, the build should be agnostic of the VCS.  But
> we've already broken this separation by the mere existence of
> version.c, whose only point is to automate version identification of a
> binary.
>
> Actually---I just remembered that it doesn't matter if there are extra
> files.  The hash could be a random number and this would still work,
> because version.c is committed to the repository.  The advantage of a
> hash versus a random number is that the hash doesn't change if nothing
> else does, so we're not annoyed by a continuously changing version.c
>
> On Thu, Jun 24, 2010 at 5:26 PM, Vadim Bendebury <vbendeb@chromium.org> wrote:
>> LGTM
>>
>> I was going to do it, but then noticed your email, I'll leave this fix
>> up to you then.
>>
>>  Also, we might be better off using git to find all files which are
>> tracked under this tree - this would cover situations when there are
>> some leftover files  present in the directory.
>>
>> cheers,
>> /v
>>
>>
>> On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
>>> Reviewers: vb,
>>>
>>> Message:
>>> Thanks!
>>>
>>> Description:
>>> Fix wrong "Makefile*" pattern in find.
>>>
>>> Please review this at http://codereview.chromium.org/2868035/show
>>>
>>> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>>>
>>> Affected files:
>>>  M src/tlcl/Makefile
>>>  M src/tlcl/version.h
>>>
>>>
>>> Index: src/tlcl/Makefile
>>> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
>>> index
>>>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
>>> 100644
>>> --- a/src/tlcl/Makefile
>>> +++ b/src/tlcl/Makefile
>>> @@ -28,7 +28,7 @@ update-structures: generator
>>>                  cp structures.tmp structures.h )
>>>
>>>  update-version:
>>> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name version.h
>>> \
>>> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name version.h
>>> \
>>>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>>>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>>>        cmp -s version.tmp version.h || \
>>> Index: src/tlcl/version.h
>>> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
>>> index
>>>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
>>> 100644
>>> --- a/src/tlcl/version.h
>>> +++ b/src/tlcl/version.h
>>> @@ -1 +1 @@
>>> -char* TlclVersion = "TLCLv=5538f324";
>>> +char* TlclVersion = "TLCLv=a8e5e857";
>>>
>>>
>>>
>>
>

[vb, 2010-06-25 01:02:11]

On Thu, Jun 24, 2010 at 5:55 PM, Luigi Semenzato <semenzato@chromium.org> wrote:
> Interesting idea.
>
> As Gaurav was saying, the build should be agnostic of the VCS.  But
> we've already broken this separation by the mere existence of
> version.c, whose only point is to automate version identification of a
> binary.
>
> Actually---I just remembered that it doesn't matter if there are extra
> files.  The hash could be a random number and this would still work,
> because version.c is committed to the repository.  The advantage of a
> hash versus a random number is that the hash doesn't change if nothing
> else does, so we're not annoyed by a continuously changing version.c
>

Right, but I am more concerned with a situation when we add a file to
the repository (say config.mk), which does not match the search
pattern, then that file would be excluded from the hash calculation.

BTW, I am not sure that  build should be agnostic of VCS. If I
remember correctly, make even has some implicit rules to try to derive
files from RCS, CVS, etc. if it can't find an appropriate source file.

cheers,
/v

> On Thu, Jun 24, 2010 at 5:26 PM, Vadim Bendebury <vbendeb@chromium.org> wrote:
>> LGTM
>>
>> I was going to do it, but then noticed your email, I'll leave this fix
>> up to you then.
>>
>>  Also, we might be better off using git to find all files which are
>> tracked under this tree - this would cover situations when there are
>> some leftover files  present in the directory.
>>
>> cheers,
>> /v
>>
>>
>> On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
>>> Reviewers: vb,
>>>
>>> Message:
>>> Thanks!
>>>
>>> Description:
>>> Fix wrong "Makefile*" pattern in find.
>>>
>>> Please review this at http://codereview.chromium.org/2868035/show
>>>
>>> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>>>
>>> Affected files:
>>>  M src/tlcl/Makefile
>>>  M src/tlcl/version.h
>>>
>>>
>>> Index: src/tlcl/Makefile
>>> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
>>> index
>>>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
>>> 100644
>>> --- a/src/tlcl/Makefile
>>> +++ b/src/tlcl/Makefile
>>> @@ -28,7 +28,7 @@ update-structures: generator
>>>                  cp structures.tmp structures.h )
>>>
>>>  update-version:
>>> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name version.h
>>> \
>>> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name version.h
>>> \
>>>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>>>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>>>        cmp -s version.tmp version.h || \
>>> Index: src/tlcl/version.h
>>> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
>>> index
>>>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
>>> 100644
>>> --- a/src/tlcl/version.h
>>> +++ b/src/tlcl/version.h
>>> @@ -1 +1 @@
>>> -char* TlclVersion = "TLCLv=5538f324";
>>> +char* TlclVersion = "TLCLv=a8e5e857";
>>>
>>>
>>>
>>
>

[gauravsh, 2010-06-25 01:02:12]

On Thu, Jun 24, 2010 at 5:55 PM, Luigi Semenzato <semenzato@chromium.org> wrote:
> Interesting idea.
>
> As Gaurav was saying, the build should be agnostic of the VCS.  But
> we've already broken this separation by the mere existence of
> version.c, whose only point is to automate version identification of a
> binary.
>
> Actually---I just remembered that it doesn't matter if there are extra
> files.  The hash could be a random number and this would still work,
> because version.c is committed to the repository.  The advantage of a
> hash versus a random number is that the hash doesn't change if nothing
> else does, so we're not annoyed by a continuously changing version.c

I am looking into a git-specific way of doing this. Seems like
gitattributes
(http://www.kernel.org/pub/software/scm/git/docs/gitattributes.html)
with the "ident" attribute might be able to help us here. I am trying
to figure out if there's a way to make it embed some sort of embedded
view of the whole tree instead of just the blob id of particular file
on which the attribute set.

>
> On Thu, Jun 24, 2010 at 5:26 PM, Vadim Bendebury <vbendeb@chromium.org> wrote:
>> LGTM
>>
>> I was going to do it, but then noticed your email, I'll leave this fix
>> up to you then.
>>
>>  Also, we might be better off using git to find all files which are
>> tracked under this tree - this would cover situations when there are
>> some leftover files  present in the directory.
>>
>> cheers,
>> /v
>>
>>
>> On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
>>> Reviewers: vb,
>>>
>>> Message:
>>> Thanks!
>>>
>>> Description:
>>> Fix wrong "Makefile*" pattern in find.
>>>
>>> Please review this at http://codereview.chromium.org/2868035/show
>>>
>>> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>>>
>>> Affected files:
>>>  M src/tlcl/Makefile
>>>  M src/tlcl/version.h
>>>
>>>
>>> Index: src/tlcl/Makefile
>>> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
>>> index
>>>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
>>> 100644
>>> --- a/src/tlcl/Makefile
>>> +++ b/src/tlcl/Makefile
>>> @@ -28,7 +28,7 @@ update-structures: generator
>>>                  cp structures.tmp structures.h )
>>>
>>>  update-version:
>>> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name version.h
>>> \
>>> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name version.h
>>> \
>>>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>>>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>>>        cmp -s version.tmp version.h || \
>>> Index: src/tlcl/version.h
>>> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
>>> index
>>>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
>>> 100644
>>> --- a/src/tlcl/version.h
>>> +++ b/src/tlcl/version.h
>>> @@ -1 +1 @@
>>> -char* TlclVersion = "TLCLv=5538f324";
>>> +char* TlclVersion = "TLCLv=a8e5e857";
>>>
>>>
>>>
>>
>



-- 
-g

[gauravsh, 2010-06-25 01:08:27]

On Thu, Jun 24, 2010 at 6:02 PM, Gaurav Shah <gauravsh@chromium.org> wrote:
> On Thu, Jun 24, 2010 at 5:55 PM, Luigi Semenzato <semenzato@chromium.org>
wrote:
>> Interesting idea.
>>
>> As Gaurav was saying, the build should be agnostic of the VCS.  But
>> we've already broken this separation by the mere existence of
>> version.c, whose only point is to automate version identification of a
>> binary.
>>
>> Actually---I just remembered that it doesn't matter if there are extra
>> files.  The hash could be a random number and this would still work,
>> because version.c is committed to the repository.  The advantage of a
>> hash versus a random number is that the hash doesn't change if nothing
>> else does, so we're not annoyed by a continuously changing version.c
>
> I am looking into a git-specific way of doing this. Seems like
> gitattributes
(http://www.kernel.org/pub/software/scm/git/docs/gitattributes.html)
> with the "ident" attribute might be able to help us here. I am trying
> to figure out if there's a way to make it embed some sort of embedded
> view of the whole tree instead of just the blob id of particular file
> on which the attribute set.

To add to this, this page describes the git way of doing what we want:

http://progit.org/book/ch7-2.html (scroll down to the section on
"Keyword Expansion").

We can use a "clean" filter to make git to do this for us - there seem
to be recipes to embed versioning/date information for us. If both of
you think this is something worth trying, I can go ahead and create a
CL for doing this. Let me know.

>
>>
>> On Thu, Jun 24, 2010 at 5:26 PM, Vadim Bendebury <vbendeb@chromium.org>
wrote:
>>> LGTM
>>>
>>> I was going to do it, but then noticed your email, I'll leave this fix
>>> up to you then.
>>>
>>>  Also, we might be better off using git to find all files which are
>>> tracked under this tree - this would cover situations when there are
>>> some leftover files  present in the directory.
>>>
>>> cheers,
>>> /v
>>>
>>>
>>> On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
>>>> Reviewers: vb,
>>>>
>>>> Message:
>>>> Thanks!
>>>>
>>>> Description:
>>>> Fix wrong "Makefile*" pattern in find.
>>>>
>>>> Please review this at http://codereview.chromium.org/2868035/show
>>>>
>>>> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>>>>
>>>> Affected files:
>>>>  M src/tlcl/Makefile
>>>>  M src/tlcl/version.h
>>>>
>>>>
>>>> Index: src/tlcl/Makefile
>>>> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
>>>> index
>>>>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
>>>> 100644
>>>> --- a/src/tlcl/Makefile
>>>> +++ b/src/tlcl/Makefile
>>>> @@ -28,7 +28,7 @@ update-structures: generator
>>>>                  cp structures.tmp structures.h )
>>>>
>>>>  update-version:
>>>> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name
version.h
>>>> \
>>>> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name version.h
>>>> \
>>>>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>>>>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>>>>        cmp -s version.tmp version.h || \
>>>> Index: src/tlcl/version.h
>>>> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
>>>> index
>>>>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
>>>> 100644
>>>> --- a/src/tlcl/version.h
>>>> +++ b/src/tlcl/version.h
>>>> @@ -1 +1 @@
>>>> -char* TlclVersion = "TLCLv=5538f324";
>>>> +char* TlclVersion = "TLCLv=a8e5e857";
>>>>
>>>>
>>>>
>>>
>>
>
>
>
> --
> -g
>



-- 
-g

[Luigi Semenzato, 2010-06-25 01:35:57]

Yes.

I gave up on finding the git equivalent of keyword substitution when I
found Torvalds's negative responses regarding the concept.  Thanks for
looking harder.


On Thu, Jun 24, 2010 at 6:08 PM, Gaurav Shah <gauravsh@chromium.org> wrote:
> On Thu, Jun 24, 2010 at 6:02 PM, Gaurav Shah <gauravsh@chromium.org> wrote:
>> On Thu, Jun 24, 2010 at 5:55 PM, Luigi Semenzato <semenzato@chromium.org>
wrote:
>>> Interesting idea.
>>>
>>> As Gaurav was saying, the build should be agnostic of the VCS.  But
>>> we've already broken this separation by the mere existence of
>>> version.c, whose only point is to automate version identification of a
>>> binary.
>>>
>>> Actually---I just remembered that it doesn't matter if there are extra
>>> files.  The hash could be a random number and this would still work,
>>> because version.c is committed to the repository.  The advantage of a
>>> hash versus a random number is that the hash doesn't change if nothing
>>> else does, so we're not annoyed by a continuously changing version.c
>>
>> I am looking into a git-specific way of doing this. Seems like
>> gitattributes
(http://www.kernel.org/pub/software/scm/git/docs/gitattributes.html)
>> with the "ident" attribute might be able to help us here. I am trying
>> to figure out if there's a way to make it embed some sort of embedded
>> view of the whole tree instead of just the blob id of particular file
>> on which the attribute set.
>
> To add to this, this page describes the git way of doing what we want:
>
> http://progit.org/book/ch7-2.html (scroll down to the section on
> "Keyword Expansion").
>
> We can use a "clean" filter to make git to do this for us - there seem
> to be recipes to embed versioning/date information for us. If both of
> you think this is something worth trying, I can go ahead and create a
> CL for doing this. Let me know.
>
>>
>>>
>>> On Thu, Jun 24, 2010 at 5:26 PM, Vadim Bendebury <vbendeb@chromium.org>
wrote:
>>>> LGTM
>>>>
>>>> I was going to do it, but then noticed your email, I'll leave this fix
>>>> up to you then.
>>>>
>>>>  Also, we might be better off using git to find all files which are
>>>> tracked under this tree - this would cover situations when there are
>>>> some leftover files  present in the directory.
>>>>
>>>> cheers,
>>>> /v
>>>>
>>>>
>>>> On Thu, Jun 24, 2010 at 5:19 PM,  <semenzato@chromium.org> wrote:
>>>>> Reviewers: vb,
>>>>>
>>>>> Message:
>>>>> Thanks!
>>>>>
>>>>> Description:
>>>>> Fix wrong "Makefile*" pattern in find.
>>>>>
>>>>> Please review this at http://codereview.chromium.org/2868035/show
>>>>>
>>>>> SVN Base: ssh://git@chromiumos-git/tpm_lite.git
>>>>>
>>>>> Affected files:
>>>>>  M src/tlcl/Makefile
>>>>>  M src/tlcl/version.h
>>>>>
>>>>>
>>>>> Index: src/tlcl/Makefile
>>>>> diff --git a/src/tlcl/Makefile b/src/tlcl/Makefile
>>>>> index
>>>>>
791ff5da1fec74dad1f8ab34f20d53d221033929..e384aa90d4ddd19646d8049817249d9ee51f082b
>>>>> 100644
>>>>> --- a/src/tlcl/Makefile
>>>>> +++ b/src/tlcl/Makefile
>>>>> @@ -28,7 +28,7 @@ update-structures: generator
>>>>>                  cp structures.tmp structures.h )
>>>>>
>>>>>  update-version:
>>>>> -       find \( -name '*.[ch]' -o -name 'Makefile*' \) -a \! -name
version.h
>>>>> \
>>>>> +       find \( -name '*.[ch]' -o -name 'Makefile' \) -a \! -name
version.h
>>>>> \
>>>>>                | sort | xargs cat | md5sum | cut -c 25-32 > x.tmp
>>>>>        echo "char* TlclVersion = \"TLCLv=$$(cat x.tmp)\";" > version.tmp
>>>>>        cmp -s version.tmp version.h || \
>>>>> Index: src/tlcl/version.h
>>>>> diff --git a/src/tlcl/version.h b/src/tlcl/version.h
>>>>> index
>>>>>
d6c67ea63bfd620bd462924f2bed837ed6bf4273..4050a8316729c6c1a7cd0e30a617363e6db4c11c
>>>>> 100644
>>>>> --- a/src/tlcl/version.h
>>>>> +++ b/src/tlcl/version.h
>>>>> @@ -1 +1 @@
>>>>> -char* TlclVersion = "TLCLv=5538f324";
>>>>> +char* TlclVersion = "TLCLv=a8e5e857";
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>>
>> --
>> -g
>>
>
>
>
> --
> -g
>