Hi Ruében
Thanks for looking into the test failure.
On 17/07/2024 18:20, Rubén Justo wrote:
Squashing this fixes the test:
--->8---
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index c60589cb94..bb360c92a0 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -616,7 +616,12 @@ test_expect_success TTY 'P handles SIGPIPE when writing to pager' '
test_when_finished "rm -f huge_file; git reset" &&
printf "\n%2500000s" Y >huge_file &&
git add -N huge_file &&
- test_write_lines P q | GIT_PAGER="head -n 1" test_terminal git add -p
+ test_write_lines P q |
+ (
+ GIT_PAGER="head -n 1" &&
+ export GIT_PAGER &&
+ test_terminal git add -p >actual
+ )
That's surprising, why does running git in a sub-shell stop it from
segfaulting?
---8<---
However, this error has exposed a problem: calling `wait_for_pager` if
`setup_pager` hasn't worked is an issue that needs to be addressed in this
series: `setup_pager` should return a result. I was planning to do that
in a future series, for the other commented command: `|[cmd]`.
What was causing setup pager to fail in this test?
I'm wondering if the best way to proceed here is to revert to:
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");
This was a change already commented here:
https://lore.kernel.org/git/3f085795-79bd-4a56-9df8-659e32179925@xxxxxxxxx/
My worry was that this would paper over a bug as we shouldn't be calling
wait_for_pager() without setting up the pager successfully. How easy
would it be to fix the source of the problem?
Best Wishes
Phillip
