On Mon, Jul 22, 2024 at 10:22:58AM -0700, Junio C Hamano wrote:
> Attached at the end is a test tweak patch, taking inspirations from
> Phillip's comments, to see what value GIT_PAGER has in the shell
> function. I shortened the huge_file a bit so that I do not have to
> have an infinite scrollback buffer,but otherwise, the test_quirk
> intermediate shell function should work just like the test_terminal
> helper in the original position would.
>
> And I see in the output from "sh t3701-add-interactive.sh -i -v":
>
> expecting success of 3701.51 'P handles SIGPIPE when writing to pager':
> test_when_finished "rm -f huge_file; git reset" &&
> printf "\n%250s" Y >huge_file &&
> git add -N huge_file &&
> echo "in env: GIT_PAGER=$(env | grep GIT_PAGER=)" &&
> test_write_lines P q | GIT_PAGER="head -n 1" test_quirk &&
> echo "after test_quirk returns: GIT_PAGER=$GIT_PAGER"
>
> in env: GIT_PAGER=
> in test_quirk: GIT_PAGER=head -n 1
> in env: GIT_PAGER=GIT_PAGER=head -n 1
> In test_terminal: GIT_PAGER=GIT_PAGER=head -n 1
> test-terminal: GIT_PAGER=head -n 1
> diff --git a/huge_file b/huge_file
> new file mode 100644
> index 0000000..d06820d
> --- /dev/null
> +++ b/huge_file
> @@ -0,0 +1,2 @@
> +
> + Y
> \ No newline at end of file
> (1/1) Stage addition [y,n,q,a,d,e,p,?]? @@ -0,0 +1,2 @@
> (1/1) Stage addition [y,n,q,a,d,e,p,?]?
> after test_quirk returns: GIT_PAGER=
> Unstaged changes after reset:
> M test
> ok 51 - P handles SIGPIPE when writing to pager
>
> So:
>
> - before the one-shot thing, in the envrionment GIT_PAGER is empty.
> - in the helper function,
> - shell variable GIT_PAGER is set to the expected value.
> - GIT_PAGER env is exported.
> - test-terminal.perl sees $ENV{GIT_PAGER} set to the expected value.
> - after the helper returns GIT_PAGER is empty
>
> It's a very convincing theory but it does not seem to match my
> observation. Is there a difference in shells used, or something?
Have you tried your tweak in the "linux-gcc (ubuntu-20.04)" test
environment where the problem was detected? In that environment, the
value of GIT_PAGER is not passed to Git in that test.
To fix the test, as already said, we need this:
test_write_lines P q |
(
GIT_PAGER="head -n 1" &&
export GIT_PAGER &&
test_terminal git add -p >actual
)
And this series also need the other other change that I'm discussing
with Phillip:
diff --git a/pager.c b/pager.c
index 5f0c1e9cce..5586e751dc 100644
--- a/pager.c
+++ b/pager.c
@@ -46,6 +46,8 @@ static void wait_for_pager_atexit(void)
void wait_for_pager(void)
{
+ if (old_fd1 == -1)
+ return;
finish_pager();
sigchain_pop_common();
unsetenv("GIT_PAGER_IN_USE");
