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