{"thread":{"id":"11355","subject":"[PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","startedAt":"2007-12-19T10:57:52Z","lastAt":"2007-12-19T12:25:27Z","messageCount":9,"participants":["Charles Bailey","Wincent Colaiuta"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"63742","messageId":"20071219105752.GA23932@hashpling.org","threadId":"11355","inReplyTo":null,"subject":"[PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2007-12-19T10:57:52Z","receivedAt":"2007-12-19T10:57:52Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"git-instaweb relied on a pipe in a sed script, but this is not supported\nby MacOS X sed when using BREs.  git-instaweb relies on a working perl\nin any case, and perl re are more consistent between platforms, so\nreplace sed invocation with an equivalent perl invocation.\n\nAlso, fix the documented -b \"\" to work without giving a spurious 'command\nnot found' error.\n\nSigned-off-by: Charles Bailey <charles@hashpling.org>\n---\n git-instaweb.sh |   14 ++++++++------\n 1 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 42d8d7f..3565734 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -232,16 +232,18 @@ EOF\n }\n \n script='\n-s#^\\(my\\|our\\) $projectroot =.*#\\1 $projectroot = \"'$(dirname \"$fqgitdir\")'\";#\n-s#\\(my\\|our\\) $gitbin =.*#\\1 $gitbin = \"'$GIT_EXEC_PATH'\";#\n-s#\\(my\\|our\\) $projects_list =.*#\\1 $projects_list = $projectroot;#\n-s#\\(my\\|our\\) $git_temp =.*#\\1 $git_temp = \"'$fqgitdir/gitweb/tmp'\";#'\n+s#^(my|our) \\$projectroot =.*#$1 \\$projectroot = \"'$(dirname \"$fqgitdir\")'\";#;\n+s#(my|our) \\$gitbin =.*#$1 \\$gitbin = \"'$GIT_EXEC_PATH'\";#;\n+s#(my|our) \\$projects_list =.*#$1 \\$projects_list = \\$projectroot;#;\n+s#(my|our) \\$git_temp =.*#$1 \\$git_temp = \"'$fqgitdir/gitweb/tmp'\";#;'\n \n gitweb_cgi () {\n \tcat > \"$1.tmp\" <<\\EOFGITWEB\n @@GITWEB_CGI@@\n EOFGITWEB\n-\tsed \"$script\" \"$1.tmp\"  > \"$1\"\n+\t# The generated scripts assume that perl is /usr/bin/perl\n+\t# so the assumption here should be no more harmful\n+\t/usr/bin/perl -p -e \"$script\" \"$1.tmp\"  > \"$1\"\n \tchmod +x \"$1\"\n \trm -f \"$1.tmp\"\n }\n@@ -273,4 +275,4 @@ esac\n \n start_httpd\n url=http://127.0.0.1:$port\n-\"$browser\" $url || echo $url\n+test -n \"$browser\" && \"$browser\" $url || echo $url\n-- \n1.5.4.rc0\n"},{"id":"63746","messageId":"23306E3C-24F1-4626-A956-02531644B786@wincent.com","threadId":"11355","inReplyTo":"20071219105752.GA23932@hashpling.org","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-19T11:28:04Z","receivedAt":"2007-12-19T11:28:04Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 19/12/2007, a las 11:57, Charles Bailey escribió:\n\n> -\tsed \"$script\" \"$1.tmp\"  > \"$1\"\n> +\t# The generated scripts assume that perl is /usr/bin/perl\n> +\t# so the assumption here should be no more harmful\n> +\t/usr/bin/perl -p -e \"$script\" \"$1.tmp\"  > \"$1\"\n\nI think it's a bad idea to hard-code the perl path there; the  \ngenerated scripts only assume /usr/bin/perl if the user hasn't  \noverridden it at build time:\n\n\tPERL_PATH=/foo/perl make ...\n\nUsing just this should work fine anyway:\n\n\tperl -p -e ...\n\nCheers,\nWincent\n"},{"id":"63748","messageId":"20071219113617.GA26520@hashpling.org","threadId":"11355","inReplyTo":"23306E3C-24F1-4626-A956-02531644B786@wincent.com","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2007-12-19T11:36:17Z","receivedAt":"2007-12-19T11:36:17Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Wed, Dec 19, 2007 at 12:28:04PM +0100, Wincent Colaiuta wrote:\n> \n> I think it's a bad idea to hard-code the perl path there; the  \n> generated scripts only assume /usr/bin/perl if the user hasn't  \n> overridden it at build time:\n> \n> \tPERL_PATH=/foo/perl make ...\n> \n> Using just this should work fine anyway:\n> \n> \tperl -p -e ...\n> \n\nI agree completely, but all the generated scripts output hard coded\npaths so it would seem inconsistent not to qualify the path in this\ncase too.  Would  @@PERL_PATH@@perl -p -e work, do you know?\n\nCharles.\n"},{"id":"63752","messageId":"CA30F61D-4AE1-4100-A632-20B5587EB4D0@wincent.com","threadId":"11355","inReplyTo":"20071219113617.GA26520@hashpling.org","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-19T11:43:12Z","receivedAt":"2007-12-19T11:43:12Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 19/12/2007, a las 12:36, Charles Bailey escribió:\n\n> On Wed, Dec 19, 2007 at 12:28:04PM +0100, Wincent Colaiuta wrote:\n>>\n>> I think it's a bad idea to hard-code the perl path there; the\n>> generated scripts only assume /usr/bin/perl if the user hasn't\n>> overridden it at build time:\n>>\n>> \tPERL_PATH=/foo/perl make ...\n>>\n>> Using just this should work fine anyway:\n>>\n>> \tperl -p -e ...\n>>\n>\n> I agree completely, but all the generated scripts output hard coded\n> paths so it would seem inconsistent not to qualify the path in this\n> case too.\n\nIt's not hard-coded, it's dynamic. Witness:\n\n$ make PERL_PATH=/Volumes/Clon/usr/bin/perl\n$ head -1 git-add--interactive\n#!/Volumes/Clon/usr/bin/perl -w\n\n>  Would  @@PERL_PATH@@perl -p -e work, do you know?\n\nI don't think so, but judging from the following section of the  \nMakefile, I think @@PERL@@ would. Why don't you give it a try?\n\n$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n         $(QUIET_GEN)$(RM) $@ $@+ && \\\n         sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n             -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n             -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n             -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n             -e 's|@@HTMLDIR@@|$(htmldir_SQ)|g' \\\n             $@.sh >$@+ && \\\n         chmod +x $@+ && \\\n         mv $@+ $@\n\nCheers,\nWincent\n"},{"id":"63754","messageId":"20071219114823.GA28273@hashpling.org","threadId":"11355","inReplyTo":"CA30F61D-4AE1-4100-A632-20B5587EB4D0@wincent.com","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2007-12-19T11:48:23Z","receivedAt":"2007-12-19T11:48:23Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Wed, Dec 19, 2007 at 12:43:12PM +0100, Wincent Colaiuta wrote:\n> El 19/12/2007, a las 12:36, Charles Bailey escribió:\n> \n> >\n> >I agree completely, but all the generated scripts output hard coded\n> >paths so it would seem inconsistent not to qualify the path in this\n> >case too.\n> \n> It's not hard-coded, it's dynamic. Witness:\n> \n\nIt's *output* hardcoded, it's dynamic during script generation.\n\nA raw 'perl' in a shell script is dynamic during script run.\n\n\n> $ make PERL_PATH=/Volumes/Clon/usr/bin/perl\n> $ head -1 git-add--interactive\n> #!/Volumes/Clon/usr/bin/perl -w\n> \n> > Would  @@PERL_PATH@@perl -p -e work, do you know?\n> \n> I don't think so, but judging from the following section of the  \n> Makefile, I think @@PERL@@ would. Why don't you give it a try?\n> \n> $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n>         $(QUIET_GEN)$(RM) $@ $@+ && \\\n>         sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n>             -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n>             -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n>             -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n>             -e 's|@@HTMLDIR@@|$(htmldir_SQ)|g' \\\n>             $@.sh >$@+ && \\\n>         chmod +x $@+ && \\\n>         mv $@+ $@\n> \n\ngit-instaweb is treated specially, so the answer is 'no, not yet', but\nI have a patch on the way.\n\nCharles.\n"},{"id":"63755","messageId":"20071219115319.GA29184@hashpling.org","threadId":"11355","inReplyTo":"20071219114823.GA28273@hashpling.org","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2007-12-19T11:53:19Z","receivedAt":"2007-12-19T11:53:19Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"git-instaweb relied on a pipe in a sed script, but this is not supported\nby MacOS X sed when using BREs.  git-instaweb relies on a working perl\nin any case, and perl re are more consistent between platforms, so\nreplace sed invocation with an equivalent perl invocation.\n\nAlso, fix the documented -b \"\" to work without giving a spurious 'command\nnot found' error.\n\nSigned-off-by: Charles Bailey <charles@hashpling.org>\n---\n Makefile        |    1 +\n git-instaweb.sh |   15 +++++++++------\n 2 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 617e5f5..d6d3d65 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -880,6 +880,7 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css\n \t    -e '/@@GITWEB_CGI@@/d' \\\n \t    -e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \\\n \t    -e '/@@GITWEB_CSS@@/d' \\\n+\t    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n \t    $@.sh > $@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 42d8d7f..9f86709 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -3,6 +3,7 @@\n # Copyright (c) 2006 Eric Wong\n #\n \n+PERL='@@PERL@@'\n OPTIONS_KEEPDASHDASH=\n OPTIONS_SPEC=\"\\\n git-instaweb [options] (--start | --stop | --restart)\n@@ -232,16 +233,18 @@ EOF\n }\n \n script='\n-s#^\\(my\\|our\\) $projectroot =.*#\\1 $projectroot = \"'$(dirname \"$fqgitdir\")'\";#\n-s#\\(my\\|our\\) $gitbin =.*#\\1 $gitbin = \"'$GIT_EXEC_PATH'\";#\n-s#\\(my\\|our\\) $projects_list =.*#\\1 $projects_list = $projectroot;#\n-s#\\(my\\|our\\) $git_temp =.*#\\1 $git_temp = \"'$fqgitdir/gitweb/tmp'\";#'\n+s#^(my|our) \\$projectroot =.*#$1 \\$projectroot = \"'$(dirname \"$fqgitdir\")'\";#;\n+s#(my|our) \\$gitbin =.*#$1 \\$gitbin = \"'$GIT_EXEC_PATH'\";#;\n+s#(my|our) \\$projects_list =.*#$1 \\$projects_list = \\$projectroot;#;\n+s#(my|our) \\$git_temp =.*#$1 \\$git_temp = \"'$fqgitdir/gitweb/tmp'\";#;'\n \n gitweb_cgi () {\n \tcat > \"$1.tmp\" <<\\EOFGITWEB\n @@GITWEB_CGI@@\n EOFGITWEB\n-\tsed \"$script\" \"$1.tmp\"  > \"$1\"\n+\t# The generated scripts assume that perl is /usr/bin/perl\n+\t# so the assumption here should be no more harmful\n+\t\"$PERL\" -p -e \"$script\" \"$1.tmp\"  > \"$1\"\n \tchmod +x \"$1\"\n \trm -f \"$1.tmp\"\n }\n@@ -273,4 +276,4 @@ esac\n \n start_httpd\n url=http://127.0.0.1:$port\n-\"$browser\" $url || echo $url\n+test -n \"$browser\" && \"$browser\" $url || echo $url\n-- \n1.5.4.rc0\n"},{"id":"63756","messageId":"DE63D291-1B73-41FB-AF56-9B671A6A66E4@wincent.com","threadId":"11355","inReplyTo":"20071219114823.GA28273@hashpling.org","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-19T11:55:37Z","receivedAt":"2007-12-19T11:55:37Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 19/12/2007, a las 12:48, Charles Bailey escribió:\n\n> On Wed, Dec 19, 2007 at 12:43:12PM +0100, Wincent Colaiuta wrote:\n>> El 19/12/2007, a las 12:36, Charles Bailey escribió:\n>>\n>>> I agree completely, but all the generated scripts output hard coded\n>>> paths so it would seem inconsistent not to qualify the path in this\n>>> case too.\n>>\n>> It's not hard-coded, it's dynamic. Witness:\n>\n> It's *output* hardcoded, it's dynamic during script generation.\n\nAh, ok. Seems like we were working with different definitions of \"hard- \ncoded\". When I said \"hard-coded\" I meant \"determined by you and not  \noverrideable by the user doing the build without hand-editing the  \nsource file\".\n\n> A raw 'perl' in a shell script is dynamic during script run.\n\nSure, and seeing as you're only doing a simple find/replace any  \nversion of perl installed on the path is probably fine.\n\nBut if you are going to provide an absolute path then you should at  \nleast make it user-configurable like all the other perl paths.\n\nCheers,\nWincent\n"},{"id":"63763","messageId":"20071219122308.GA32088@hashpling.org","threadId":"11355","inReplyTo":"20071219115319.GA29184@hashpling.org","subject":"Re: [PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2007-12-19T12:23:08Z","receivedAt":"2007-12-19T12:23:08Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"Now that I think about it, this replacement patch has invalidated the\noriginal comment to some extent.  A replacement patch follows, with a\nmore consistent comment.\n\nCharles.\n"},{"id":"63764","messageId":"20071219122527.GA503@hashpling.org","threadId":"11355","inReplyTo":"20071219122308.GA32088@hashpling.org","subject":"[PATCH] Fix git-instaweb breakage on MacOS X due to the limited sed functionality","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2007-12-19T12:25:27Z","receivedAt":"2007-12-19T12:25:27Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"git-instaweb relied on a pipe in a sed script, but this is not supported\nby MacOS X sed when using BREs.  git-instaweb relies on a working perl\nin any case, and perl re are more consistent between platforms, so\nreplace sed invocation with an equivalent perl invocation.\n\nAlso, fix the documented -b \"\" to work without giving a spurious 'command\nnot found' error.\n\nSigned-off-by: Charles Bailey <charles@hashpling.org>\n---\n Makefile        |    1 +\n git-instaweb.sh |   15 +++++++++------\n 2 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 617e5f5..d6d3d65 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -880,6 +880,7 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css\n \t    -e '/@@GITWEB_CGI@@/d' \\\n \t    -e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \\\n \t    -e '/@@GITWEB_CSS@@/d' \\\n+\t    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n \t    $@.sh > $@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex 42d8d7f..ad0723c 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -3,6 +3,7 @@\n # Copyright (c) 2006 Eric Wong\n #\n \n+PERL='@@PERL@@'\n OPTIONS_KEEPDASHDASH=\n OPTIONS_SPEC=\"\\\n git-instaweb [options] (--start | --stop | --restart)\n@@ -232,16 +233,18 @@ EOF\n }\n \n script='\n-s#^\\(my\\|our\\) $projectroot =.*#\\1 $projectroot = \"'$(dirname \"$fqgitdir\")'\";#\n-s#\\(my\\|our\\) $gitbin =.*#\\1 $gitbin = \"'$GIT_EXEC_PATH'\";#\n-s#\\(my\\|our\\) $projects_list =.*#\\1 $projects_list = $projectroot;#\n-s#\\(my\\|our\\) $git_temp =.*#\\1 $git_temp = \"'$fqgitdir/gitweb/tmp'\";#'\n+s#^(my|our) \\$projectroot =.*#$1 \\$projectroot = \"'$(dirname \"$fqgitdir\")'\";#;\n+s#(my|our) \\$gitbin =.*#$1 \\$gitbin = \"'$GIT_EXEC_PATH'\";#;\n+s#(my|our) \\$projects_list =.*#$1 \\$projects_list = \\$projectroot;#;\n+s#(my|our) \\$git_temp =.*#$1 \\$git_temp = \"'$fqgitdir/gitweb/tmp'\";#;'\n \n gitweb_cgi () {\n \tcat > \"$1.tmp\" <<\\EOFGITWEB\n @@GITWEB_CGI@@\n EOFGITWEB\n-\tsed \"$script\" \"$1.tmp\"  > \"$1\"\n+\t# Use the configured full path to perl to match the generated\n+\t# scripts' 'hashpling' line\n+\t\"$PERL\" -p -e \"$script\" \"$1.tmp\"  > \"$1\"\n \tchmod +x \"$1\"\n \trm -f \"$1.tmp\"\n }\n@@ -273,4 +276,4 @@ esac\n \n start_httpd\n url=http://127.0.0.1:$port\n-\"$browser\" $url || echo $url\n+test -n \"$browser\" && \"$browser\" $url || echo $url\n-- \n1.5.4.rc0\n\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"}]}