git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

      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).