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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 17, 2024, 20:03 UTC
Message-ID: <xmqqfrs723bp.fsf@gitster.g>
In-Reply-To: <88f9256e-04ba-4799-8048-406863054106@gmail.com>

phillip.wood123@gmail.com writes:

Show 10 quoted lines

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