{"thread":{"id":"24623","subject":"git instaweb + webrick does not work","startedAt":"2010-08-03T21:37:39Z","lastAt":"2010-08-05T09:14:12Z","messageCount":5,"participants":["Michael Dippery","Jakub Narebski","Jared Hance","Eric Wong"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"147074","messageId":"1272BF62-A0C8-4940-9472-E46C05BF1723@gmail.com","threadId":"24623","inReplyTo":null,"subject":"git instaweb + webrick does not work","fromName":"Michael Dippery","fromEmail":"mdippery@gmail.com","sentAt":"2010-08-03T21:37:39Z","receivedAt":"2010-08-03T21:37:39Z","isPatch":false,"sender":{"key":"mdippery@gmail.com","avatar":null},"body":"I'm trying to use `git instaweb` with Ruby's webrick (which I have installed) on Mac OS X, but every time I fire up it up via `git instaweb --httpd=webrick`, I get the following error:\n\n\"webrick not found. Install webrick or use --httpd to specify another httpd daemon.\"\n\nI _do_ have webrick. Asking around a bit, I was told the the problem may have been introduced in commit be5347b. Is this so? If not, any other ideas on what may be causing the problem?\n\nI'm using Git v1.7.2.1 on Mac OS X 10.6.\n\n\nThanks,\n\n\n----\nMichael Dippery\nmdippery@gmail.com | www.monkey-robot.com\n\n"},{"id":"147076","messageId":"m34ofbpcyr.fsf@localhost.localdomain","threadId":"24623","inReplyTo":"1272BF62-A0C8-4940-9472-E46C05BF1723@gmail.com","subject":"Re: git instaweb + webrick does not work","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-08-03T22:07:48Z","receivedAt":"2010-08-03T22:07:48Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michael Dippery writes:\n\n> I'm trying to use `git instaweb` with Ruby's webrick (which I have\n> installed) on Mac OS X, but every time I fire up it up via `git\n> instaweb --httpd=webrick`, I get the following error:\n> \n> \"webrick not found. Install webrick or use --httpd to specify\n> another httpd daemon.\"\n> \n> I _do_ have webrick. Asking around a bit, I was told the the problem\n> may have been introduced in commit be5347b. Is this so? If not, any\n> other ideas on what may be causing the problem?\n> \n> I'm using Git v1.7.2.1 on Mac OS X 10.6.\n\nTo be more exact commit be5347b (git-instaweb: Put httpd logs in a\n\"$httpd_only\" subdirectory, 2010-05-28) by Pavan Kumar Sunkara added\nresolve_full_httpd before running *_config (webrick_config in this\ncase). But resolve_full_httpd() beside setting $httpd_only needed\nlater for functionality provided by this commit, does also setting\n$full_httpd and checking if given web server can be run.\n\nThe `webrick' support in git-instaweb is peculiar in that webrick_conf\ncreates 'webrick' shell script in \"$GIT_DIR/gitweb/\". The code that\nchecks if web server is available in resolve_full_httpd() searches\nalso in \"$GIT_DIR/gitweb/\"... but it is run before webrick_conf\nfunction in git-instaweb has a chance to generate 'webrick' script.\n\nThe solution would be to either split resolve_full_httpd() into one\nfunction generating $httpd and $httpd_only, and second function\ngenerating $full_httpd and checing for web server existence, or create\na separate check for 'webrick'.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"147077","messageId":"c14fac3f8bedfe451e7b06b626632e724c993c52.1280874664.git.jaredhance@gmail.com","threadId":"24623","inReplyTo":"m34ofbpcyr.fsf@localhost.localdomain","subject":"[RFC/PATCH] Split resolve_full_httpd to prevent bug.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-08-03T22:34:36Z","receivedAt":"2010-08-03T22:34:36Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"resolve_full_httpd tries to find the httpd path, which can break if the\nhttpd configuration hasn't happened. Specifically, this occurs with\nwebrick.\n\nSigned-off-by: Jared Hance \n---\n\nWARNING: I haven't tested this patch, as I don't use git-instaweb. I\nalso don't have webrick, so I can't be sure this actually works.\nHopefully this can be used as a foundation for a real fix.\n\n git-instaweb.sh | 18 ++++++++++++++----\n 1 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex b7342e2..b7950ac 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -41,7 +41,7 @@ test -z \"$root\" && root='@@GITWEBDIR@@'\n # any untaken local port will do...\n test -z \"$port\" && port=1234\n \n-resolve_full_httpd () {\n+resolve_httpd () {\n \tcase \"$httpd\" in\n \t*apache2*|*lighttpd*|*httpd*)\n \t\t# yes, *httpd* covers *lighttpd* above, but it is there for clarity\n@@ -52,14 +52,21 @@ resolve_full_httpd () {\n \t\tfi\n \t\t;;\n \t*plackup*)\n-\t\t# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb\n-\t\tfull_httpd=\"$fqgitdir/gitweb/gitweb.psgi\"\n \t\thttpd_only=\"${httpd%% *}\" # cut on first space\n \t\treturn\n \t\t;;\n \tesac\n \n \thttpd_only=\"$(echo $httpd | cut -f1 -d' ')\"\n+}\n+\n+# please run resolve_httpd first\n+resolve_full_httpd() {\n+\tif test $httpd = \"plackup\"; then\n+\t\t# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb\n+\t\tfull_httpd=\"$fqgitdir/gitweb/gitweb.psgi\"\n+\tfi\n+\n \tif case \"$httpd_only\" in /*) : ;; *) which $httpd_only >/dev/null 2>&1;; esac\n \tthen\n \t\tfull_httpd=$httpd\n@@ -90,6 +97,7 @@ start_httpd () {\n \tfi\n \n \t# here $httpd should have a meaningful value\n+\tresolve_httpd\n \tresolve_full_httpd\n \n \t# don't quote $full_httpd, there can be arguments to it (-f)\n@@ -354,6 +362,7 @@ PerlPassEnv GITWEB_CONFIG\n EOF\n \telse\n \t\t# plain-old CGI\n+\t\tresolve_httpd\n \t\tresolve_full_httpd\n \t\tlist_mods=$(echo \"$full_httpd\" | sed 's/-f$/-l/')\n \t\t$list_mods | sane_grep 'mod_cgi\\.c' >/dev/null 2>&1 || \\\n@@ -565,7 +574,7 @@ EOF\n \n gitweb_conf\n \n-resolve_full_httpd\n+resolve_httpd\n mkdir -p \"$fqgitdir/gitweb/$httpd_only\"\n \n case \"$httpd\" in\n@@ -590,6 +599,7 @@ webrick)\n \t;;\n esac\n \n+\n start_httpd\n url=http://127.0.0.1:$port\n \n-- \n1.7.2\n"},{"id":"147104","messageId":"20100804102545.GA32128@dcvr.yhbt.net","threadId":"24623","inReplyTo":"m34ofbpcyr.fsf@localhost.localdomain","subject":"Re: git instaweb + webrick does not work","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2010-08-04T10:25:45Z","receivedAt":"2010-08-04T10:25:45Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jakub Narebski wrote:\n> Michael Dippery writes:\n> > I _do_ have webrick. Asking around a bit, I was told the the problem\n> > may have been introduced in commit be5347b. Is this so? If not, any\n> > other ideas on what may be causing the problem?\n> \n> To be more exact commit be5347b (git-instaweb: Put httpd logs in a\n> \"$httpd_only\" subdirectory, 2010-05-28) by Pavan Kumar Sunkara added\n> resolve_full_httpd before running *_config (webrick_config in this\n> case). But resolve_full_httpd() beside setting $httpd_only needed\n> later for functionality provided by this commit, does also setting\n> $full_httpd and checking if given web server can be run.\n> \n> The `webrick' support in git-instaweb is peculiar in that webrick_conf\n> creates 'webrick' shell script in \"$GIT_DIR/gitweb/\". The code that\n> checks if web server is available in resolve_full_httpd() searches\n> also in \"$GIT_DIR/gitweb/\"... but it is run before webrick_conf\n> function in git-instaweb has a chance to generate 'webrick' script.\n\nI wouldn't mind making it more like what we do with plackup and\nhaving a single Ruby script, eventually.\n\n> The solution would be to either split resolve_full_httpd() into one\n> function generating $httpd and $httpd_only, and second function\n> generating $full_httpd and checing for web server existence, or create\n> a separate check for 'webrick'.\n\nI just split out the check and started modelling things after the code\nfor plackup. Unfortunately, I haven't had any luck getting gitweb.cgi\nto respect $GITWEB_CONFIG environment with webrick so the following\npatch just ensures webrick is properly started, not useful.\n\nUnfortunately I'm barely awake now, but I'll revisit this in ~16 hours\nif nobody beats me to it.\n\n>From 70b1773fc0bcb550788b26f2fda6ad6423960115 Mon Sep 17 00:00:00 2001\nFrom: Eric Wong \nDate: Wed, 4 Aug 2010 09:51:25 +0000\nSubject: [PATCH] instaweb: fix webrick server startup\n\nThis has been broken since commit be5347b\n(\"httpd logs in a \"$httpd_only\" subdirectory\")\n\nSigned-off-by: Eric Wong \n---\n git-instaweb.sh | 11 +++++++++--\n 1 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/git-instaweb.sh b/git-instaweb.sh\nindex b7342e2..1282395 100755\n--- a/git-instaweb.sh\n+++ b/git-instaweb.sh\n@@ -57,6 +57,13 @@ resolve_full_httpd () {\n \t\thttpd_only=\"${httpd%% *}\" # cut on first space\n \t\treturn\n \t\t;;\n+\t*webrick*)\n+\t\t# server is started by running via generated gitweb.rb in\n+\t\t# $fqgitdir/gitweb\n+\t\tfull_httpd=\"$fqgitdir/gitweb/webrick.sh\"\n+\t\thttpd_only=\"${httpd%% *}\" # cut on first space\n+\t\treturn\n+\t\t;;\n \tesac\n \n \thttpd_only=\"$(echo $httpd | cut -f1 -d' ')\"\n@@ -209,11 +216,11 @@ EOF\n \t# which assumes _ruby_ is in the user's $PATH. that's _one_\n \t# portable way to run ruby, which could be installed anywhere,\n \t# really.\n-\tcat >\"$fqgitdir/gitweb/$httpd\" <\"$fqgitdir/gitweb/$httpd.sh\" <\"$conf\" < wrote:\n> Jakub Narebski wrote:\n> > Michael Dippery writes:\n> > > I _do_ have webrick. Asking around a bit, I was told the the problem\n> > > may have been introduced in commit be5347b. Is this so? If not, any\n> > > other ideas on what may be causing the problem?\n> > \n> > To be more exact commit be5347b (git-instaweb: Put httpd logs in a\n> > \"$httpd_only\" subdirectory, 2010-05-28) by Pavan Kumar Sunkara added\n> > resolve_full_httpd before running *_config (webrick_config in this\n> > case). But resolve_full_httpd() beside setting $httpd_only needed\n> > later for functionality provided by this commit, does also setting\n> > $full_httpd and checking if given web server can be run.\n> > \n> I wouldn't mind making it more like what we do with plackup and\n> having a single Ruby script, eventually.\n\nDone in 2/3 of my instaweb patch series.\n3/3 also adds logging support to WEBrick so it should support\neverything other web servers support.\n\n> > The solution would be to either split resolve_full_httpd() into one\n> > function generating $httpd and $httpd_only, and second function\n> > generating $full_httpd and checing for web server existence, or create\n> > a separate check for 'webrick'.\n> \n> I just split out the check and started modelling things after the code\n> for plackup. Unfortunately, I haven't had any luck getting gitweb.cgi\n> to respect $GITWEB_CONFIG environment with webrick so the following\n> patch just ensures webrick is properly started, not useful.\n\nI had to use the undocumented :CGIInterpreter option of WEBrick\nalong with a shell script wrapper to pass environment variables.\n\nCGI support in WEBrick is implemented strangely: it executes a new Ruby\ninterpreter after forking (but before executing the actual gitweb.cgi).\nThus I couldn't just neuter the \"ENV.delete\" method in the webrick.rb\nfile as the child process would just restore the default behavior.\n\nPushed out to the \"webrick\" branch of git://git.bogomips.org/git-svn\n\nEric Wong (3):\n instaweb: fix WEBrick server support\n instaweb: minimize moving parts for WEBrick\n instaweb: add access+error logging for WEBrick\n\n-- \nEric Wong\n"}]}