All of lore.kernel.org
 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 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.