Re: [PATCH 06/18] version-gen: remove redundant check

From: Todd Zullinger <tmz@pobox.com>
Date: Apr 14, 2023, 19:41 UTC
Message-ID: <ZDmsg0njhtv_qM9a@pobox.com>
In-Reply-To: <6439aa5b2c776_6058729489@chronos.notmuch>

Felipe Contreras wrote:

Show 20 quoted lines

> Todd Zullinger wrote:
>> If I have a git repo, say ~/fedora/git which contains the
>> fedora packaging (spec file, etc.) and extract a git archive
>> in this directory, the describe will now pick up the data
>> from the parent git directory, won't it?
>> 
>>     $ git -C ~/fedora clone https://src.fedoraproject.org/rpms/git.git
>>     $ cd ~/fedora/git
>>     $ git -C ~/upstream/git archive --format=tar --prefix=git/ HEAD | tar xf -
>>     $ cd git
>>     $ make GIT-VERSION-GEN
>>     $ cat GIT-VERSION-FILE 
>>     GIT_VERSION = 
> 
> I don't think this is a realistic use-case, but supposing it is, what would be
> the desired outcome in this case?
> 
>  GIT_VERSION = 2.40.GIT
> 
> ?

That's what we get before this change. I don't have an opinion on whether that could/should change, but if it does, it should be deliberate I think.

Show 7 quoted lines

>> The version file in the tarballs prevents this from
>> happening in the most common case, but it still feels like
>> this is loosening things a little more than it should.
> 
> If we care about this, the same behavior can be achieved with GIT_CEILING_DIRECTORIES:
> 
>  GIT_CEILING_DIRECTORIES=$(cd .. && pwd) git describe ...

I only mention it because it seemed like a change in behavior when the series aimed to not change any behavior. I'm sure there are plenty of things which could be changed while making imrpveoments here. I think we'll be better off if those changes are all noted and deliberate.

Thanks for working on this stuff, it is good to see improvements to this sort of plumbing.

-- 
Todd