| Message ID | d9cfad7caf9ff5bf88eb06cf7bb3be5e70e6d96f.1680689378.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | global: resolve Perl executable via PATH | expand |
On Wed, Apr 5, 2023 at 5:53 AM Patrick Steinhardt <ps@pks.im> wrote: > > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > shebang. This is not a portable location for the Perl interpreter and > may thus break on some systems that have the interpreter installed in a > different location. One such example is NixOS, where the only executable > installed in `/usr/bin` is env(1). > > Convert the shebangs to resolve the location of the Perl interpreter via > env(1) to make these scripts more portable. While the location of env(1) > is not guaranteed by any standard either, in practice all distributions > including NixOS have it available at `/usr/bin/env`. We're also already > using this idiom in a small set of other scripts, and until now nobody > complained about them. This is standard practice in Ruby, and it does seem to work everywhere. However, I wonder if /bin/env does also work. I can't imagine a system system providing /usr/bin/env but not /bin/env.
Patrick Steinhardt wrote: > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > shebang. This is not a portable location for the Perl interpreter and > may thus break on some systems that have the interpreter installed in a > different location. One such example is NixOS, where the only executable > installed in `/usr/bin` is env(1). Is there a reason to not set PERL_PATH, which is the documented method to handle this? From the Makefike: # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
On Wed, Apr 05, 2023 at 08:35:50AM -0500, Felipe Contreras wrote: > On Wed, Apr 5, 2023 at 5:53 AM Patrick Steinhardt <ps@pks.im> wrote: > > > > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > > shebang. This is not a portable location for the Perl interpreter and > > may thus break on some systems that have the interpreter installed in a > > different location. One such example is NixOS, where the only executable > > installed in `/usr/bin` is env(1). > > > > Convert the shebangs to resolve the location of the Perl interpreter via > > env(1) to make these scripts more portable. While the location of env(1) > > is not guaranteed by any standard either, in practice all distributions > > including NixOS have it available at `/usr/bin/env`. We're also already > > using this idiom in a small set of other scripts, and until now nobody > > complained about them. > > This is standard practice in Ruby, and it does seem to work everywhere. > > However, I wonder if /bin/env does also work. I can't imagine a system > system providing /usr/bin/env but not /bin/env. NixOS does indeed only have /usr/bin/env and does not have /bin/env, so it wouldn't. Patrick
On Wed, Apr 05, 2023 at 10:42:52AM -0400, Todd Zullinger wrote: > Patrick Steinhardt wrote: > > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > > shebang. This is not a portable location for the Perl interpreter and > > may thus break on some systems that have the interpreter installed in a > > different location. One such example is NixOS, where the only executable > > installed in `/usr/bin` is env(1). > > Is there a reason to not set PERL_PATH, which is the > documented method to handle this? From the Makefike: > > # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). Setting PERL_PATH helps with a subset of invocations where the Makefile either executes Perl directly or where it writes the shebang itself. But the majority of scripts I'm touching have `#!/usr/bin/perl` as shebang, and that path is not adjusted by setting PERL_PATH. I'd be happy to amend the patch series to only fix up shebangs which would not be helped by setting PERL_PATH. But if we can make it work without having to set PERL_PATH at all I don't quite see the point. Patrick
Patrick Steinhardt wrote: > On Wed, Apr 05, 2023 at 10:42:52AM -0400, Todd Zullinger wrote: >> Is there a reason to not set PERL_PATH, which is the >> documented method to handle this? From the Makefike: >> >> # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). > > Setting PERL_PATH helps with a subset of invocations where the Makefile > either executes Perl directly or where it writes the shebang itself. But > the majority of scripts I'm touching have `#!/usr/bin/perl` as shebang, > and that path is not adjusted by setting PERL_PATH. Ahh. I wonder if that's intentional? I haven't dug into the history, so I'm not sure. It seems like an oversight, as an initial reaction. > I'd be happy to amend the patch series to only fix up shebangs which > would not be helped by setting PERL_PATH. But if we can make it work > without having to set PERL_PATH at all I don't quite see the point. It's certainly debatable whether using /path/to/env perl is better than hard-coding it at build time (forgetting about the usage of RUNTIME_PREFIX). [Debatable in a friendly sense, of course.] As a distribution packager, I prefer to set the path at build time to help ensure that an end user can't easily break things by installing a different perl in PATH. The Fedora build system will munge /path/to/env perl shebangs to /usr/bin/perl and it won't effect us much. That may not be true for other distributions and they may care more if they want to keep using a hard-coded path to perl. I don't know how it may affects the Windows folks either, who are further askew from our other, more UNIX-like targets. I don't know what the right choice is for upstream Git, it can easily be argued in either direction. :) Cheers,
On Wed, Apr 05, 2023 at 04:52:40PM +0200, Patrick Steinhardt wrote: > > Is there a reason to not set PERL_PATH, which is the > > documented method to handle this? From the Makefike: > > > > # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). > > Setting PERL_PATH helps with a subset of invocations where the Makefile > either executes Perl directly or where it writes the shebang itself. But > the majority of scripts I'm touching have `#!/usr/bin/perl` as shebang, > and that path is not adjusted by setting PERL_PATH. Which scripts? If I do: mkdir /tmp/foo ln -s /usr/bin/perl /tmp/foo/my-perl make PERL_PATH=/tmp/foo/my-perl prefix=/tmp/foo install head -n 1 /tmp/foo/bin/git-cvsserver Then I see: #!/tmp/foo/my-perl And that is due to this segment in the Makefile: $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE $(QUIET_GEN) \ sed -e '1{' \ -e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \ -e ' r GIT-PERL-HEADER' \ -e ' G' \ -e '}' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ $< >$@+ && \ chmod +x $@+ && \ mv $@+ $@ And that behavior goes all the way back to bc6146d2abc ('build' scripts before installing., 2005-09-08). If there are some perl scripts we are "building" outside of this rule, then that is probably a bug. The only thing I found via: find /tmp/foo -type | xargs grep /usr/bin/perl was a sample hook (which is probably a bug; we do munge the hook scripts to replace @PERL_PATH@, etc, but I think the Makefile never learned that the template hook scripts might be something other than shell scripts). -Peff
On Wed, Apr 5, 2023 at 11:31 AM Todd Zullinger <tmz@pobox.com> wrote: > > Patrick Steinhardt wrote: > > On Wed, Apr 05, 2023 at 10:42:52AM -0400, Todd Zullinger wrote: > >> Is there a reason to not set PERL_PATH, which is the > >> documented method to handle this? From the Makefike: > >> > >> # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). > > > > Setting PERL_PATH helps with a subset of invocations where the Makefile > > either executes Perl directly or where it writes the shebang itself. But > > the majority of scripts I'm touching have `#!/usr/bin/perl` as shebang, > > and that path is not adjusted by setting PERL_PATH. > > Ahh. I wonder if that's intentional? I haven't dug into > the history, so I'm not sure. It seems like an oversight, > as an initial reaction. Generally the people interested in setting PERL_PATH are packagers, and it's so the installed scripts have the desired hardcoded path. So a script that is never installed isn't considered. > > I'd be happy to amend the patch series to only fix up shebangs which > > would not be helped by setting PERL_PATH. But if we can make it work > > without having to set PERL_PATH at all I don't quite see the point. > > It's certainly debatable whether using /path/to/env perl is > better than hard-coding it at build time (forgetting about > the usage of RUNTIME_PREFIX). [Debatable in a friendly > sense, of course.] It's better because it allows the user to choose another version dynamically, for example: PATH=/opt/perl7/bin:$PATH script > As a distribution packager, I prefer to set the path at > build time to help ensure that an end user can't easily > break things by installing a different perl in PATH. And as a user I would rather packagers don't treat me like a child. I decide how to use *my* system. And BTW, newer generations of developers use all kinds of version managers like RVM and nvm, and perl has one as well: perlbrew [1]. Not to mention docker, for which the Perl official image installs into /usr/local/bin/perl. > The Fedora build system will munge /path/to/env perl > shebangs to /usr/bin/perl and it won't effect us much. So you can override '#!/usr/bin/env perl' instead of overriding `#!/usr/bin/perl`, which the Git Makefile does by default anyway. What's the difference to you? > That may not be true for other distributions and they may > care more if they want to keep using a hard-coded path to > perl. Arch Linux does whatever upstream does by default. > I don't know how it may affects the Windows folks either, who are > further askew from our other, more UNIX-like targets. Windows doesn't use shebangs, you have to do `perl script`. > I don't know what the right choice is for upstream Git, it > can easily be argued in either direction. :) I don't think it matters to packagers how people developing Git run the internal scripts. Cheers. [1] https://metacpan.org/pod/perlbrew
On Wed, Apr 05, 2023 at 12:54:14PM -0400, Jeff King wrote: > On Wed, Apr 05, 2023 at 04:52:40PM +0200, Patrick Steinhardt wrote: > > > > Is there a reason to not set PERL_PATH, which is the > > > documented method to handle this? From the Makefike: > > > > > > # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). > > > > Setting PERL_PATH helps with a subset of invocations where the Makefile > > either executes Perl directly or where it writes the shebang itself. But > > the majority of scripts I'm touching have `#!/usr/bin/perl` as shebang, > > and that path is not adjusted by setting PERL_PATH. > > Which scripts? If I do: > > mkdir /tmp/foo > ln -s /usr/bin/perl /tmp/foo/my-perl > make PERL_PATH=/tmp/foo/my-perl prefix=/tmp/foo install > > head -n 1 /tmp/foo/bin/git-cvsserver > > Then I see: > > #!/tmp/foo/my-perl > > And that is due to this segment in the Makefile: > > $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE > $(QUIET_GEN) \ > sed -e '1{' \ > -e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \ > -e ' r GIT-PERL-HEADER' \ > -e ' G' \ > -e '}' \ > -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ > $< >$@+ && \ > chmod +x $@+ && \ > mv $@+ $@ > > And that behavior goes all the way back to bc6146d2abc ('build' scripts > before installing., 2005-09-08). If there are some perl scripts we are > "building" outside of this rule, then that is probably a bug. > > The only thing I found via: > > find /tmp/foo -type | xargs grep /usr/bin/perl > > was a sample hook (which is probably a bug; we do munge the hook scripts > to replace @PERL_PATH@, etc, but I think the Makefile never learned that > the template hook scripts might be something other than shell scripts). Yeah, agreed, the scripts we install are fine from all I can tell. I should've clarified, but what I care about is our build infra as well as our test scripts. That's neither clear from the commit description nor from the changes that I'm doing. I'd be happy to keep the current state of installed scripts as-is and resend another iteration of this patch that only addresses shebangs used in internal scripts. Patrick
On Wed, Apr 05, 2023 at 11:54:49AM -0400, Todd Zullinger wrote: > Patrick Steinhardt wrote: > > On Wed, Apr 05, 2023 at 10:42:52AM -0400, Todd Zullinger wrote: > >> Is there a reason to not set PERL_PATH, which is the > >> documented method to handle this? From the Makefike: > >> > >> # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). > > > > Setting PERL_PATH helps with a subset of invocations where the Makefile > > either executes Perl directly or where it writes the shebang itself. But > > the majority of scripts I'm touching have `#!/usr/bin/perl` as shebang, > > and that path is not adjusted by setting PERL_PATH. > > Ahh. I wonder if that's intentional? I haven't dug into > the history, so I'm not sure. It seems like an oversight, > as an initial reaction. > > > I'd be happy to amend the patch series to only fix up shebangs which > > would not be helped by setting PERL_PATH. But if we can make it work > > without having to set PERL_PATH at all I don't quite see the point. > > It's certainly debatable whether using /path/to/env perl is > better than hard-coding it at build time (forgetting about > the usage of RUNTIME_PREFIX). [Debatable in a friendly > sense, of course.] > > As a distribution packager, I prefer to set the path at > build time to help ensure that an end user can't easily > break things by installing a different perl in PATH. > > The Fedora build system will munge /path/to/env perl > shebangs to /usr/bin/perl and it won't effect us much. > > That may not be true for other distributions and they may > care more if they want to keep using a hard-coded path to > perl. I don't know how it may affects the Windows folks > either, who are further askew from our other, more UNIX-like > targets. > > I don't know what the right choice is for upstream Git, it > can easily be argued in either direction. :) I agree, there is no clearly-superior choice -- both have their merits. I'll probably send a v2 that only munges internal scripts that are used as part of our build and testing infrastructure. That's the area I care most about in this context anyway. Patrick
On Wed, Apr 05, 2023 at 07:32:22PM +0200, Patrick Steinhardt wrote: > Yeah, agreed, the scripts we install are fine from all I can tell. I > should've clarified, but what I care about is our build infra as well as > our test scripts. That's neither clear from the commit description nor > from the changes that I'm doing. Ah, OK, that makes more sense. > I'd be happy to keep the current state of installed scripts as-is and > resend another iteration of this patch that only addresses shebangs used > in internal scripts. We generally try to use $PERL_PATH even for building and testing by invoking "$PERL_PATH script.pl", and declaring a perl() wrapper within the test scripts. But I would not be surprised if there are cases where we fail to (and nobody noticed because it usually just works to find one at /usr/bin/perl). 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). -Peff
On Wed, Apr 5, 2023, at 12:10, Patrick Steinhardt wrote: > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > shebang. This is not a portable location for the Perl interpreter and > may thus break on some systems that have the interpreter installed in a > different location. One such example is NixOS, where the only executable > installed in `/usr/bin` is env(1). > > Convert the shebangs to resolve the location of the Perl interpreter via > env(1) to make these scripts more portable. While the location of env(1) > is not guaranteed by any standard either, in practice all distributions > including NixOS have it available at `/usr/bin/env`. We're also already > using this idiom in a small set of other scripts, and until now nobody > complained about them. This is great. I love to see efforts towards portability, and particularly when it benefits NixOS (as well).
Patrick Steinhardt <ps@pks.im> writes: >> I don't know what the right choice is for upstream Git, it >> can easily be argued in either direction. :) > > I agree, there is no clearly-superior choice -- both have their merits. > I'll probably send a v2 that only munges internal scripts that are used > as part of our build and testing infrastructure. That's the area I care > most about in this context anyway. My preference is (1) not to touch scripts that are processed by Makefile to use $PERL_PATH, (2) fix callers of "./foo.pl" to invoke "$PERL_PATH ./foo.pl" where the perl () { command "$PERL_PATH" "$@" } wrapper is not avialable, and (3) fix them to use "perl foo.pl" where the wrapper is visible. That way, we can wean ourselves away from the assumption that perl interpreter should exist at /usr/bin/perl without introducing a new assumption that everybody's env should exist at /usr/bin/env. There may already be scripts that assume "#!/usr/bin/env foo" is acceptable, but fixing them would be outside the scope of this discussion, I would say. Thanks all for a good discussion.
Patrick Steinhardt <ps@pks.im> wrote: > diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl > index 755a110bc4..3fe43b8968 100755 > --- a/Documentation/cmd-list.perl > +++ b/Documentation/cmd-list.perl > @@ -1,5 +1,6 @@ > -#!/usr/bin/perl -w > +#!/usr/bin/env perl > > +use warnings; Fwiw, adding `use warnings' only affects the current scope (package main), whereas `-w' affects the entire Perl process. I prefer `-w' since adding `use warnings' everywhere is annoyingly verbose and I only use 3rd-party code that's warning-clean. In *.t test scripts and stuff I intend to be overwritten in install scripts; I've been using `#!perl -w' as the shebang as a clear signal that it should be overwritten on install or or run via `$(PERL) FOO' in a Makefile. For personal scripts in ~/bin, I've been going shebang-less and having the following as the first two lines: eval 'exec perl -w -S $0 ${1+"$@"}' if 0; # running under some shell (Only tested GNU/Linux and FreeBSD, though). This (and `env perl') will fail if distros someday decide to start using `perl5' as the executable name. That said, I don't know if anything I've said above is appropriate for the git project aside from noting the difference between `-w' and `use warnings'.
On Wed, Apr 5, 2023 at 4:50 PM Eric Wong <e@80x24.org> wrote: > (Only tested GNU/Linux and FreeBSD, though). This (and > `env perl') will fail if distros someday decide to start > using `perl5' as the executable name. When that happens you can just do `ln -s perl ~/bin/perl5`. This is similar to what I did with the horrendous python3 transition.
On Wed, Apr 5, 2023 at 2:09 PM Jeff King <peff@peff.net> 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.
On Wed, Apr 5, 2023 at 2:29 PM Junio C Hamano <gitster@pobox.com> wrote: > > Patrick Steinhardt <ps@pks.im> writes: > > >> I don't know what the right choice is for upstream Git, it > >> can easily be argued in either direction. :) > > > > I agree, there is no clearly-superior choice -- both have their merits. > > I'll probably send a v2 that only munges internal scripts that are used > > as part of our build and testing infrastructure. That's the area I care > > most about in this context anyway. > > My preference is > > (1) not to touch scripts that are processed by Makefile to use > $PERL_PATH, > > (2) fix callers of "./foo.pl" to invoke "$PERL_PATH ./foo.pl" where > the perl () { command "$PERL_PATH" "$@" } wrapper is not > avialable, and > > (3) fix them to use "perl foo.pl" where the wrapper is visible. That is orthogonal to the patch. All those steps can be done *eventually* while the proposed patch is applied *today*. > That way, we can wean ourselves away from the assumption that perl > interpreter should exist at /usr/bin/perl without introducing a new > assumption that everybody's env should exist at /usr/bin/env. The patch doesn't introduce such an assumption. Changing the shebang only affects scripts that are 1) not processed by the Makefile, and 2) not called as "${PERL_PATH-perl} foo.pl". If your system does not have /usr/bin/env and everything you cared about worked yesterday, it would still work with the patch applied today. Having a `#!/usr/bin/env perl` shebang is simply a good practice to write in all scripts. But that is just the *default*, nobody is being forced to actually use that shebang because 1) the Makefile is still going to override that and replace it with $PERL_PATH in generated scripts, and 2) the scripts that do "$PERL_PATH ./foo.pl" are essentially overriding it to, and so does 3). I believe this is a red herring (which might be desirable to fix some day). Cheers.
On Wed, Apr 05, 2023 at 12:10:10PM +0200, Patrick Steinhardt wrote: > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > shebang. This is not a portable location for the Perl interpreter and > may thus break on some systems that have the interpreter installed in a > different location. One such example is NixOS, where the only executable > installed in `/usr/bin` is env(1). > > Convert the shebangs to resolve the location of the Perl interpreter via > env(1) to make these scripts more portable. While the location of env(1) > is not guaranteed by any standard either, in practice all distributions > including NixOS have it available at `/usr/bin/env`. We're also already > using this idiom in a small set of other scripts, and until now nobody > complained about them. > > This makes the test suite pass on NixOS. Can you tell us more about which tests failed? Skimming over the list of files here, the first few examples: > Documentation/build-docdep.perl | 2 +- > Documentation/cat-texi.perl | 2 +- > Documentation/cmd-list.perl | 3 ++- > Documentation/fix-texi.perl | 4 +++- > Documentation/lint-fsck-msgids.perl | 2 +- will not be affected by your patch, because we never use their shebang lines at all (we say "$PERL_PATH cmd-list.perl" in the Makefile). I did try removing /usr/bin/perl completely (and thus likewise had no perl in my path), setting PERL_PATH, and got a few broken tests, which could be fixed as below. Does this fix the cases you saw, or are there others? -- >8 -- Subject: [PATCH] t/lib-httpd: pass PERL_PATH to CGI scripts As discussed in t/README, tests should aim to use PERL_PATH rather than straight "perl". We usually do this automatically with a "perl" function in test-lib.sh, but a few cases need to be handled specially. One such case is the apply-one-time-perl.sh CGI, which invokes plain "perl". It should be using $PERL_PATH, but to make that work, we must also instruct Apache to pass through the variable. Prior to this patch, doing: mv /usr/bin/perl /usr/bin/my-perl make PERL_PATH=/usr/bin/my-perl test would fail t5702, t5703, and t5616. After this it passes. This is a pretty extreme case, as even if you install perl elsewhere, you'd likely still have it in your $PATH. A more realistic case is that you don't want to use the perl in your $PATH (because it's older, broken, etc) and expect PERL_PATH to consistently override that (since that's what it's documented to do). Removing it completely is just a convenient way of completely breaking it for testing purposes. Signed-off-by: Jeff King <peff@peff.net> --- t/lib-httpd/apache.conf | 2 ++ t/lib-httpd/apply-one-time-perl.sh | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index f43a25c1f10..9e6892970de 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -101,6 +101,8 @@ PassEnv LC_ALL Alias /dumb/ www/ Alias /auth/dumb/ www/auth/dumb/ +SetEnv PERL_PATH ${PERL_PATH} + <LocationMatch /smart/> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL diff --git a/t/lib-httpd/apply-one-time-perl.sh b/t/lib-httpd/apply-one-time-perl.sh index 09a0abdff7c..d7f9fed6aee 100644 --- a/t/lib-httpd/apply-one-time-perl.sh +++ b/t/lib-httpd/apply-one-time-perl.sh @@ -13,7 +13,7 @@ then export LC_ALL "$GIT_EXEC_PATH/git-http-backend" >out - perl -pe "$(cat one-time-perl)" out >out_modified + "$PERL_PATH" -pe "$(cat one-time-perl)" out >out_modified if cmp -s out out_modified then
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@peff.net> 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). -Peff
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@peff.net> 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@coredump.intra.peff.net> in the side-thread. Using "env" liket his is also incorrect. 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. I also wonder if something like this (untested) wouldn't be useful to provide an earlier warning of this, instead of failing when we fail to invoke the relevant scripts. diff --git a/Makefile b/Makefile index 60ab1a8b4f4..9abc2e52cfa 100644 --- a/Makefile +++ b/Makefile @@ -910,6 +910,15 @@ ifndef PYTHON_PATH PYTHON_PATH = /usr/bin/python endif +define check-path-exists +ifeq ($$(wildcard $$($(1))),) +$$(error $(1) set to '$(2)', which does not exist) +endif +endef + +$(eval $(call check-path-exists,SHELL_PATH,$(SHELL_PATH))) +$(eval $(call check-path-exists,PERL_PATH,$(PERL_PATH))) + export PERL_PATH export PYTHON_PATH That should not break anything in principle, as we already rely on these to build git itself, but note that that's not the case with "PYTHON_PATH". In the case of "PERL_PATH" I think that's limited to building the docs, so if we did something like this perhaps it should be in shared.mak, and that check should be in Documentation/Makefile. But perhaps it's all reduntant to just running into an error when we try to generate-cmdlist.sh or whatever...
On Wed, Apr 05 2023, Eric Wong wrote: > Patrick Steinhardt <ps@pks.im> wrote: >> diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl >> index 755a110bc4..3fe43b8968 100755 >> --- a/Documentation/cmd-list.perl >> +++ b/Documentation/cmd-list.perl >> @@ -1,5 +1,6 @@ >> -#!/usr/bin/perl -w >> +#!/usr/bin/env perl >> >> +use warnings; > > Fwiw, adding `use warnings' only affects the current scope > (package main), whereas `-w' affects the entire Perl process. > > I prefer `-w' since adding `use warnings' everywhere is > annoyingly verbose and I only use 3rd-party code that's > warning-clean. I much prefer "use warnings", and it's what we should be using. Note that "-w" isn't just "global warnings", but for anything non-trivial you'll likely be tripped up by "-w" being dynamically scoped, and "use warnings" being lexically scoped. See "What's wrong with -w and $^W" in "perldoc warnings". In any case, I think it's fine to change these to "use warnings", and the behavior won't change, as the scripts are trivial, and the either don't use any modules, or use perl core modules that are (presumably) warning-free. But I think that change really should be split up from any proposed shebang change, as it's just being made here because by usin g"env" in particular we can't pass the "-w" argument...
On Wed, Apr 05, 2023 at 02:15:05PM -0400, Jeff King wrote: > On Wed, Apr 05, 2023 at 07:32:22PM +0200, Patrick Steinhardt wrote: > > > Yeah, agreed, the scripts we install are fine from all I can tell. I > > should've clarified, but what I care about is our build infra as well as > > our test scripts. That's neither clear from the commit description nor > > from the changes that I'm doing. > > Ah, OK, that makes more sense. > > > I'd be happy to keep the current state of installed scripts as-is and > > resend another iteration of this patch that only addresses shebangs used > > in internal scripts. > > We generally try to use $PERL_PATH even for building and testing by > invoking "$PERL_PATH script.pl", and declaring a perl() wrapper within > the test scripts. But I would not be surprised if there are cases where > we fail to (and nobody noticed because it usually just works to find one > at /usr/bin/perl). > > 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). > > -Peff Makes sense to me, I'll send a v2 that goes into this direction. Thanks all for your input! Patrick
On Wed, Apr 05, 2023 at 11:26:47PM -0400, Jeff King wrote: > On Wed, Apr 05, 2023 at 12:10:10PM +0200, Patrick Steinhardt wrote: > > > The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` > > shebang. This is not a portable location for the Perl interpreter and > > may thus break on some systems that have the interpreter installed in a > > different location. One such example is NixOS, where the only executable > > installed in `/usr/bin` is env(1). > > > > Convert the shebangs to resolve the location of the Perl interpreter via > > env(1) to make these scripts more portable. While the location of env(1) > > is not guaranteed by any standard either, in practice all distributions > > including NixOS have it available at `/usr/bin/env`. We're also already > > using this idiom in a small set of other scripts, and until now nobody > > complained about them. > > > > This makes the test suite pass on NixOS. > > Can you tell us more about which tests failed? > > Skimming over the list of files here, the first few examples: > > > Documentation/build-docdep.perl | 2 +- > > Documentation/cat-texi.perl | 2 +- > > Documentation/cmd-list.perl | 3 ++- > > Documentation/fix-texi.perl | 4 +++- > > Documentation/lint-fsck-msgids.perl | 2 +- > > will not be affected by your patch, because we never use their shebang > lines at all (we say "$PERL_PATH cmd-list.perl" in the Makefile). > > I did try removing /usr/bin/perl completely (and thus likewise had no > perl in my path), setting PERL_PATH, and got a few broken tests, which > could be fixed as below. > > Does this fix the cases you saw, or are there others? You know, let's just go with your patch. With PERL_PATH set it fixes all the issues I have observed. At some point in time I saw more issues than the one you fix here, but that's because my `config.mak` got lost without me noticing. Oops, embarassing. Thanks! Patrick > -- >8 -- > Subject: [PATCH] t/lib-httpd: pass PERL_PATH to CGI scripts > > As discussed in t/README, tests should aim to use PERL_PATH rather than > straight "perl". We usually do this automatically with a "perl" function > in test-lib.sh, but a few cases need to be handled specially. > > One such case is the apply-one-time-perl.sh CGI, which invokes plain > "perl". It should be using $PERL_PATH, but to make that work, we must > also instruct Apache to pass through the variable. > > Prior to this patch, doing: > > mv /usr/bin/perl /usr/bin/my-perl > make PERL_PATH=/usr/bin/my-perl test > > would fail t5702, t5703, and t5616. After this it passes. This is a > pretty extreme case, as even if you install perl elsewhere, you'd likely > still have it in your $PATH. A more realistic case is that you don't > want to use the perl in your $PATH (because it's older, broken, etc) and > expect PERL_PATH to consistently override that (since that's what it's > documented to do). Removing it completely is just a convenient way of > completely breaking it for testing purposes. > > Signed-off-by: Jeff King <peff@peff.net> > --- > t/lib-httpd/apache.conf | 2 ++ > t/lib-httpd/apply-one-time-perl.sh | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf > index f43a25c1f10..9e6892970de 100644 > --- a/t/lib-httpd/apache.conf > +++ b/t/lib-httpd/apache.conf > @@ -101,6 +101,8 @@ PassEnv LC_ALL > Alias /dumb/ www/ > Alias /auth/dumb/ www/auth/dumb/ > > +SetEnv PERL_PATH ${PERL_PATH} > + > <LocationMatch /smart/> > SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} > SetEnv GIT_HTTP_EXPORT_ALL > diff --git a/t/lib-httpd/apply-one-time-perl.sh b/t/lib-httpd/apply-one-time-perl.sh > index 09a0abdff7c..d7f9fed6aee 100644 > --- a/t/lib-httpd/apply-one-time-perl.sh > +++ b/t/lib-httpd/apply-one-time-perl.sh > @@ -13,7 +13,7 @@ then > export LC_ALL > > "$GIT_EXEC_PATH/git-http-backend" >out > - perl -pe "$(cat one-time-perl)" out >out_modified > + "$PERL_PATH" -pe "$(cat one-time-perl)" out >out_modified > > if cmp -s out out_modified > then > -- > 2.40.0.824.g7b678b1f643 >
Æ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@peff.net> 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@coredump.intra.peff.net> 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.
diff --git a/Documentation/build-docdep.perl b/Documentation/build-docdep.perl index 1b3ac8fdd9..fe6a1e724d 100755 --- a/Documentation/build-docdep.perl +++ b/Documentation/build-docdep.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl my %include = (); my %included = (); diff --git a/Documentation/cat-texi.perl b/Documentation/cat-texi.perl index 14d2f83415..5b44d64f7d 100755 --- a/Documentation/cat-texi.perl +++ b/Documentation/cat-texi.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl use strict; use warnings; diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl index 755a110bc4..3fe43b8968 100755 --- a/Documentation/cmd-list.perl +++ b/Documentation/cmd-list.perl @@ -1,5 +1,6 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl +use warnings; use File::Compare qw(compare); sub format_one { diff --git a/Documentation/fix-texi.perl b/Documentation/fix-texi.perl index ff7d78f620..61287a0a9e 100755 --- a/Documentation/fix-texi.perl +++ b/Documentation/fix-texi.perl @@ -1,4 +1,6 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl + +use warnings; while (<>) { if (/^\@setfilename/) { diff --git a/Documentation/lint-fsck-msgids.perl b/Documentation/lint-fsck-msgids.perl index 1233ffe815..fd2209e4f8 100755 --- a/Documentation/lint-fsck-msgids.perl +++ b/Documentation/lint-fsck-msgids.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl my ($fsck_h, $fsck_msgids_txt, $okfile) = @ARGV; diff --git a/Documentation/lint-gitlink.perl b/Documentation/lint-gitlink.perl index 1c61dd9512..b27418ee16 100755 --- a/Documentation/lint-gitlink.perl +++ b/Documentation/lint-gitlink.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/Documentation/lint-man-end-blurb.perl b/Documentation/lint-man-end-blurb.perl index 6bdb13ad9f..d1ee7ba130 100755 --- a/Documentation/lint-man-end-blurb.perl +++ b/Documentation/lint-man-end-blurb.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/Documentation/lint-man-section-order.perl b/Documentation/lint-man-section-order.perl index 02408a0062..27921ef66f 100755 --- a/Documentation/lint-man-section-order.perl +++ b/Documentation/lint-man-section-order.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl index 3bd824154b..74be842ebd 100755 --- a/compat/vcbuild/scripts/clink.pl +++ b/compat/vcbuild/scripts/clink.pl @@ -1,4 +1,7 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl + +use warnings; + ###################################################################### # Compiles or links files # diff --git a/compat/vcbuild/scripts/lib.pl b/compat/vcbuild/scripts/lib.pl index d8054e469f..6f2c2cb89f 100755 --- a/compat/vcbuild/scripts/lib.pl +++ b/compat/vcbuild/scripts/lib.pl @@ -1,4 +1,7 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl + +use warnings; + ###################################################################### # Libifies files on Windows # diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 2f6e0197ff..43d64909cb 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -848,7 +848,7 @@ string(REPLACE "@@INSTLIBDIR@@" "${INSTLIBDIR}" perl_header "${perl_header}") foreach(script ${git_perl_scripts}) file(STRINGS ${CMAKE_SOURCE_DIR}/${script}.perl content NEWLINE_CONSUME) - string(REPLACE "#!/usr/bin/perl" "#!/usr/bin/perl\n${perl_header}\n" content "${content}") + string(REPLACE "#!/usr/bin/env perl" "#!/usr/bin/env perl\n${perl_header}\n" content "${content}") string(REPLACE "@@GIT_VERSION@@" "${PROJECT_VERSION}" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/${script} ${content}) endforeach() diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl index ed6c45988a..ea32de593c 100755 --- a/contrib/buildsystems/engine.pl +++ b/contrib/buildsystems/engine.pl @@ -1,4 +1,7 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl + +use warnings; + ###################################################################### # Do not call this script directly! # diff --git a/contrib/buildsystems/generate b/contrib/buildsystems/generate index bc10f25ff2..41c1967496 100755 --- a/contrib/buildsystems/generate +++ b/contrib/buildsystems/generate @@ -1,4 +1,7 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl + +use warnings; + ###################################################################### # Generate buildsystem files # diff --git a/contrib/buildsystems/parse.pl b/contrib/buildsystems/parse.pl index c9656ece99..03050ab28d 100755 --- a/contrib/buildsystems/parse.pl +++ b/contrib/buildsystems/parse.pl @@ -1,4 +1,7 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl + +use warnings; + ###################################################################### # Do not call this script directly! # diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts index 85ad732fc0..a9ce8edb85 100755 --- a/contrib/contacts/git-contacts +++ b/contrib/contacts/git-contacts @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # List people who might be interested in a patch. Useful as the argument to # git-send-email --cc-cmd option, and in other situations. diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl index 9fb998ae09..514f68d00b 100755 --- a/contrib/credential/netrc/git-credential-netrc.perl +++ b/contrib/credential/netrc/git-credential-netrc.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl index c0fb3718b2..4387379d52 100755 --- a/contrib/credential/netrc/test.pl +++ b/contrib/credential/netrc/test.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use warnings; use strict; diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile index f2be7cc924..9da2a0b442 100644 --- a/contrib/diff-highlight/Makefile +++ b/contrib/diff-highlight/Makefile @@ -1,6 +1,6 @@ all: diff-highlight -PERL_PATH = /usr/bin/perl +PERL_PATH = /usr/bin/env perl -include ../../config.mak PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH)) diff --git a/contrib/fast-import/git-import.perl b/contrib/fast-import/git-import.perl index 0891b9e366..440790523d 100755 --- a/contrib/fast-import/git-import.perl +++ b/contrib/fast-import/git-import.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Performs an initial import of a directory. This is the equivalent # of doing 'git init; git add .; git commit'. It's a little slower, diff --git a/contrib/fast-import/import-directories.perl b/contrib/fast-import/import-directories.perl index a16f79cfdc..5ea90dada9 100755 --- a/contrib/fast-import/import-directories.perl +++ b/contrib/fast-import/import-directories.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Copyright 2008-2009 Peter Krefting <peter@softwolves.pp.se> # diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl index d50ce26d5d..d28c7b1a4a 100755 --- a/contrib/fast-import/import-tars.perl +++ b/contrib/fast-import/import-tars.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl ## tar archive frontend for git-fast-import ## diff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl index 2770a1b1d2..18444e0157 100755 --- a/contrib/hooks/setgitperms.perl +++ b/contrib/hooks/setgitperms.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Copyright (c) 2006 Josh England # diff --git a/contrib/hooks/update-paranoid b/contrib/hooks/update-paranoid index 0092d67b8a..a85d3badd1 100755 --- a/contrib/hooks/update-paranoid +++ b/contrib/hooks/update-paranoid @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use File::Spec; diff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl index a677569ddd..0ca49e370b 100755 --- a/contrib/long-running-filter/example.pl +++ b/contrib/long-running-filter/example.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Example implementation for the Git filter protocol version 2 # See Documentation/gitattributes.txt, section "Filter Protocol" diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl index eb52a53d32..cb0cd63ea7 100755 --- a/contrib/mw-to-git/git-mw.perl +++ b/contrib/mw-to-git/git-mw.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # Copyright (C) 2013 # Benoit Person <benoit.person@ensimag.imag.fr> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl index a5624413dc..9ae596f3b7 100755 --- a/contrib/mw-to-git/git-remote-mediawiki.perl +++ b/contrib/mw-to-git/git-remote-mediawiki.perl @@ -1,4 +1,4 @@ -#! /usr/bin/perl +#!/usr/bin/env perl # Copyright (C) 2011 # Jérémie Nikaes <jeremie.nikaes@ensimag.imag.fr> diff --git a/contrib/mw-to-git/t/test-gitmw.pl b/contrib/mw-to-git/t/test-gitmw.pl index c5d687f078..9005837713 100755 --- a/contrib/mw-to-git/t/test-gitmw.pl +++ b/contrib/mw-to-git/t/test-gitmw.pl @@ -1,4 +1,8 @@ -#!/usr/bin/perl -w -s +#!/usr/bin/env perl + +use strict; +use warnings; + # Copyright (C) 2012 # Charles Roussel <charles.roussel@ensimag.imag.fr> # Simon Cathebras <simon.cathebras@ensimag.imag.fr> diff --git a/contrib/stats/mailmap.pl b/contrib/stats/mailmap.pl index 9513f5e35b..469af8240c 100755 --- a/contrib/stats/mailmap.pl +++ b/contrib/stats/mailmap.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use warnings 'all'; use strict; diff --git a/contrib/stats/packinfo.pl b/contrib/stats/packinfo.pl index be188c0f11..51823ac942 100755 --- a/contrib/stats/packinfo.pl +++ b/contrib/stats/packinfo.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # This tool will print vaguely pretty information about a pack. It # expects the output of "git verify-pack -v" as input on stdin. diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile index 093399c788..b22cd62236 100644 --- a/contrib/subtree/t/Makefile +++ b/contrib/subtree/t/Makefile @@ -8,7 +8,7 @@ #GIT_TEST_OPTS=--verbose --debug SHELL_PATH ?= $(SHELL) -PERL_PATH ?= /usr/bin/perl +PERL_PATH ?= /usr/bin/env perl TAR ?= $(TAR) RM ?= rm -f PROVE ?= prove diff --git a/git-archimport.perl b/git-archimport.perl index b7c173c345..3f46e0c1dc 100755 --- a/git-archimport.perl +++ b/git-archimport.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # This tool is copyright (c) 2005, Martin Langhoff. # It is released under the Gnu Public License, version 2. diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index 289d4bc684..c674c22e4f 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use 5.008; use strict; diff --git a/git-cvsimport.perl b/git-cvsimport.perl index 7bf3c12d67..56011991d6 100755 --- a/git-cvsimport.perl +++ b/git-cvsimport.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # This tool is copyright (c) 2005, Matthias Urlichs. # It is released under the Gnu Public License, version 2. diff --git a/git-cvsserver.perl b/git-cvsserver.perl index 7b757360e2..9388efddf2 100755 --- a/git-cvsserver.perl +++ b/git-cvsserver.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl #### #### This application is a CVS emulation layer for git. diff --git a/git-send-email.perl b/git-send-email.perl index 07f2a0cbea..d532c08439 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com> # Copyright 2005 Ryan Anderson <ryan@michonline.com> diff --git a/git-svn.perl b/git-svn.perl index be987e316f..1ba835613b 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net> # License: GPL v2 or later use 5.008; diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index e66eb3d9ba..943099a024 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # gitweb - simple web interface to track changes in git repositories # diff --git a/t/Git-SVN/Utils/can_compress.t b/t/Git-SVN/Utils/can_compress.t index d7b49b8d54..0cdedb25bf 100755 --- a/t/Git-SVN/Utils/can_compress.t +++ b/t/Git-SVN/Utils/can_compress.t @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/t/Git-SVN/Utils/fatal.t b/t/Git-SVN/Utils/fatal.t index 49e1438295..75aaf1633d 100755 --- a/t/Git-SVN/Utils/fatal.t +++ b/t/Git-SVN/Utils/fatal.t @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl index dd8107cd7d..6fbec0162e 100755 --- a/t/check-non-portable-shell.pl +++ b/t/check-non-portable-shell.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # Test t0000..t9999.sh for non portable shell scripts # This script can be called with one or more filenames as parameters diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh index 1f32ca66ea..882c2e96f1 100644 --- a/t/lib-gitweb.sh +++ b/t/lib-gitweb.sh @@ -7,7 +7,7 @@ gitweb_init () { safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')" cat >gitweb_config.perl <<EOF -#!/usr/bin/perl +#!/usr/bin/env perl # gitweb configuration for tests diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 575d2000cc..1791c7528a 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use lib '../../perl/build/lib'; use strict; diff --git a/t/perf/min_time.perl b/t/perf/min_time.perl index c1a2717e07..2595eae616 100755 --- a/t/perf/min_time.perl +++ b/t/perf/min_time.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl my $minrt = 1e100; my $min; diff --git a/t/t0019/parse_json.perl b/t/t0019/parse_json.perl index fea87fb81b..7e455dee3c 100644 --- a/t/t0019/parse_json.perl +++ b/t/t0019/parse_json.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; use JSON; diff --git a/t/t0202/test.pl b/t/t0202/test.pl index 2cbf7b9590..d8e967bbcb 100755 --- a/t/t0202/test.pl +++ b/t/t0202/test.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use 5.008; use lib (split(/:/, $ENV{GITPERLLIB})); use strict; diff --git a/t/t0210/scrub_normal.perl b/t/t0210/scrub_normal.perl index 7cc4de392a..0c21a5438d 100644 --- a/t/t0210/scrub_normal.perl +++ b/t/t0210/scrub_normal.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Scrub the variable fields from the normal trace2 output to # make testing easier. diff --git a/t/t0211/scrub_perf.perl b/t/t0211/scrub_perf.perl index 7a50bae646..5ebdd5fec5 100644 --- a/t/t0211/scrub_perf.perl +++ b/t/t0211/scrub_perf.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Scrub the variable fields from the perf trace2 output to # make testing easier. diff --git a/t/t0212/parse_events.perl b/t/t0212/parse_events.perl index 30a9f51e9f..122a245c5d 100644 --- a/t/t0212/parse_events.perl +++ b/t/t0212/parse_events.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Parse event stream and convert individual events into a summary # record for the process. diff --git a/t/t4034/perl/post b/t/t4034/perl/post index e8b72ef5dc..87500971d0 100644 --- a/t/t4034/perl/post +++ b/t/t4034/perl/post @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; diff --git a/t/t4034/perl/pre b/t/t4034/perl/pre index f6610d37b8..5ab5aa42f3 100644 --- a/t/t4034/perl/pre +++ b/t/t4034/perl/pre @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; diff --git a/t/t7519/fsmonitor-all-v2 b/t/t7519/fsmonitor-all-v2 index 061907e88b..ef7c754afb 100755 --- a/t/t7519/fsmonitor-all-v2 +++ b/t/t7519/fsmonitor-all-v2 @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman index 264b9daf83..b22904964c 100755 --- a/t/t7519/fsmonitor-watchman +++ b/t/t7519/fsmonitor-watchman @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/t/t7519/fsmonitor-watchman-v2 b/t/t7519/fsmonitor-watchman-v2 index 14ed0aa42d..013d349b15 100755 --- a/t/t7519/fsmonitor-watchman-v2 +++ b/t/t7519/fsmonitor-watchman-v2 @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings; diff --git a/t/t9700/test.pl b/t/t9700/test.pl index 6d753708d2..2785e177d2 100755 --- a/t/t9700/test.pl +++ b/t/t9700/test.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use lib (split(/:/, $ENV{GITPERLLIB})); use 5.008; diff --git a/t/test-terminal.perl b/t/test-terminal.perl index 1bcf01a9a4..1c29027aa7 100755 --- a/t/test-terminal.perl +++ b/t/test-terminal.perl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use 5.008; use strict; use warnings; diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample index 23e856f5de..367d46266c 100755 --- a/templates/hooks--fsmonitor-watchman.sample +++ b/templates/hooks--fsmonitor-watchman.sample @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl use strict; use warnings;
The majority of Perl scripts we carry in Git have a `#!/usr/bin/perl` shebang. This is not a portable location for the Perl interpreter and may thus break on some systems that have the interpreter installed in a different location. One such example is NixOS, where the only executable installed in `/usr/bin` is env(1). Convert the shebangs to resolve the location of the Perl interpreter via env(1) to make these scripts more portable. While the location of env(1) is not guaranteed by any standard either, in practice all distributions including NixOS have it available at `/usr/bin/env`. We're also already using this idiom in a small set of other scripts, and until now nobody complained about them. This makes the test suite pass on NixOS. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Documentation/build-docdep.perl | 2 +- Documentation/cat-texi.perl | 2 +- Documentation/cmd-list.perl | 3 ++- Documentation/fix-texi.perl | 4 +++- Documentation/lint-fsck-msgids.perl | 2 +- Documentation/lint-gitlink.perl | 2 +- Documentation/lint-man-end-blurb.perl | 2 +- Documentation/lint-man-section-order.perl | 2 +- compat/vcbuild/scripts/clink.pl | 5 ++++- compat/vcbuild/scripts/lib.pl | 5 ++++- contrib/buildsystems/CMakeLists.txt | 2 +- contrib/buildsystems/engine.pl | 5 ++++- contrib/buildsystems/generate | 5 ++++- contrib/buildsystems/parse.pl | 5 ++++- contrib/contacts/git-contacts | 2 +- contrib/credential/netrc/git-credential-netrc.perl | 2 +- contrib/credential/netrc/test.pl | 2 +- contrib/diff-highlight/Makefile | 2 +- contrib/fast-import/git-import.perl | 2 +- contrib/fast-import/import-directories.perl | 2 +- contrib/fast-import/import-tars.perl | 2 +- contrib/hooks/setgitperms.perl | 2 +- contrib/hooks/update-paranoid | 2 +- contrib/long-running-filter/example.pl | 2 +- contrib/mw-to-git/git-mw.perl | 2 +- contrib/mw-to-git/git-remote-mediawiki.perl | 2 +- contrib/mw-to-git/t/test-gitmw.pl | 6 +++++- contrib/stats/mailmap.pl | 2 +- contrib/stats/packinfo.pl | 2 +- contrib/subtree/t/Makefile | 2 +- git-archimport.perl | 2 +- git-cvsexportcommit.perl | 2 +- git-cvsimport.perl | 2 +- git-cvsserver.perl | 2 +- git-send-email.perl | 2 +- git-svn.perl | 2 +- gitweb/gitweb.perl | 2 +- t/Git-SVN/Utils/can_compress.t | 2 +- t/Git-SVN/Utils/fatal.t | 2 +- t/check-non-portable-shell.pl | 2 +- t/lib-gitweb.sh | 2 +- t/perf/aggregate.perl | 2 +- t/perf/min_time.perl | 2 +- t/t0019/parse_json.perl | 2 +- t/t0202/test.pl | 2 +- t/t0210/scrub_normal.perl | 2 +- t/t0211/scrub_perf.perl | 2 +- t/t0212/parse_events.perl | 2 +- t/t4034/perl/post | 2 +- t/t4034/perl/pre | 2 +- t/t7519/fsmonitor-all-v2 | 2 +- t/t7519/fsmonitor-watchman | 2 +- t/t7519/fsmonitor-watchman-v2 | 2 +- t/t9700/test.pl | 2 +- t/test-terminal.perl | 2 +- templates/hooks--fsmonitor-watchman.sample | 2 +- 56 files changed, 78 insertions(+), 56 deletions(-)