From: Charles Bailey <hidden> Date: 2016-06-15 22:44:00
git-instaweb relied on a pipe in a sed script, but this is not supported
by MacOS X sed when using BREs. git-instaweb relies on a working perl
in any case, and perl re are more consistent between platforms, so
replace sed invocation with an equivalent perl invocation.
Also, fix the documented -b "" to work without giving a spurious 'command
not found' error.
Signed-off-by: Charles Bailey <redacted>
---
git-instaweb.sh | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
@@ -232,16 +232,18 @@ EOF}script='-s#^\(my\|our\)$projectroot=.*#\1$projectroot="'$(dirname"$fqgitdir")'";#-s#\(my\|our\)$gitbin=.*#\1$gitbin="'$GIT_EXEC_PATH'";#-s#\(my\|our\)$projects_list=.*#\1$projects_list=$projectroot;#-s#\(my\|our\)$git_temp=.*#\1$git_temp="'$fqgitdir/gitweb/tmp'";#'+s#^(my|our)\$projectroot=.*#$1\$projectroot="'$(dirname"$fqgitdir")'";#;+s#(my|our)\$gitbin=.*#$1\$gitbin="'$GIT_EXEC_PATH'";#;+s#(my|our)\$projects_list=.*#$1\$projects_list=\$projectroot;#;+s#(my|our)\$git_temp=.*#$1\$git_temp="'$fqgitdir/gitweb/tmp'";#;' gitweb_cgi(){cat>"$1.tmp"<<\EOFGITWEB @@GITWEB_CGI@@ EOFGITWEB-sed"$script""$1.tmp">"$1"+# The generated scripts assume that perl is /usr/bin/perl+# so the assumption here should be no more harmful+/usr/bin/perl-p-e"$script""$1.tmp">"$1"chmod+x"$1"rm-f"$1.tmp"}
El 19/12/2007, a las 11:57, Charles Bailey escribió:
- sed "$script" "$1.tmp" > "$1"
+ # The generated scripts assume that perl is /usr/bin/perl
+ # so the assumption here should be no more harmful
+ /usr/bin/perl -p -e "$script" "$1.tmp" > "$1"
I think it's a bad idea to hard-code the perl path there; the
generated scripts only assume /usr/bin/perl if the user hasn't
overridden it at build time:
PERL_PATH=/foo/perl make ...
Using just this should work fine anyway:
perl -p -e ...
Cheers,
Wincent
From: Charles Bailey <hidden> Date: 2016-06-15 22:44:00
On Wed, Dec 19, 2007 at 12:28:04PM +0100, Wincent Colaiuta wrote:
I think it's a bad idea to hard-code the perl path there; the
generated scripts only assume /usr/bin/perl if the user hasn't
overridden it at build time:
PERL_PATH=/foo/perl make ...
Using just this should work fine anyway:
perl -p -e ...
I agree completely, but all the generated scripts output hard coded
paths so it would seem inconsistent not to qualify the path in this
case too. Would @@PERL_PATH@@perl -p -e work, do you know?
Charles.
El 19/12/2007, a las 12:36, Charles Bailey escribió:
On Wed, Dec 19, 2007 at 12:28:04PM +0100, Wincent Colaiuta wrote:
quoted
I think it's a bad idea to hard-code the perl path there; the
generated scripts only assume /usr/bin/perl if the user hasn't
overridden it at build time:
PERL_PATH=/foo/perl make ...
Using just this should work fine anyway:
perl -p -e ...
I agree completely, but all the generated scripts output hard coded
paths so it would seem inconsistent not to qualify the path in this
case too.
It's not hard-coded, it's dynamic. Witness:
$ make PERL_PATH=/Volumes/Clon/usr/bin/perl
$ head -1 git-add--interactive
#!/Volumes/Clon/usr/bin/perl -w
Would @@PERL_PATH@@perl -p -e work, do you know?
I don't think so, but judging from the following section of the
Makefile, I think @@PERL@@ would. Why don't you give it a try?
$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-e 's/@@NO_CURL@@/$(NO_CURL)/g' \
-e 's|@@HTMLDIR@@|$(htmldir_SQ)|g' \
$@.sh >$@+ && \
chmod +x $@+ && \
mv $@+ $@
Cheers,
Wincent
From: Charles Bailey <hidden> Date: 2016-06-15 22:44:00
On Wed, Dec 19, 2007 at 12:43:12PM +0100, Wincent Colaiuta wrote:
El 19/12/2007, a las 12:36, Charles Bailey escribió:
quoted
I agree completely, but all the generated scripts output hard coded
paths so it would seem inconsistent not to qualify the path in this
case too.
It's not hard-coded, it's dynamic. Witness:
It's *output* hardcoded, it's dynamic during script generation.
A raw 'perl' in a shell script is dynamic during script run.
$ make PERL_PATH=/Volumes/Clon/usr/bin/perl
$ head -1 git-add--interactive
#!/Volumes/Clon/usr/bin/perl -w
quoted
Would @@PERL_PATH@@perl -p -e work, do you know?
I don't think so, but judging from the following section of the
Makefile, I think @@PERL@@ would. Why don't you give it a try?
$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-e 's/@@NO_CURL@@/$(NO_CURL)/g' \
-e 's|@@HTMLDIR@@|$(htmldir_SQ)|g' \
$@.sh >$@+ && \
chmod +x $@+ && \
mv $@+ $@
git-instaweb is treated specially, so the answer is 'no, not yet', but
I have a patch on the way.
Charles.
From: Charles Bailey <hidden> Date: 2016-06-15 22:44:00
git-instaweb relied on a pipe in a sed script, but this is not supported
by MacOS X sed when using BREs. git-instaweb relies on a working perl
in any case, and perl re are more consistent between platforms, so
replace sed invocation with an equivalent perl invocation.
Also, fix the documented -b "" to work without giving a spurious 'command
not found' error.
Signed-off-by: Charles Bailey <redacted>
---
Makefile | 1 +
git-instaweb.sh | 15 +++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
@@ -3,6 +3,7 @@# Copyright (c) 2006 Eric Wong#+PERL='@@PERL@@'OPTIONS_KEEPDASHDASH=OPTIONS_SPEC="\ git-instaweb[options](--start|--stop|--restart)
@@ -232,16 +233,18 @@ EOF}script='-s#^\(my\|our\)$projectroot=.*#\1$projectroot="'$(dirname"$fqgitdir")'";#-s#\(my\|our\)$gitbin=.*#\1$gitbin="'$GIT_EXEC_PATH'";#-s#\(my\|our\)$projects_list=.*#\1$projects_list=$projectroot;#-s#\(my\|our\)$git_temp=.*#\1$git_temp="'$fqgitdir/gitweb/tmp'";#'+s#^(my|our)\$projectroot=.*#$1\$projectroot="'$(dirname"$fqgitdir")'";#;+s#(my|our)\$gitbin=.*#$1\$gitbin="'$GIT_EXEC_PATH'";#;+s#(my|our)\$projects_list=.*#$1\$projects_list=\$projectroot;#;+s#(my|our)\$git_temp=.*#$1\$git_temp="'$fqgitdir/gitweb/tmp'";#;' gitweb_cgi(){cat>"$1.tmp"<<\EOFGITWEB @@GITWEB_CGI@@ EOFGITWEB-sed"$script""$1.tmp">"$1"+# The generated scripts assume that perl is /usr/bin/perl+# so the assumption here should be no more harmful+"$PERL"-p-e"$script""$1.tmp">"$1"chmod+x"$1"rm-f"$1.tmp"}
El 19/12/2007, a las 12:48, Charles Bailey escribió:
On Wed, Dec 19, 2007 at 12:43:12PM +0100, Wincent Colaiuta wrote:
quoted
El 19/12/2007, a las 12:36, Charles Bailey escribió:
quoted
I agree completely, but all the generated scripts output hard coded
paths so it would seem inconsistent not to qualify the path in this
case too.
It's not hard-coded, it's dynamic. Witness:
It's *output* hardcoded, it's dynamic during script generation.
Ah, ok. Seems like we were working with different definitions of "hard-
coded". When I said "hard-coded" I meant "determined by you and not
overrideable by the user doing the build without hand-editing the
source file".
A raw 'perl' in a shell script is dynamic during script run.
Sure, and seeing as you're only doing a simple find/replace any
version of perl installed on the path is probably fine.
But if you are going to provide an absolute path then you should at
least make it user-configurable like all the other perl paths.
Cheers,
Wincent
From: Charles Bailey <hidden> Date: 2016-06-15 22:44:00
Now that I think about it, this replacement patch has invalidated the
original comment to some extent. A replacement patch follows, with a
more consistent comment.
Charles.
From: Charles Bailey <hidden> Date: 2016-06-15 22:44:00
git-instaweb relied on a pipe in a sed script, but this is not supported
by MacOS X sed when using BREs. git-instaweb relies on a working perl
in any case, and perl re are more consistent between platforms, so
replace sed invocation with an equivalent perl invocation.
Also, fix the documented -b "" to work without giving a spurious 'command
not found' error.
Signed-off-by: Charles Bailey <redacted>
---
Makefile | 1 +
git-instaweb.sh | 15 +++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
@@ -3,6 +3,7 @@# Copyright (c) 2006 Eric Wong#+PERL='@@PERL@@'OPTIONS_KEEPDASHDASH=OPTIONS_SPEC="\ git-instaweb[options](--start|--stop|--restart)
@@ -232,16 +233,18 @@ EOF}script='-s#^\(my\|our\)$projectroot=.*#\1$projectroot="'$(dirname"$fqgitdir")'";#-s#\(my\|our\)$gitbin=.*#\1$gitbin="'$GIT_EXEC_PATH'";#-s#\(my\|our\)$projects_list=.*#\1$projects_list=$projectroot;#-s#\(my\|our\)$git_temp=.*#\1$git_temp="'$fqgitdir/gitweb/tmp'";#'+s#^(my|our)\$projectroot=.*#$1\$projectroot="'$(dirname"$fqgitdir")'";#;+s#(my|our)\$gitbin=.*#$1\$gitbin="'$GIT_EXEC_PATH'";#;+s#(my|our)\$projects_list=.*#$1\$projects_list=\$projectroot;#;+s#(my|our)\$git_temp=.*#$1\$git_temp="'$fqgitdir/gitweb/tmp'";#;' gitweb_cgi(){cat>"$1.tmp"<<\EOFGITWEB @@GITWEB_CGI@@ EOFGITWEB-sed"$script""$1.tmp">"$1"+# Use the configured full path to perl to match the generated+# scripts' 'hashpling' line+"$PERL"-p-e"$script""$1.tmp">"$1"chmod+x"$1"rm-f"$1.tmp"}