git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git 2.36.0 regression: pre-commit hooks no longer have stdout/stderr as tty
@ 2022-04-19 18:59 Anthony Sottile
  2022-04-19 23:37 ` Emily Shaffer
                   ` (2 more replies)
  0 siblings, 3 replies; 85+ messages in thread
From: Anthony Sottile @ 2022-04-19 18:59 UTC (permalink / raw)
  To: Git Mailing List

here's the shortest reproduction --

```console
$ cat ../testrepo/.git/hooks/pre-commit
#!/usr/bin/env bash
if [ -t 1 ]; then
    echo GOOD
fi
```

in previous git versions:

```
$ git commit -q --allow-empty -m foo
GOOD
$
```

with git 2.36.0:

````
$ git commit -q --allow-empty -m foo
$
```

why I care: I maintain a git hooks framework which uses `isatty` to
detect whether it's appropriate to color the output.  many tools
utilize the same check.  in 2.36.0+ isatty is false for stdout and
stderr causing coloring to be turned off.

I bisected this (it was a little complicated, needed to force a pty):

`../testrepo`: a git repo set up with the hook above

`../bisect.sh`:

```bash
#!/usr/bin/env bash
set -eux
git clean -fxfd >& /dev/null
make -j6 prefix="$PWD/prefix" NO_GETTEXT=1 NO_TCLTK=1 install >& /dev/null
export PATH="$PWD/prefix/bin:$PATH"
cd ../testrepo
(../pty git commit -q --allow-empty -m foo || true) | grep GOOD
```

`../pty`:

```python
#!/usr/bin/env python3
import errno
import os
import subprocess
import sys

x: int = 'nope'


class Pty(object):
    def __init__(self):
        self.r = self.w = None

    def __enter__(self):
        self.r, self.w = os.openpty()

        return self

    def close_w(self):
        if self.w is not None:
            os.close(self.w)
            self.w = None

    def close_r(self):
        assert self.r is not None
        os.close(self.r)
        self.r = None

    def __exit__(self, exc_type, exc_value, traceback):
        self.close_w()
        self.close_r()


def cmd_output_p(*cmd, **kwargs):
    with open(os.devnull) as devnull, Pty() as pty:
        kwargs = {'stdin': devnull, 'stdout': pty.w, 'stderr': pty.w}
        proc = subprocess.Popen(cmd, **kwargs)
        pty.close_w()

        buf = b''
        while True:
            try:
                bts = os.read(pty.r, 4096)
            except OSError as e:
                if e.errno == errno.EIO:
                    bts = b''
                else:
                    raise
            else:
                buf += bts
            if not bts:
                break

    return proc.wait(), buf, None


if __name__ == '__main__':
    _, buf, _ = cmd_output_p(*sys.argv[1:])
    sys.stdout.buffer.write(buf)
```

the first commit it points out:

```
f443246b9f29b815f0b98a07bb2d425628ae6522 is the first bad commit
commit f443246b9f29b815f0b98a07bb2d425628ae6522
Author: Emily Shaffer <emilyshaffer@google.com>
Date:   Wed Dec 22 04:59:40 2021 +0100

    commit: convert {pre-commit,prepare-commit-msg} hook to hook.h

    Move these hooks hook away from run-command.h to and over to the new
    hook.h library.

    Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    Acked-by: Emily Shaffer <emilyshaffer@google.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

 commit.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
bisect run success
```


Anthony

^ permalink raw reply	[flat|nested] 85+ messages in thread

end of thread, other threads:[~2022-06-17  0:07 UTC | newest]

Thread overview: 85+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 18:59 git 2.36.0 regression: pre-commit hooks no longer have stdout/stderr as tty Anthony Sottile
2022-04-19 23:37 ` Emily Shaffer
2022-04-19 23:52   ` Anthony Sottile
2022-04-20  9:00   ` Phillip Wood
2022-04-20 12:25     ` Ævar Arnfjörð Bjarmason
2022-04-20 16:22       ` Emily Shaffer
2022-04-20 16:42     ` Junio C Hamano
2022-04-20 17:09       ` Emily Shaffer
2022-04-20 17:25         ` Junio C Hamano
2022-04-20 17:41           ` Emily Shaffer
2022-04-21 12:03             ` Ævar Arnfjörð Bjarmason
2022-04-21 17:24               ` Junio C Hamano
2022-04-21 18:40                 ` Junio C Hamano
2022-04-20  4:23 ` Junio C Hamano
2022-04-21 12:25 ` [PATCH 0/6] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Ævar Arnfjörð Bjarmason
2022-04-21 12:25   ` [PATCH 1/6] run-command API: replace run_processes_parallel_tr2() with opts struct Ævar Arnfjörð Bjarmason
2022-04-23  4:24     ` Junio C Hamano
2022-04-28 23:16     ` Emily Shaffer
2022-04-29 16:44       ` Junio C Hamano
2022-04-21 12:25   ` [PATCH 2/6] run-command tests: test stdout of run_command_parallel() Ævar Arnfjörð Bjarmason
2022-04-23  4:24     ` Junio C Hamano
2022-04-21 12:25   ` [PATCH 3/6] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-04-23  3:54     ` Junio C Hamano
2022-04-28 23:26     ` Emily Shaffer
2022-04-21 12:25   ` [PATCH 4/6] hook tests: fix redirection logic error in 96e7225b310 Ævar Arnfjörð Bjarmason
2022-04-23  3:54     ` Junio C Hamano
2022-04-21 12:25   ` [PATCH 5/6] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr" Ævar Arnfjörð Bjarmason
2022-04-29 22:54     ` Junio C Hamano
2022-04-21 12:25   ` [PATCH 6/6] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-04-28 23:31     ` Emily Shaffer
2022-04-29 23:09     ` Junio C Hamano
2022-04-21 17:35   ` [PATCH 0/6] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano
2022-04-21 18:50     ` Ævar Arnfjörð Bjarmason
2022-05-18 20:05   ` [PATCH v2 0/8] " Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 1/8] run-command tests: change if/if/... to if/else if/else Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 2/8] run-command API: use "opts" struct for run_processes_parallel{,_tr2}() Ævar Arnfjörð Bjarmason
2022-05-18 21:45       ` Junio C Hamano
2022-05-25 13:18       ` Emily Shaffer
2022-05-18 20:05     ` [PATCH v2 3/8] run-command tests: test stdout of run_command_parallel() Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 4/8] run-command.c: add an initializer for "struct parallel_processes" Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 5/8] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-05-18 21:51       ` Junio C Hamano
2022-05-26 17:18       ` Emily Shaffer
2022-05-27 16:08         ` Junio C Hamano
2022-05-18 20:05     ` [PATCH v2 6/8] hook tests: fix redirection logic error in 96e7225b310 Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 7/8] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr" Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 8/8] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-05-18 21:53       ` Junio C Hamano
2022-05-26 17:23       ` Emily Shaffer
2022-05-26 18:23         ` Ævar Arnfjörð Bjarmason
2022-05-26 18:54           ` Emily Shaffer
2022-05-25 11:30     ` [PATCH v2 0/8] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Johannes Schindelin
2022-05-25 13:00       ` Ævar Arnfjörð Bjarmason
2022-05-25 16:57       ` Junio C Hamano
2022-05-26  1:10         ` Junio C Hamano
2022-05-26 10:16           ` Ævar Arnfjörð Bjarmason
2022-05-26 16:36             ` Junio C Hamano
2022-05-26 19:13               ` Ævar Arnfjörð Bjarmason
2022-05-26 19:56                 ` Junio C Hamano
2022-05-27  9:14     ` [PATCH v3 0/2] " Ævar Arnfjörð Bjarmason
2022-05-27  9:14       ` [PATCH v3 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-05-27 16:58         ` Junio C Hamano
2022-05-27  9:14       ` [PATCH v3 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-05-27 17:17       ` [PATCH v3 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano
2022-05-31 17:32       ` [PATCH v4 " Ævar Arnfjörð Bjarmason
2022-05-31 17:32         ` [PATCH v4 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-06-01 16:49           ` Johannes Schindelin
2022-06-01 17:09             ` Junio C Hamano
2022-05-31 17:32         ` [PATCH v4 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-06-01 16:50           ` Johannes Schindelin
2022-06-01 16:53         ` [PATCH v4 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Johannes Schindelin
2022-06-02 14:07         ` [PATCH v5 " Ævar Arnfjörð Bjarmason
2022-06-02 14:07           ` [PATCH v5 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-06-02 14:07           ` [PATCH v5 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-06-02 20:05           ` [PATCH v5 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano
2022-06-03  8:51           ` Phillip Wood
2022-06-03  9:20             ` Ævar Arnfjörð Bjarmason
2022-06-03 13:21               ` Phillip Wood
2022-06-07  8:49                 ` Ævar Arnfjörð Bjarmason
2022-06-07  8:48           ` [PATCH v6 " Ævar Arnfjörð Bjarmason
2022-06-07  8:48             ` [PATCH v6 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-06-17  0:07               ` Emily Shaffer
2022-06-07  8:48             ` [PATCH v6 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-06-07 17:08               ` Junio C Hamano
2022-06-07 17:02             ` [PATCH v6 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).