From: Enzo Matsumiya Date: Mon, 22 Nov 2021 14:52:22 GMT Subject: Re: [PATCH v2] pager: fix crash when pager program doesn't exist Message-ID: <20211122145222.m2zrmtbaeu5kzbtt@cyberdelia> In-Reply-To: On 11/21, Jeff King wrote: >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. as I have no familiarity with other parts of the code >> 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... >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