git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Allow PERL_PATH="/usr/bin/env perl"
@ 2007-05-03 20:33 Bryan Larsen
  2007-05-03 21:26 ` Shawn O. Pearce
  0 siblings, 1 reply; 3+ messages in thread
From: Bryan Larsen @ 2007-05-03 20:33 UTC (permalink / raw)
  To: git

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 is just added comfort for calling make directly in perl dir
-- 
1.5.1.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Allow PERL_PATH="/usr/bin/env perl"
  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
  0 siblings, 1 reply; 3+ messages in thread
From: Shawn O. Pearce @ 2007-05-03 21:26 UTC (permalink / raw)
  To: Bryan Larsen; +Cc: git

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 '\''.

This change also breaks for anyone who had spaces in their PERL_PATH.

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

-- 
Shawn.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Allow PERL_PATH="/usr/bin/env perl"
  2007-05-03 21:26 ` Shawn O. Pearce
@ 2007-05-03 22:58   ` Bryan Larsen
  0 siblings, 0 replies; 3+ messages in thread
From: Bryan Larsen @ 2007-05-03 22:58 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-05-03 22:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 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).