Subject: Re: [PATCH v3 4/4] add-patch: render hunks through the pager

From: Junio C Hamano <[email-masked]>
Date: Wed, 17 Jul 2024 13:03:54 -0700

[email-masked] writes:

>> -	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?

Yeah, it indeed is curious.  

The rewrite resolves another iffy point in the original---you are
not supposed to attempt a one-shot assignment to the environment
variable when you are running a shell function, as that is not
portable.  And the above rewrite is a common way to fix that.

But still, yes, it is curious why the original segfaults.  Is there
some race there and having a subshell shifts the timing, or
something?

> 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?

;-)  Nice to see people trying to do the right thing.

Thanks.