Re: [PATCH v2] pager: fix crash when pager program doesn't exist

From: Enzo Matsumiya <ematsumiya@suse.de>
Date: Nov 22, 2021, 14:52 UTC
Message-ID: <20211122145222.m2zrmtbaeu5kzbtt@cyberdelia>
In-Reply-To: <YZseJ4jOVIK3+bUD@coredump.intra.peff.net>

On 11/21, Jeff King wrote:

Show 6 quoted lines

>On Sun, Nov 21, 2021 at 06:10:08PM -0800, Junio C Hamano wrote:
>> Well, not to me X-<.  This is way too aggressive a change to be made
>> lightly without auditing the current users of run_command API.
>
>Yikes. Thanks for a dose of sanity. I was looking too much at just the
>pager tests.

Thanks for your input, Junio. I was, too, focused only on pager side and even more so on the bug itself.

<snip> as I have no familiarity with other parts of the code </snip>

>> When merged to 'seen', this literally destroys tons of tests (the
>> first and easiest one to observe may be t0002).
>
>Forget 'seen'. Applying it on master shows plenty of breakages. :)

I should *probably* run the full test suite next time...

Show 5 quoted lines

>I think we should probably punt on this direction, and just make sure
>that setup_pager() either reinitializes the child_process as appropriate
>(as in the patch I showed in the earlier thread) or just refuses to try
>running the pager twice (I didn't show a patch, but it should just be a
>matter of setting a static flag).

I'm preparing v3 with the above suggestions in mind.

Cheers,

Enzo