Re: git-version-gen not working with Savannah/cgit snapshots

[G. Branden Robinson]

Hi Bruno,

Thank you for the quick response!

At 2022-06-04T00:38:23+0200, Bruno Haible wrote:
> Hi Branden,
> 
> > -elif test "x$fallback" = x || git --version >/dev/null 2>&1; then
> > +elif test "x$fallback" = x && ! git --version >/dev/null 2>&1; then
> >      v=UNKNOWN
> >  else
> >      v=$fallback
> 
> I don't think that's a bug. The --fallback option is documented like this:
> 
>    --fallback VERSION
>                       fallback version to use if \"git --version\" fails
> 
> And that's what the code does: If the --fallback option was given AND
> 'git --version' fails, then the line
>     v=$fallback
> is executed. Otherwise the line
>     v=UNKNOWN
> is executed.

I guess I don't understand why "git --version" is being run in the first
place.  It seems to simply be checking whether the git command is
installed (aren't there are more idiomatic ways to do that, like
"command -v", if I remember my POSIX correctly?).  Should the exit
status of "git describe" be tested instead?

$ patch -R -d gnulib-$hash -p0 < ../git-version-gen.patch
patching file build-aux/git-version-gen
$ ./build-aux/git-version-gen --prefix "" ./.tarball-version # no output
$ $ ./build-aux/git-version-gen --prefix "" --fallback 1.23.0.rc1 
./.tarball-version
1.23.0.rc1$
$ git --version
git version 2.30.2
$ echo $?
0
$ git describe
fatal: not a git repository (or any of the parent directories): .git
$ echo $?
128

It is not clear to me that knowing whether a working git executable is
available tells git-version-gen anything useful at the point that test
is performed.  But I am sure my understanding is incomplete.

> Your proposed patch would have the effect that v gets set to
> empty in some case.

Yes, but as seen above, if the --fallback option is omitted, that can
happen anyway.

> > Here is the output of "git submodule" for this groff release.
> > 
> >  c8b8f3bbcde37a53cd226f4c9cebd0dde6aca37f gnulib (v0.1-5208-gc8b8f3bbcd)
> > 
> > We therefore do the following.
> > 
> >   $ hash=c8b8f3bbcde37a53cd226f4c9cebd0dde6aca37f
> >   $ wget -O gnulib.tar.gz https://git.savannah.gnu.org/cgit/gnulib.git/\
> > snapshot/gnulib-$hash.tar.gz
> >   $ tar xf gnulib.tar.gz
> > 
> > ...
> >
> >   $ ./bootstrap --gnulib-srcdir=gnulib-$hash
> 
> What you are doing, seems contorted to me:

It absolutely is!  I am not happy with it.

>   - You have submodule information in the parent git repository (groff).

Yes.  But, importantly, _not in the snapshot archives that Savannah cgit
generates_.  Per 'grep -r', the only places the hash for the gnulib
submodule we're using appears in those archives are in our ChangeLog and
the INSTALL.REPO file quoted above.

>   - Then you download gnulib, explicitly throwing away its git versioning.

In this build scenario, that information has been thrown away already.

After resolving this, I had plans to go complain to the Savannah
administrators that the snapshot archives they generate on demand
neither (1) recurse through submodules nor (2) provide a
".tarball-submodules" file with the requisite information such that
users can construct URLs and retrieve those bits themselves.

But, first, I need to figure out how to get a snapshot archive build to
work at all, and that's why I'm here.

>   - Then you run bootstrap without '--no-git' option, thereby telling it
>     to use to the indicated git commits.

Yes, I run without that option because apparently --gnulib-srcdir
implies it.  If I run bootstrap with _neither_ option, I have no
success.

$ ./bootstrap
./bootstrap: Bootstrapping from checked-out groff sources...
./bootstrap: getting gnulib files...
fatal: not a git repository (or any of the parent directories): .git
$ ./bootstrap --no-git
./bootstrap: Error: --no-git requires --gnulib-srcdir
$ ./bootstrap --no-git^C
$ ./bootstrap --gnulib-srcdir=gnulib-$hash
./bootstrap: Bootstrapping from checked-out groff sources...
./bootstrap: gnulib-c8b8f3bbcde37a53cd226f4c9cebd0dde6aca37f/gnulib-tool    
--no-changelog   --aux-dir=build-aux   --doc-base=doc   --lib=libgnu   
--m4-base=gnulib_m4/   --source-base=lib/   --tests-base=tests   --local-dir=gl
[etc.; this "works" to bootstrap but infers a groff version
number of "UNKNOWN", which causes failures later]

> IMO you should either
>   - let 'bootstrap' use the indicated git commits; then of course it needs
>     the .git directory of the gnulib checkout (and that implies 'git clone'
>     or 'git submodule' instead of 'wget'), or

The snapshot archives that Savannah/cgit provides lack these
prerequisites.

>   - let 'bootstrap' abstain from any git operation; that implies passing
>     not only --gnulib-srcdir but also --no-git.

Adding '--no-git' to '--gnulib-srcdir' doesn't seem to change anything
for me (with my patch reverted).  I end up with "UNKOWN" where
"1.23.0.rc1" should be, even though configure.ac says this.

AC_INIT([GNU roff],
        m4_esyscmd([build-aux/git-version-gen --fallback 1.23.0.rc1 \
          --prefix "" .tarball-version]),
        http://savannah.gnu.org/bugs/?group=groff,
        [groff])

> By the way, if submodules are not what you want, i.e. if you always
> want to use the newest gnulib, I suggest to use the
> gnulib/top/gitsub.sh script.

I have no principled objection to submodules; the decision to use them
(well, just this one, for gnulib) was taken before I joined groff
development.

Regards,
Branden

signature.asc
Description: PGP signature