* [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
@ 2007-12-19 10:57 Charles Bailey
2007-12-19 11:28 ` Wincent Colaiuta
0 siblings, 1 reply; 9+ messages in thread
From: Charles Bailey @ 2007-12-19 10:57 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 10:57 [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality Charles Bailey
@ 2007-12-19 11:28 ` Wincent Colaiuta
2007-12-19 11:36 ` Charles Bailey
0 siblings, 1 reply; 9+ messages in thread
From: Wincent Colaiuta @ 2007-12-19 11:28 UTC (permalink / raw)
To: Charles Bailey; +Cc: git
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 11:28 ` Wincent Colaiuta
@ 2007-12-19 11:36 ` Charles Bailey
2007-12-19 11:43 ` Wincent Colaiuta
0 siblings, 1 reply; 9+ messages in thread
From: Charles Bailey @ 2007-12-19 11:36 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 11:36 ` Charles Bailey
@ 2007-12-19 11:43 ` Wincent Colaiuta
2007-12-19 11:48 ` Charles Bailey
0 siblings, 1 reply; 9+ messages in thread
From: Wincent Colaiuta @ 2007-12-19 11:43 UTC (permalink / raw)
To: Charles Bailey; +Cc: git
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 11:43 ` Wincent Colaiuta
@ 2007-12-19 11:48 ` Charles Bailey
2007-12-19 11:53 ` Charles Bailey
2007-12-19 11:55 ` Wincent Colaiuta
0 siblings, 2 replies; 9+ messages in thread
From: Charles Bailey @ 2007-12-19 11:48 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 11:48 ` Charles Bailey
@ 2007-12-19 11:53 ` Charles Bailey
2007-12-19 12:23 ` Charles Bailey
2007-12-19 11:55 ` Wincent Colaiuta
1 sibling, 1 reply; 9+ messages in thread
From: Charles Bailey @ 2007-12-19 11:53 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 11:53 ` Charles Bailey
@ 2007-12-19 12:23 ` Charles Bailey
2007-12-19 12:25 ` Charles Bailey
0 siblings, 1 reply; 9+ messages in thread
From: Charles Bailey @ 2007-12-19 12:23 UTC (permalink / raw)
To: git
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 12:23 ` Charles Bailey
@ 2007-12-19 12:25 ` Charles Bailey
0 siblings, 0 replies; 9+ messages in thread
From: Charles Bailey @ 2007-12-19 12:25 UTC (permalink / raw)
To: git
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/
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality
2007-12-19 11:48 ` Charles Bailey
2007-12-19 11:53 ` Charles Bailey
@ 2007-12-19 11:55 ` Wincent Colaiuta
1 sibling, 0 replies; 9+ messages in thread
From: Wincent Colaiuta @ 2007-12-19 11:55 UTC (permalink / raw)
To: Charles Bailey; +Cc: git
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-12-19 12:25 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-19 10:57 [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality Charles Bailey
2007-12-19 11:28 ` Wincent Colaiuta
2007-12-19 11:36 ` Charles Bailey
2007-12-19 11:43 ` Wincent Colaiuta
2007-12-19 11:48 ` Charles Bailey
2007-12-19 11:53 ` Charles Bailey
2007-12-19 12:23 ` Charles Bailey
2007-12-19 12:25 ` Charles Bailey
2007-12-19 11:55 ` Wincent Colaiuta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox