[flashrom] [PATCH] Add git support to the Makefile

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Aug 30 15:48:55 CEST 2010


On 28.08.2010 00:00, Vadim Bendebury wrote:
> guys, here is the change for your review:
>
> http://codereview.chromium.org/3265003/show
>   

I have added my review there. The suggested code breaks for me.


> It does not deal with the recent addition which retrieves SVN revision
> number, that part can be easily added, it mostly makes the git version
> display much more useful.
>
> Ideally we get something like this into the tree.
>   

What exactly was wrong with the patch I posted?


> On Fri, Aug 27, 2010 at 2:08 PM, Vadim Bendebury (вб)
> <vbendeb at google.com> wrote:
>   
>> On Fri, Aug 27, 2010 at 1:52 PM, David Hendricks <dhendrix at google.com> wrote:
>>     
>>> [+vadim]
>>>
>>> On Fri, Aug 27, 2010 at 1:49 PM, David Hendricks <dhendrix at google.com>
>>> wrote:
>>>       
>>>> Looks pretty good so far!
>>>> For the benefit of everyone who missed the excitement on IRC, there is one
>>>> major caveat to this patch: Those using a git repository probably have stale
>>>> SVN metadata, which means the upstream Flashrom SVN information in the
>>>>
>>>>         
>> guys, I am joining the party late so to speak, I have a couple of questions:
>>
>> - why do you need to integrate shell scripts into the Makefile. It is
>> perfectly valid to put the code in a file in ./util/
>>
>> - when local git tree (or svn tree to that matter) has modified files
>> - this should be reflected in the version string, to indicate that the
>> code was built off modified source tree.
>>     

Debatable.


>> - the git hash on its own is not good enough: you can't tell looking
>> at two hashes which one is more recent, this is why in case svn
>> information is not available the date should be included.  In case the
>> source is built off pristine sources, the date should be the git log
>> date, in case the tree was modified, the date should be the build
>> date.
>>     

If svn information is not available at all, the report is totally
worthless anyway.


>> - *anything* isbetter than producing multiple different images with
>> the same version string
>>     

Of course images with the same version string will differ. Or do you
really suggest to ship debug builds only, and print the version number
of every used header file?

>> I have a patch which achieves most of that, I'll share it with you in
>> a bit later,
>>     

I think we need a few more iterations to get this through review.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list