Re: [PATCH 06/18] version-gen: remove redundant check
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Apr 14, 2023, 19:32 UTC
- Message-ID
- <6439aa5b2c776_6058729489@chronos.notmuch>
- In-Reply-To
- <ZDmjIqNwVMESgHnn@pobox.com>
Todd Zullinger wrote:
Show 43 quoted lines
> Felipe Contreras wrote: > > Todd Zullinger wrote: > >> Felipe Contreras wrote: > >>> If we are not in a git repository `git describe` will fail anyway. > >> > >> The parent directory may be a git repository though. The > >> current code ensures that we're running `git describe` in > >> the proper repository. > > > > How exactly does it do that? > > > > The current code expects the cwd to be the git repo, run it in any other > > directory and it will generate GIT-VERSION-FILE in that directory, which is > > clearly not intended. > > Whether it's fool proof isn't really my point. It did > attempt to check that .git was a file or directory. Not > checking at all isn't necessarily an improvement, was my > concern. > > >> If we drop this, aren't we breaking things for someone > >> building a git tarball which is in a subdirectory of a git > >> repository? > > > > How exactly would this hypothetical person build such a tarball? > > > > git init /tmp/foo > > mkdir -p /tmp/foo/bar > > cd /tmp/foo/bar > > make -C ~/dev/git dist > > 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
?
> 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 ...
-- Felipe Contreras