build-aux/git-hooks/pre-push

build-aux/git-hooks/pre-push

[Po Lu]

This script apparently loses when pushing from a separate worktree,
where .git is a file containing the name of the repository the worktree
was created from.

The following adjustment seems to fix it.  However, I don't know much
about git.  Objections?

diff --git a/build-aux/git-hooks/pre-push b/build-aux/git-hooks/pre-push
index 8e8277cba4f..8a2866f9d4c 100755
--- a/build-aux/git-hooks/pre-push
+++ b/build-aux/git-hooks/pre-push
@@ -83,4 +83,4 @@ $awk -v origin_name="$1" '
     # Print every SHA after oldref, up to (and including) newref.
     system("git rev-list --first-parent --reverse " oldref ".." newref)
   }
-' | $awk -v reason=pre-push -f .git/hooks/commit-msg-files.awk
+' | $awk -v reason=pre-push -f build-aux/git-hooks/commit-msg-files.awk

Re: build-aux/git-hooks/pre-push

[Po Lu]

Po Lu <luangruo@yahoo.com> writes:

> This script apparently loses when pushing from a separate worktree,
> where .git is a file containing the name of the repository the worktree
> was created from.
>
> The following adjustment seems to fix it.  However, I don't know much
> about git.  Objections?
>
> diff --git a/build-aux/git-hooks/pre-push b/build-aux/git-hooks/pre-push
> index 8e8277cba4f..8a2866f9d4c 100755
> --- a/build-aux/git-hooks/pre-push
> +++ b/build-aux/git-hooks/pre-push
> @@ -83,4 +83,4 @@ $awk -v origin_name="$1" '
>      # Print every SHA after oldref, up to (and including) newref.
>      system("git rev-list --first-parent --reverse " oldref ".." newref)
>    }
> -' | $awk -v reason=pre-push -f .git/hooks/commit-msg-files.awk
> +' | $awk -v reason=pre-push -f build-aux/git-hooks/commit-msg-files.awk

Btw, I notice that the script falls back to awk if gawk cannot be found.
Maybe in that case, the awk script should be rewritten to not utilize
getline, sub, function, and other features which do not work with Unix
awk?  Or maybe it should try harder to find a POSIX awk?

Thanks.

Re: build-aux/git-hooks/pre-push

[Jim Porter]

On 4/30/2023 11:33 PM, Po Lu wrote:
> Btw, I notice that the script falls back to awk if gawk cannot be found.
> Maybe in that case, the awk script should be rewritten to not utilize
> getline, sub, function, and other features which do not work with Unix
> awk?  Or maybe it should try harder to find a POSIX awk?

I'm not sure the best way to do it, but I think trying to find a POSIX 
awk would be the simplest solution. That awk script isn't *terribly* 
complex, but it uses enough POSIX awk features that I wouldn't want to 
try and make it work with UNIX awk (even if I knew exactly how to do that).

Re: build-aux/git-hooks/pre-push

[Po Lu]

Jim Porter <jporterbugs@gmail.com> writes:

> On 4/30/2023 11:33 PM, Po Lu wrote:
>> Btw, I notice that the script falls back to awk if gawk cannot be found.
>> Maybe in that case, the awk script should be rewritten to not utilize
>> getline, sub, function, and other features which do not work with Unix
>> awk?  Or maybe it should try harder to find a POSIX awk?
>
> I'm not sure the best way to do it, but I think trying to find a POSIX
> awk would be the simplest solution.

Hmm, ok.  Unfortunately for some reason the most reliable way to do that
I can think of would be to generate the script with Autoconf... and that
leads to a chicken-egg problem, since the hook is installed from
autogen.sh.

Perhaps we could simply look for:

  - /usr/xpg4/bin/awk
  - mawk
  - nawk

in that order, and not run the script at all if a suitable awk can't be
found.

> That awk script isn't *terribly* complex, but it uses enough POSIX awk
> features that I wouldn't want to try and make it work with UNIX awk
> (even if I knew exactly how to do that).

`(autoconf)Limitations of Usual Tools' describes the limitations of Unix
awk.

Re: build-aux/git-hooks/pre-push

[Jim Porter]

On 4/30/2023 11:29 PM, Po Lu wrote:
> This script apparently loses when pushing from a separate worktree,
> where .git is a file containing the name of the repository the worktree
> was created from.

Thanks for catching this bug.

> The following adjustment seems to fix it.  However, I don't know much
> about git.  Objections?
> 
> diff --git a/build-aux/git-hooks/pre-push b/build-aux/git-hooks/pre-push
> index 8e8277cba4f..8a2866f9d4c 100755
> --- a/build-aux/git-hooks/pre-push
> +++ b/build-aux/git-hooks/pre-push
> @@ -83,4 +83,4 @@ $awk -v origin_name="$1" '
>       # Print every SHA after oldref, up to (and including) newref.
>       system("git rev-list --first-parent --reverse " oldref ".." newref)
>     }
> -' | $awk -v reason=pre-push -f .git/hooks/commit-msg-files.awk
> +' | $awk -v reason=pre-push -f build-aux/git-hooks/commit-msg-files.awk
> 
> 

That won't quite work in general, since it would mean that if you check 
out an old branch, "build-aux/git-hooks/commit-msg-files.awk" won't 
exist, and then the hook will fail. Instead, I pushed a change to 
replace ".git" with "${GIT_DIR:-.git}", which is (as far as I can tell) 
the right way to do this.

(The fallback to ".git" in the expansion is purely defensive, just in 
case there's some old Git version that doesn't set that variable.)

Re: build-aux/git-hooks/pre-push

[Po Lu]

Jim Porter <jporterbugs@gmail.com> writes:

> On 4/30/2023 11:29 PM, Po Lu wrote:
>> This script apparently loses when pushing from a separate worktree,
>> where .git is a file containing the name of the repository the worktree
>> was created from.
>
> Thanks for catching this bug.
>
>> The following adjustment seems to fix it.  However, I don't know much
>> about git.  Objections?
>> diff --git a/build-aux/git-hooks/pre-push
>> b/build-aux/git-hooks/pre-push
>> index 8e8277cba4f..8a2866f9d4c 100755
>> --- a/build-aux/git-hooks/pre-push
>> +++ b/build-aux/git-hooks/pre-push
>> @@ -83,4 +83,4 @@ $awk -v origin_name="$1" '
>>       # Print every SHA after oldref, up to (and including) newref.
>>       system("git rev-list --first-parent --reverse " oldref ".." newref)
>>     }
>> -' | $awk -v reason=pre-push -f .git/hooks/commit-msg-files.awk
>> +' | $awk -v reason=pre-push -f build-aux/git-hooks/commit-msg-files.awk
>> 
>
> That won't quite work in general, since it would mean that if you
> check out an old branch, "build-aux/git-hooks/commit-msg-files.awk"
> won't exist, and then the hook will fail. Instead, I pushed a change
> to replace ".git" with "${GIT_DIR:-.git}", which is (as far as I can
> tell) the right way to do this.
>
> (The fallback to ".git" in the expansion is purely defensive, just in
> case there's some old Git version that doesn't set that variable.)

Oh, OK.  Thanks!

Re: build-aux/git-hooks/pre-push

[Basil Contovounesios]

Jim Porter [2023-05-01 09:55 -0700] wrote:

> On 4/30/2023 11:29 PM, Po Lu wrote:
>> This script apparently loses when pushing from a separate worktree,
>> where .git is a file containing the name of the repository the worktree
>> was created from.
>
> Thanks for catching this bug.
>
>> The following adjustment seems to fix it.  However, I don't know much
>> about git.  Objections?
>> diff --git a/build-aux/git-hooks/pre-push b/build-aux/git-hooks/pre-push
>> index 8e8277cba4f..8a2866f9d4c 100755
>> --- a/build-aux/git-hooks/pre-push
>> +++ b/build-aux/git-hooks/pre-push
>> @@ -83,4 +83,4 @@ $awk -v origin_name="$1" '
>>       # Print every SHA after oldref, up to (and including) newref.
>>       system("git rev-list --first-parent --reverse " oldref ".." newref)
>>     }
>> -' | $awk -v reason=pre-push -f .git/hooks/commit-msg-files.awk
>> +' | $awk -v reason=pre-push -f build-aux/git-hooks/commit-msg-files.awk
>> 
>
> That won't quite work in general, since it would mean that if you check out an
> old branch, "build-aux/git-hooks/commit-msg-files.awk" won't exist, and then the
> hook will fail. Instead, I pushed a change to replace ".git" with
> "${GIT_DIR:-.git}", which is (as far as I can tell) the right way to do this.
>
> (The fallback to ".git" in the expansion is purely defensive, just in case
> there's some old Git version that doesn't set that variable.)

I have a clone of emacs.git under ~/.local/src/emacs, where the master
branch is checked out under the same name.

Then I have a worktree under ~/.local/src/emacs-29, where the emacs-29
branch is checked out in a branch named wt/emacs-29.

I get the following when I try to push from that worktree:

$ git push upstream wt/emacs-29:emacs-29
gawk: fatal: cannot open source file `/home/blc/.local/src/emacs/.git/worktrees/emacs-29/hooks/commit-msg-files.awk' for reading: No such file or directory
error: failed to push some refs to 'git.savannah.gnu.org:/srv/git/emacs.git'

So maybe we should be defensive about the existence of this hook?
Or am I doing something wrong?

Thanks,

-- 
Basil

Re: build-aux/git-hooks/pre-push

[Jim Porter]

[-- Attachment #1: Type: text/plain, Size: 267 bytes --]

On 5/4/2023 3:37 AM, Basil Contovounesios wrote:
> So maybe we should be defensive about the existence of this hook?
> Or am I doing something wrong?

I guess that's what I get for just trusting the Git manual instead of 
verifying that my fix worked. How about this?

[-- Attachment #2: 0001-Fix-post-commit-and-pre-push-hooks-in-worktrees-agai.patch --]
[-- Type: text/plain, Size: 1755 bytes --]

From d3ec68f5e433e5792c1c63672c7b437bb29c5759 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 4 May 2023 09:22:40 -0700
Subject: [PATCH] ; Fix post-commit and pre-push hooks in worktrees again

* build-aux/git-hooks/post-commit:
* build-aux/git-hooks/pre-push: Use "$(dirname $0)" to get the hooks
directory.
---
 build-aux/git-hooks/post-commit | 4 +++-
 build-aux/git-hooks/pre-push    | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/build-aux/git-hooks/post-commit b/build-aux/git-hooks/post-commit
index 05f2d778b5c..10f43b539ac 100755
--- a/build-aux/git-hooks/post-commit
+++ b/build-aux/git-hooks/post-commit
@@ -34,6 +34,8 @@
 
 ### Code:
 
+HOOKS_DIR=$(dirname $0)
+
 # Prefer gawk if available, as it handles NUL bytes properly.
 if type gawk >/dev/null 2>&1; then
   awk="gawk"
@@ -42,4 +44,4 @@ else
 fi
 
 git rev-parse HEAD | $awk -v reason=post-commit \
-                          -f ${GIT_DIR:-.git}/hooks/commit-msg-files.awk
+                          -f $HOOKS_DIR/commit-msg-files.awk
diff --git a/build-aux/git-hooks/pre-push b/build-aux/git-hooks/pre-push
index 6ff59102fd7..8d5dde2bbaf 100755
--- a/build-aux/git-hooks/pre-push
+++ b/build-aux/git-hooks/pre-push
@@ -31,6 +31,8 @@
 
 ### Code:
 
+HOOKS_DIR=$(dirname $0)
+
 # Prefer gawk if available, as it handles NUL bytes properly.
 if type gawk >/dev/null 2>&1; then
   awk="gawk"
@@ -83,4 +85,4 @@ $awk -v origin_name="$1" '
     # Print every SHA after oldref, up to (and including) newref.
     system("git rev-list --first-parent --reverse " oldref ".." newref)
   }
-' | $awk -v reason=pre-push -f ${GIT_DIR:-.git}/hooks/commit-msg-files.awk
+' | $awk -v reason=pre-push -f $HOOKS_DIR/commit-msg-files.awk
-- 
2.25.1

Re: build-aux/git-hooks/pre-push

[Basil Contovounesios]

Jim Porter [2023-05-04 09:28 -0700] wrote:

> How about this?

It allows git-push to succeed in the emacs-29 worktree.
Thanks,

-- 
Basil

Re: build-aux/git-hooks/pre-push

[Jim Porter]

On 5/4/2023 9:42 AM, Basil Contovounesios wrote:
> It allows git-push to succeed in the emacs-29 worktree.
> Thanks,

Thanks, pushed to master.