Re: maint.mk: absolute VPATH build fix

[Jim Meyering]

Akim Demaille wrote:
> Le 28 juil. 2012 à 23:26, Bruno Haible a écrit :
>> Hi Akim,
>>
>> Akim Demaille wrote:
>>> +  $$(git --git-dir $(srcdir)/.git cat-file tag v$(VERSION) \
>>
>> This code makes the assumption that the top-level dir of a project
>> (that is, the one with the top-level Makefile.am and configure.ac)
>> is at the root of the git project. Which is not necessarily the case.
>> For an example, see the
>>  http://git.savannah.gnu.org/gitweb/?p=bibledit.git;a=tree
>> project: it has several subdirectories with configure.ac and Makefile.am
>> each, but none at the top level.
>>
>> Can you make it work without this assumption? I.e. from $(srcdir), walk
>> up to the first ancestor directory that contains a .git subdir?

Good idea, Bruno.

> This is easy to do in that case:
>
> commit 767b67254209634a7a13eb99a51df5dc1e9388f0
> Author: Akim Demaille <address@hidden>
> Date:   Sat Jul 28 12:40:53 2012 +0200
>
>     maint.mk: absolute VPATH build fix
>
>     * top/maint.mk (gpg_key_ID): Help git find .git when, for instance,
>     $(srcdir) is not a parent of $(builddir).
>
> diff --git a/ChangeLog b/ChangeLog
> index 378c405..51f4a76 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2012-07-29  Akim Demaille  <address@hidden>
> +
> +     maint.mk: absolute VPATH build fix
> +     * top/maint.mk (gpg_key_ID): Help git find .git when, for instance,
> +     $(srcdir) is not a parent of $(builddir).
> +
>  2012-07-27  Jim Meyering  <address@hidden>
>
>       maint.mk: new rule: refresh-gnulib-patches
> diff --git a/top/maint.mk b/top/maint.mk
> index baff0e9..2af938e 100644
> --- a/top/maint.mk
> +++ b/top/maint.mk
> @@ -1256,10 +1256,11 @@ bootstrap-tools ?= autoconf,automake,gnulib
>
>  # If it's not already specified, derive the GPG key ID from
>  # the signed tag we've just applied to mark this release.
> -gpg_key_ID ?= \
> -  $$(git cat-file tag v$(VERSION) \
> -     | gpgv --status-fd 1 --keyring /dev/null - - 2>/dev/null \
> -     | awk '/^\[GNUPG:\] ERRSIG / {print $$3; exit}')
> +gpg_key_ID ?=                                                                
> \
> +  $$(cd $(srcdir)                                                    \
> +     && git cat-file tag v$(VERSION)                                 \
> +        | gpgv --status-fd 1 --keyring /dev/null - - 2>/dev/null     \
> +        | awk '/^\[GNUPG:\] ERRSIG / {print $$3; exit}')

ACK.

> But there's another occurrence of that issue.  I suggest this patch.
> (I'd like a critical eye from a native on the --help snippet :).

Good catch.

...
> @@ -68,7 +68,8 @@ OPTIONS:
>                    header; the default is to cluster adjacent commit messages
>                    if their headers are the same and neither commit message
>                    contains multiple paragraphs.
> -   --srcdir=DIR the root of the source tree, containing the '.git' directory.
> +   --srcdir=DIR the root of the source tree, a parent of whom contains
> +                the '.git' directory.

How about this?

      --srcdir=DIR the root of the source tree, from which the .git/
                     directory can be derived

> +# git_dir_option $SRCDIR
> +#
> +# From $SRCDIR, the --git-dir option to pass to git (none if $SRCDIR
> +# is undef).  Return as a list (0 or 1 element).
> +sub git_dir_option($)
> +{
> +  my ($srcdir) = @_;
> +  my @res = ();
> +  if (defined $srcdir)
> +    {
> +      my $qdir = shell_quote $srcdir;
> +      my $cmd = "cd $qdir && git rev-parse --show-toplevel";
> +      my $qcmd = shell_quote $cmd;
> +      my $git_dir = qx($cmd);
> +      defined $git_dir
> +        or die "$ME: cannot run $qcmd: $!\n";
> +      die "$ME: $qcmd had unexpected exit code or signal ($?)\n"
> +        if $?;

Very nice.

I prefer to write it this way, putting the less-likely code in the
indented clause:  (or testing $? != 0)

        $?
          and die "$ME: $qcmd had unexpected exit code or signal ($?)\n"

> +      chomp $git_dir;
> +      push @res, "--git-dir=$git_dir/.git";
> +    }
> +  @res;
> +}
> +
>  {
>    my $since_date;
>    my $format_string = '%s%n%b%n';
> @@ -224,7 +249,7 @@ sub parse_amend_file($)
>    my $amend_code = defined $amend_file ? parse_amend_file $amend_file : {};
>
>    my @cmd = ('git',
> -             defined $srcdir ? ("--git-dir=$srcdir/.git") : (),
> +             git_dir_option $srcdir,
>               qw(log --log-size),
>               '--pretty=format:%H:%ct  %an  <%ae>%n%n'.$format_string, @ARGV);
>    open PIPE, '-|', @cmd