threads / discuss / 64786

pre-push hooks and stdout regression

Subject: pre-push hooks and stdout regression

## tl;dr

5 messages between Jan 12, 2026 and Jan 13, 2026.

replies: 4people: 3as markdown or json

Chris Darroch· Jan 12, 2026, 23:21 UTC · lore
Hello --
   I'm one of the current maintainers of the Git LFS project, and we
happened to notice that a recent change in Git's "master" branch has
introduced a regression in our test suite.
   Specifically, with commit 3e2836a742d8b2b2da25ca06e9d0ac3a539bd966
("transport: convert pre-push to hook API") from the "ar/run-command-hook"
merged last week, it appears that when a pre-push hook such as our
git-lfs-pre-push program runs, messages it writes to its standard output
are now delivered to the user's standard error stream instead of
their standard output stream.
   I suspect this is because the pick_next_hook() function in hook.c
sets the stdout_to_stderr flag for its "cb" child_process argument,
and that function is now used to run the pre-push hook.
   Arguably, the Git LFS pre-push hook program should write its
progress meter messages to stderr, but since at least 2017 it appears
we have used stdout for this purpose:

https://github.com/git-lfs/git-lfs/commit/d665f7d725150761fe3b196da2c2d4448f7d2c61 https://github.com/git-lfs/git-lfs/pull/2732

   We can certainly work around this change in the Git LFS test suite,
since our progress messages are still output by Git, just to stderr
instead of stdout.
   However, I think there remains the larger concern that users who
depend on the existing Git pre-push behaviour in some way may also
encounter regressions, perhaps because they expect (as our test suite
does) to see certain messages either output or not output to stderr
during a Git push operation.
   Please do let me know your thoughts on this subject!  If the
consensus is that the new behaviour is correct, we'll adjust our test
suite to match it, but I'll wait to hear the outcome of any discussion
before making that change.
   Thank you again and all the best,
Chris.
-- 
GPG Key ID: 088335A9
GPG Key Fingerprint: 86CD 3297 7493 75BC F820  6715 F54F E648 0883 35A9
brian m. carlson· Jan 13, 2026, 02:12 UTC · re: Chris Darroch · lore

Re: pre-push hooks and stdout regression

On 2026-01-12 at 23:21:42, Chris Darroch wrote:
Show 12 quoted lines
> Hello --
> 
>    I'm one of the current maintainers of the Git LFS project, and we
> happened to notice that a recent change in Git's "master" branch has
> introduced a regression in our test suite.
> 
>    Specifically, with commit 3e2836a742d8b2b2da25ca06e9d0ac3a539bd966
> ("transport: convert pre-push to hook API") from the "ar/run-command-hook"
> merged last week, it appears that when a pre-push hook such as our
> git-lfs-pre-push program runs, messages it writes to its standard output
> are now delivered to the user's standard error stream instead of
> their standard output stream.
CCing the author and submitter.
>    I suspect this is because the pick_next_hook() function in hook.c
> sets the stdout_to_stderr flag for its "cb" child_process argument,
> and that function is now used to run the pre-push hook.

That's likely the case. 96e7225b31 ("hook: add 'run' subcommand", 2021-12-22), which introduced that code, didn't provide an explanation for that decision. I think that's required for hooks such as pre-receive that send data over the sideband, and it may be that the code was simply reused. Of course, I could be mistaken.

Show 5 quoted lines
>    However, I think there remains the larger concern that users who
> depend on the existing Git pre-push behaviour in some way may also
> encounter regressions, perhaps because they expect (as our test suite
> does) to see certain messages either output or not output to stderr
> during a Git push operation.

I suspect that we're also going to see similar regressions in other software. pre-push hooks are really popular, as well as pre-commit hooks, and people are going to expect both standard output and standard error to be connected to the same place they were before.

A quick check of pre-push hook frameworks on GitHub indicates that a substantial number of them print to standard output, even for error messages. I expect that we will actually see quite a bit of breakage in this case, especially from automated tooling and Git graphical frontends, which may expect data in a certain place. I don't have any proof of this, however, but it does seem like the kind of thing that people will have come to rely on.

I'd recommend that we avoid using `stdout_to_stderr` for hooks unless we did originally or really need to (e.g., for things that might go over the sideband).

>    Please do let me know your thoughts on this subject!  If the
> consensus is that the new behaviour is correct, we'll adjust our test
> suite to match it, but I'll wait to hear the outcome of any discussion
> before making that change.

I will just state for the benefit of the list that Chris and I are colleagues and we discussed this at work, so I asked him to CC me on this issue since I was curious. My opinions here, as with everything originating from this email address, are mine and not my employer's.

-- 
brian m. carlson (they/them)
Toronto, Ontario, CA
Adrian Ratiu· Jan 13, 2026, 09:49 UTC · re: Chris Darroch · lore

Re: pre-push hooks and stdout regression

On Mon, 12 Jan 2026, Chris Darroch <chrisd@apache.org> wrote:
Show 40 quoted lines
> Hello --
>
>    I'm one of the current maintainers of the Git LFS project, and we
> happened to notice that a recent change in Git's "master" branch has
> introduced a regression in our test suite.
>
>    Specifically, with commit 3e2836a742d8b2b2da25ca06e9d0ac3a539bd966
> ("transport: convert pre-push to hook API") from the "ar/run-command-hook"
> merged last week, it appears that when a pre-push hook such as our
> git-lfs-pre-push program runs, messages it writes to its standard output
> are now delivered to the user's standard error stream instead of
> their standard output stream.
>
>    I suspect this is because the pick_next_hook() function in hook.c
> sets the stdout_to_stderr flag for its "cb" child_process argument,
> and that function is now used to run the pre-push hook.
>
>    Arguably, the Git LFS pre-push hook program should write its
> progress meter messages to stderr, but since at least 2017 it appears
> we have used stdout for this purpose:
>
> https://github.com/git-lfs/git-lfs/commit/d665f7d725150761fe3b196da2c2d4448f7d2c61
> https://github.com/git-lfs/git-lfs/pull/2732
>
>    We can certainly work around this change in the Git LFS test suite,
> since our progress messages are still output by Git, just to stderr
> instead of stdout.
>
>    However, I think there remains the larger concern that users who
> depend on the existing Git pre-push behaviour in some way may also
> encounter regressions, perhaps because they expect (as our test suite
> does) to see certain messages either output or not output to stderr
> during a Git push operation.
>
>    Please do let me know your thoughts on this subject!  If the
> consensus is that the new behaviour is correct, we'll adjust our test
> suite to match it, but I'll wait to hear the outcome of any discussion
> before making that change.
>
>    Thank you again and all the best,

Thank you for reporting this, it's exactly the kind of regressions I'm looking for and the reason I did the "Extending git without breaking it" presentation during the mini-summit a few months ago (video should be online).

I tend to agree with Brian that going back to the previous behavior is best for now, maybe schedule a breaking change or extension to make hooks to print to stderr instead of stdout.

I will test this on my parallel config based hooks topic towards which this conversion is building up to and send a patch or report back ASAP.

Of course I will also run the git LFS test suite to confirm the regression and the fix.

Adrian Ratiu· Jan 13, 2026, 12:31 UTC · re: Adrian Ratiu · lore

Re: pre-push hooks and stdout regression

On Tue, 13 Jan 2026, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
Show 56 quoted lines
> On Mon, 12 Jan 2026, Chris Darroch <chrisd@apache.org> wrote:
>> Hello --
>>
>>    I'm one of the current maintainers of the Git LFS project, and we
>> happened to notice that a recent change in Git's "master" branch has
>> introduced a regression in our test suite.
>>
>>    Specifically, with commit 3e2836a742d8b2b2da25ca06e9d0ac3a539bd966
>> ("transport: convert pre-push to hook API") from the "ar/run-command-hook"
>> merged last week, it appears that when a pre-push hook such as our
>> git-lfs-pre-push program runs, messages it writes to its standard output
>> are now delivered to the user's standard error stream instead of
>> their standard output stream.
>>
>>    I suspect this is because the pick_next_hook() function in hook.c
>> sets the stdout_to_stderr flag for its "cb" child_process argument,
>> and that function is now used to run the pre-push hook.
>>
>>    Arguably, the Git LFS pre-push hook program should write its
>> progress meter messages to stderr, but since at least 2017 it appears
>> we have used stdout for this purpose:
>>
>> https://github.com/git-lfs/git-lfs/commit/d665f7d725150761fe3b196da2c2d4448f7d2c61
>> https://github.com/git-lfs/git-lfs/pull/2732
>>
>>    We can certainly work around this change in the Git LFS test suite,
>> since our progress messages are still output by Git, just to stderr
>> instead of stdout.
>>
>>    However, I think there remains the larger concern that users who
>> depend on the existing Git pre-push behaviour in some way may also
>> encounter regressions, perhaps because they expect (as our test suite
>> does) to see certain messages either output or not output to stderr
>> during a Git push operation.
>>
>>    Please do let me know your thoughts on this subject!  If the
>> consensus is that the new behaviour is correct, we'll adjust our test
>> suite to match it, but I'll wait to hear the outcome of any discussion
>> before making that change.
>>
>>    Thank you again and all the best,
>
> Thank you for reporting this, it's exactly the kind of regressions I'm
> looking for and the reason I did the "Extending git without breaking it"
> presentation during the mini-summit a few months ago (video should be
> online).
>
> I tend to agree with Brian that going back to the previous behavior is
> best for now, maybe schedule a breaking change or extension to make
> hooks to print to stderr instead of stdout.
>
> I will test this on my parallel config based hooks topic towards which
> this conversion is building up to and send a patch or report back ASAP.
>
> Of course I will also run the git LFS test suite to confirm the
> regression and the fix.

I couldn't reproduce the git-lfs test failure using the public tests from the git-lfs repo (they always pass), however this patch should fix it:

https://lore.kernel.org/git/20260113115633.230479-1-adrian.ratiu@collabora.com/T/#u
Chris or Brian, can you please confirm if it works? Much appreciated.
Chris Darroch· Jan 13, 2026, 20:44 UTC · re: Adrian Ratiu · lore

Re: pre-push hooks and stdout regression

Adrian:
Show 65 quoted lines
> On Tue, 13 Jan 2026, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>> On Mon, 12 Jan 2026, Chris Darroch <chrisd@apache.org> wrote:
>>> Hello --
>>>
>>>    I'm one of the current maintainers of the Git LFS project, and we
>>> happened to notice that a recent change in Git's "master" branch has
>>> introduced a regression in our test suite.
>>>
>>>    Specifically, with commit 3e2836a742d8b2b2da25ca06e9d0ac3a539bd966
>>> ("transport: convert pre-push to hook API") from the "ar/run-command-hook"
>>> merged last week, it appears that when a pre-push hook such as our
>>> git-lfs-pre-push program runs, messages it writes to its standard output
>>> are now delivered to the user's standard error stream instead of
>>> their standard output stream.
>>>
>>>    I suspect this is because the pick_next_hook() function in hook.c
>>> sets the stdout_to_stderr flag for its "cb" child_process argument,
>>> and that function is now used to run the pre-push hook.
>>>
>>>    Arguably, the Git LFS pre-push hook program should write its
>>> progress meter messages to stderr, but since at least 2017 it appears
>>> we have used stdout for this purpose:
>>>
>>> https://github.com/git-lfs/git-lfs/commit/d665f7d725150761fe3b196da2c2d4448f7d2c61
>>> https://github.com/git-lfs/git-lfs/pull/2732
>>>
>>>    We can certainly work around this change in the Git LFS test suite,
>>> since our progress messages are still output by Git, just to stderr
>>> instead of stdout.
>>>
>>>    However, I think there remains the larger concern that users who
>>> depend on the existing Git pre-push behaviour in some way may also
>>> encounter regressions, perhaps because they expect (as our test suite
>>> does) to see certain messages either output or not output to stderr
>>> during a Git push operation.
>>>
>>>    Please do let me know your thoughts on this subject!  If the
>>> consensus is that the new behaviour is correct, we'll adjust our test
>>> suite to match it, but I'll wait to hear the outcome of any discussion
>>> before making that change.
>>>
>>>    Thank you again and all the best,
>>
>> Thank you for reporting this, it's exactly the kind of regressions I'm
>> looking for and the reason I did the "Extending git without breaking it"
>> presentation during the mini-summit a few months ago (video should be
>> online).
>>
>> I tend to agree with Brian that going back to the previous behavior is
>> best for now, maybe schedule a breaking change or extension to make
>> hooks to print to stderr instead of stdout.
>>
>> I will test this on my parallel config based hooks topic towards which
>> this conversion is building up to and send a patch or report back ASAP.
>>
>> Of course I will also run the git LFS test suite to confirm the
>> regression and the fix.
> 
> I couldn't reproduce the git-lfs test failure using the public tests
> from the git-lfs repo (they always pass), however this patch should fix
> it:
> 
> https://lore.kernel.org/git/20260113115633.230479-1-adrian.ratiu@collabora.com/T/#u
> 
> Chris or Brian, can you please confirm if it works? Much appreciated.
   Thanks very much for taking a look at this!  I can confirm this
initial patch works to resolve our Git LFS test suite failures.  I'll
keep an eye on the subsequent patch proposals and try to test them as
they are developed.
   For reference, the Git LFS test suite failures occurred in places where
our shell tests happened to only capture messages from "git push" commands
on stdout, instead of capturing both stdout and stderr.  For example,
the "fetch: setup for include test" test in our t/t-fetch-include.sh
script was failing at this grep(1) command:
https://github.com/git-lfs/git-lfs/blob/1f2f7c697b5568b4bf1ed76e7ed4115dd1f0b3ba/t/t-fetch-include.sh#L39
   One can run these shell tests by entering the "t" directory, e.g.:

$ cd t $ make t-fetch-include.sh

   (Confusingly, the Git LFS project also has a bunch of Go language
tests which "make test" runs if you're in the top-level directory,
while "make test" in the "t" directory runs the shell test suite instead.)
   Thanks again for taking time to help resolve this issue!
All the best,
Chris.
-- 
GPG Key ID: 088335A9
GPG Key Fingerprint: 86CD 3297 7493 75BC F820  6715 F54F E648 0883 35A9

← back to recent threads