From: Bryan Larsen <bryan@larsen.st>
To: "Shawn O. Pearce" <spearce@spearce.org>
Cc: git <git@vger.kernel.org>
Subject: Re: [PATCH] Allow PERL_PATH="/usr/bin/env perl"
Date: Thu, 03 May 2007 18:58:00 -0400 [thread overview]
Message-ID: <463A68F8.4010709@larsen.st> (raw)
In-Reply-To: <20070503212618.GC16538@spearce.org>
Shawn O. Pearce wrote:
> Bryan Larsen <bryan@larsen.st> wrote:
>> The perl scripts start with "#!/usr/bin/perl". There is a mechanism
>> PERL_PATH in the Makefile to change this, but it currently doesn't work
>> with PERL_PATH="/usr/bin/env perl". This is causing problems in
>> MacPorts, where we wish to work with the MacPorts perl if it is
>> installed, but fall back to the system perl if it isn't.
>>
>> Signed-off-by: Bryan Larsen <bryan@larsen.st>
>> ---
>> perl/Makefile | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/perl/Makefile b/perl/Makefile
>> index 17d004e..2832cb4 100644
>> --- a/perl/Makefile
>> +++ b/perl/Makefile
>> @@ -33,7 +33,7 @@ $(makfile): ../GIT-CFLAGS Makefile
>> echo ' echo $(instdir_SQ)' >> $@
>> else
>> $(makfile): Makefile.PL ../GIT-CFLAGS
>> - '$(PERL_PATH_SQ)' $< PREFIX='$(prefix_SQ)'
>> + $(PERL_PATH_SQ) $< PREFIX='$(prefix_SQ)'
>> endif
>
> This will break if someone actually had ' in their PERL_PATH:
>
> PERL_PATH="/path'to'perl"
>
> as PERL_PATH_SQ tries to close the single quoted string you don't
> open anymore. That is because it is defined to be PERL_PATH,
> but with all ' replaced by '\''.
Actually, we should just use $(PERL_PATH) with no _SQ and no quotes.
The user is passing in something that is designed to go in a shebang
line: their funny characters are going to be interpreted by the
interactive shell to load the perl interpreter; we want "make" to do the
same thing.
>
> This change also breaks for anyone who had spaces in their PERL_PATH.
Actually a space didn't work before, but with my patch it works like
you'd expect in a shebang line, as the delimiter between the command and
it's arguments.
>
> Can I ask why you can't just supply a small wrapper shellscript
> with MacPorts
>
> cat >perl <<EOF
> #!/bin/sh
> exec env perl "$@"
> EOF
> chmod a+x perl
>
> ? Or better yet, supply Git the correct Perl path? If/when we ever
> go to native Perl libraries again a Git Perl library compiled for
> the system perl may not work later when the user installs a newer
> MacPorts perl. Switching automatically to the MacPorts perl without
> recompiling the native extensions is a little evil...
>
At that point we just add a single line "depends_lib port:perl5.8" to
the Portfile and everything magically works for the user. Right now
it's only in the git-svnimport part of the Portfile, which is nice: the
user doesn't get multiple megabytes worth of perl unless we also need to
install p5-svn-simple.
An even better fix can be to drop PERL_PATH from the Makefile and
replace the shebang line with "#!/usr/bin/env perl". Much simpler, and
the right thing to do, in my opinion.
cheers,
Bryan
prev parent reply other threads:[~2007-05-03 22:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-05-03 20:33 [PATCH] Allow PERL_PATH="/usr/bin/env perl" Bryan Larsen
2007-05-03 21:26 ` Shawn O. Pearce
2007-05-03 22:58 ` Bryan Larsen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=463A68F8.4010709@larsen.st \
--to=bryan@larsen.st \
--cc=git@vger.kernel.org \
--cc=spearce@spearce.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).