bug report: pre-push hook

bug report: pre-push hook

[Honza Prokeš]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
I did setup pre-push hook to run unit tests.

What did you expect to happen? (Expected behavior)
Pre-push executes tests, I can see output, and if tests fail, push
does not happen.

What happened instead? (Actual behavior)
Tests seem to execute on the background, but I do not see output and
push happens when tests fail.

What's different between what you expected and what actually happened?
As above, so:
Tests seem to execute on the background, but I do not see output and
push happens when tests fail.

Anything else you want to add:
When I run pre-push script directly from terminal, it executes tests,
I see output and if I ask for last command exit-code, it is non-zero
if tests fail, so my script is functional. It was working until
recently, noticed today, but this behavior happened at least two days
ago. It was working well in past. My fella has the same issue with
similar script on up-to-date Arch Linux.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.35.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.10.105-1-MANJARO #1 SMP PREEMPT Fri Mar 11 14:12:33 UTC
2022 x86_64
compiler info: gnuc: 11.1
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /usr/bin/zsh


[Enabled Hooks]
pre-push

S pozdravem / Best regards

Jan Prokeš

Full stack programátor / Web developer

Re: bug report: pre-push hook

[Emily Shaffer]

On Thu, Mar 24, 2022 at 9:45 AM Honza Prokeš <proke.j@gmail.com> wrote:
>
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
> I did setup pre-push hook to run unit tests.
>
> What did you expect to happen? (Expected behavior)
> Pre-push executes tests, I can see output, and if tests fail, push
> does not happen.
>
> What happened instead? (Actual behavior)
> Tests seem to execute on the background, but I do not see output

I wonder - are you printing your test output to stdout, or stderr?
stderr of the hook is printed to stdout of the Git process, so that
may be why.

> and
> push happens when tests fail.

This part I am a little less sure about, especially without seeing
your hook script - but perhaps getting the output thing figured out
first will help figure out this part?

>
> What's different between what you expected and what actually happened?
> As above, so:
> Tests seem to execute on the background, but I do not see output and
> push happens when tests fail.
>
> Anything else you want to add:
> When I run pre-push script directly from terminal, it executes tests,
> I see output and if I ask for last command exit-code, it is non-zero
> if tests fail, so my script is functional. It was working until
> recently, noticed today, but this behavior happened at least two days
> ago.

Oh, interesting! There has been a bit of hook-handling backend work
going on lately (ccing Ævar who has been more actively working on it
than me) so it's possible it's a regression, although we do have tests
covering pre-push hook in t/t5571-pre-push-hook.sh; I see that those
tests cover nonzero exit, but it doesn't look like they cover output
capturing (the test hooks are cat-ing to a file, rather than trying to
print their own output).

Any chance that you can provide a minified version of your pre-push
hook that reliably tries to fail and doesn't prevent push from
happening? That would help a lot to track down the issue.

> It was working well in past. My fella has the same issue with
> similar script on up-to-date Arch Linux.
>
> [System Info]
> git version:
> git version 2.35.1

Would also be nice to know, if you happen to know, what version you
used before when it worked nicely. But if you don't happen to have
that handy then don't worry about it.

 - Emily

Re: bug report: pre-push hook

[Taylor Blau]

On Thu, Mar 24, 2022 at 11:30:16AM -0700, Emily Shaffer wrote:
> > What happened instead? (Actual behavior)
> > Tests seem to execute on the background, but I do not see output
>
> I wonder - are you printing your test output to stdout, or stderr?
> stderr of the hook is printed to stdout of the Git process, so that
> may be why.

I didn't think that we redirected the pre-push (or any) hook's stderr to
stdout in the parent process. I wrote a small pre-push hook which is
just:

    $ cat .git/hooks/pre-push
    #!/bin/sh

    echo >&2 "hi!"

and then:

    $ git.compile push ttaylorr --dry-run 2>foo
    $ grep hi foo
    hi!

But I might be holding it wrong, since I am not a frequent user of
hooks.

Thanks,
Taylor

Re: bug report: pre-push hook

[Emily Shaffer]

On Thu, Mar 24, 2022 at 2:29 PM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Thu, Mar 24, 2022 at 11:30:16AM -0700, Emily Shaffer wrote:
> > > What happened instead? (Actual behavior)
> > > Tests seem to execute on the background, but I do not see output
> >
> > I wonder - are you printing your test output to stdout, or stderr?
> > stderr of the hook is printed to stdout of the Git process, so that
> > may be why.
>
> I didn't think that we redirected the pre-push (or any) hook's stderr to
> stdout in the parent process. I wrote a small pre-push hook which is
> just:
>
>     $ cat .git/hooks/pre-push
>     #!/bin/sh
>
>     echo >&2 "hi!"
>
> and then:
>
>     $ git.compile push ttaylorr --dry-run 2>foo
>     $ grep hi foo
>     hi!
>
> But I might be holding it wrong, since I am not a frequent user of
> hooks.

Ah, it seems I'm completely mistaken :) Sorry for the misinfo. Urk.

Having a look in 'master', run_pre_push_hook() does not use hook.c
machinery yet at all; it also seems like it doesn't try to capture
stdout from the pre-push child process anyways. That makes me ask "how
did this ever work?" which brings me back to wanting a small hook we
can try to reproduce with from Honza :)

 - Emily