[PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Charles Bailey]

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 <charles@hashpling.org>
---
 git-instaweb.sh |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 42d8d7f..3565734 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -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"
 }
@@ -273,4 +275,4 @@ esac
 
 start_httpd
 url=http://127.0.0.1:$port
-"$browser" $url || echo $url
+test -n "$browser" && "$browser" $url || echo $url
-- 
1.5.4.rc0

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Wincent Colaiuta]

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

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Charles Bailey]

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.

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Wincent Colaiuta]

El 19/12/2007, a las 12:36, Charles Bailey escribió:

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

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

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Charles Bailey]

On Wed, Dec 19, 2007 at 12:43:12PM +0100, Wincent Colaiuta wrote:
> El 19/12/2007, a las 12:36, Charles Bailey escribió:
> 
> >
> >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
> 
> > 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.

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Charles Bailey]

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 <charles@hashpling.org>
---
 Makefile        |    1 +
 git-instaweb.sh |   15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 617e5f5..d6d3d65 100644
--- a/Makefile
+++ b/Makefile
@@ -880,6 +880,7 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    -e '/@@GITWEB_CGI@@/d' \
 	    -e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
 	    -e '/@@GITWEB_CSS@@/d' \
+	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 42d8d7f..9f86709 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -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"
 }
@@ -273,4 +276,4 @@ esac
 
 start_httpd
 url=http://127.0.0.1:$port
-"$browser" $url || echo $url
+test -n "$browser" && "$browser" $url || echo $url
-- 
1.5.4.rc0

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Wincent Colaiuta]

El 19/12/2007, a las 12:48, Charles Bailey escribió:

> On Wed, Dec 19, 2007 at 12:43:12PM +0100, Wincent Colaiuta wrote:
>> El 19/12/2007, a las 12:36, Charles Bailey escribió:
>>
>>> 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

Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Charles Bailey]

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.

[PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality

[Charles Bailey]

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 <charles@hashpling.org>
---
 Makefile        |    1 +
 git-instaweb.sh |   15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 617e5f5..d6d3d65 100644
--- a/Makefile
+++ b/Makefile
@@ -880,6 +880,7 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    -e '/@@GITWEB_CGI@@/d' \
 	    -e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
 	    -e '/@@GITWEB_CSS@@/d' \
+	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 42d8d7f..ad0723c 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -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"
 }
@@ -273,4 +276,4 @@ esac
 
 start_httpd
 url=http://127.0.0.1:$port
-"$browser" $url || echo $url
+test -n "$browser" && "$browser" $url || echo $url
-- 
1.5.4.rc0


-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/