Ævar Arnfjörð Bjarmason wrote: > > On Wed, Apr 05 2023, Jeff King wrote: > > > On Wed, Apr 05, 2023 at 09:18:20PM -0500, Felipe Contreras wrote: > > > >> On Wed, Apr 5, 2023 at 2:09 PM Jeff King <peff@xxxxxxxx> wrote: > >> > On Wed, Apr 05, 2023 at 07:32:22PM +0200, Patrick Steinhardt wrote: > >> > >> > IMHO we should aim for fixing those inconsistencies, and then letting > >> > people set PERL_PATH as appropriate (even to something that will find it > >> > via $PATH if they want to). > >> > >> We can aim to fix all those inconsistencies *eventually* while in the > >> meantime make them runnable for most people *today*. > >> > >> It's not a dichotomy. > > > > It is if the proposed patches change the behavior in such a way as to > > make things less consistent. > > > > There are three plausible perls to run (whether intentionally or > > accidentally): > > > > 1. the one in PERL_PATH > > > > 2. /usr/bin/perl > > > > 3. the first one in $PATH > > > > What the code tries to do now is to consistently use (1). If there are > > cases that accidentally use (2), which is what I took Patrick's patch to > > mean, then that is a problem for people who set PERL_PATH to something > > else, but not for people who leave it as /usr/bin/perl. If we "fix" > > those cases by switching them to (3), then now things are less > > consistent for such people than when we started. > > > > But I am not clear on what those cases are (if any), and we have not > > seen Patrick's follow-up proposed patch. > > > > I did find one case that is accidentally doing (3), and posted a patch > > elsewhere in the thread to convert it to (1). If you prefer behavior > > (3), you might consider that a regression, but it seems meaningless > > given the 99% of other cases that are using (1). If you want (3) to be > > the behavior everywhere, then we'd need to completely change our stance > > on how we invoke perl, or we need to teach PERL_PATH to handle this case > > so that people building Git can choose their own preference (sadly I > > don't think "make PERL_PATH='/usr/bin/env perl'" quite works because we > > have to shell-quote it in some contexts before evaluating). > > I just want to chime in to say that I've read this whole > thread-at-large, and I think what you're pointing out here is correct, > and that we should keep hardcoding "#!/usr/bin/perl", and then just have > "PERL_PATH" set. > > I.e. most of Patrick's original patch is unnecessary, as we either use > "$PERL_PATH" in the Makefile already, or munge the shebang when we > install. > > Then the only change we should need is the one you suggested in > <20230406032647.GA2092142@xxxxxxxxxxxxxxxxxxxxxxx> in the side-thread. That is true, however, the fact that something isn't *necessary* doesn't mean it isn't good. > Using "env" liket his is also incorrect. No, it's not. > I might have a "perl" in my "$PATH" which I expect to use for e.g. by > .bashrc, but I don't want that perl to take priority over "$PERL_PATH" for > git when I run some test script. And you can keep doing that as Patrick's patch does not change that priority. I applied both Patrick's patch and Jeff's patch, and then did: ln -s /bin/false ~/bin/perl Guess what... Everything works fine (as it should). $PEARL_PATH should override the shebangs (and it does), so it does not matter what `/usr/bin/env perl` returns... unless somebody does run the offending scripts manually, in which case they are on their own. So, I think Patrick's patch should still be applied, but in practice makes no difference, because Jeff's patch fixes the actual problems. Or another way to put it: Patrick's patch is unnecessary, but in my opinion still desirable. Cheers. -- Felipe Contreras
