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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.