git instaweb + webrick does not work

git instaweb + webrick does not work

[Michael Dippery]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

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:

"webrick not found. Install webrick or use --httpd to specify another httpd daemon."

I _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?

I'm using Git v1.7.2.1 on Mac OS X 10.6.


Thanks,


----
Michael Dippery
mdippery@gmail.com | www.monkey-robot.com


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: git instaweb + webrick does not work

[Jakub Narebski]

Michael Dippery <mdippery@gmail.com> writes:

> 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:
> 
> "webrick not found. Install webrick or use --httpd to specify
> another httpd daemon."
> 
> I _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?
> 
> I'm using Git v1.7.2.1 on Mac OS X 10.6.

To be more exact commit be5347b (git-instaweb: Put httpd logs in a
"$httpd_only" subdirectory, 2010-05-28) by Pavan Kumar Sunkara added
resolve_full_httpd before running *_config (webrick_config in this
case).  But resolve_full_httpd() beside setting $httpd_only needed
later for functionality provided by this commit, does also setting
$full_httpd and checking if given web server can be run.

The `webrick' support in git-instaweb is peculiar in that webrick_conf
creates 'webrick' shell script in "$GIT_DIR/gitweb/".  The code that
checks if web server is available in resolve_full_httpd() searches
also in "$GIT_DIR/gitweb/"... but it is run before webrick_conf
function in git-instaweb has a chance to generate 'webrick' script.

The solution would be to either split resolve_full_httpd() into one
function generating $httpd and $httpd_only, and second function
generating $full_httpd and checing for web server existence, or create
a separate check for 'webrick'.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

[RFC/PATCH] Split resolve_full_httpd to prevent bug.

[Jared Hance]

resolve_full_httpd tries to find the httpd path, which can break if the
httpd configuration hasn't happened. Specifically, this occurs with
webrick.

Signed-off-by: Jared Hance <jaredhance@gmail.com>
---

WARNING: I haven't tested this patch, as I don't use git-instaweb. I
also don't have webrick, so I can't be sure this actually works.
Hopefully this can be used as a foundation for a real fix.

 git-instaweb.sh |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index b7342e2..b7950ac 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -41,7 +41,7 @@ test -z "$root" && root='@@GITWEBDIR@@'
 # any untaken local port will do...
 test -z "$port" && port=1234
 
-resolve_full_httpd () {
+resolve_httpd () {
 	case "$httpd" in
 	*apache2*|*lighttpd*|*httpd*)
 		# yes, *httpd* covers *lighttpd* above, but it is there for clarity
@@ -52,14 +52,21 @@ resolve_full_httpd () {
 		fi
 		;;
 	*plackup*)
-		# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb
-		full_httpd="$fqgitdir/gitweb/gitweb.psgi"
 		httpd_only="${httpd%% *}" # cut on first space
 		return
 		;;
 	esac
 
 	httpd_only="$(echo $httpd | cut -f1 -d' ')"
+}
+
+# please run resolve_httpd first
+resolve_full_httpd() {
+	if test $httpd = "plackup"; then
+		# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb
+		full_httpd="$fqgitdir/gitweb/gitweb.psgi"
+	fi
+
 	if case "$httpd_only" in /*) : ;; *) which $httpd_only >/dev/null 2>&1;; esac
 	then
 		full_httpd=$httpd
@@ -90,6 +97,7 @@ start_httpd () {
 	fi
 
 	# here $httpd should have a meaningful value
+	resolve_httpd
 	resolve_full_httpd
 
 	# don't quote $full_httpd, there can be arguments to it (-f)
@@ -354,6 +362,7 @@ PerlPassEnv GITWEB_CONFIG
 EOF
 	else
 		# plain-old CGI
+		resolve_httpd
 		resolve_full_httpd
 		list_mods=$(echo "$full_httpd" | sed 's/-f$/-l/')
 		$list_mods | sane_grep 'mod_cgi\.c' >/dev/null 2>&1 || \
@@ -565,7 +574,7 @@ EOF
 
 gitweb_conf
 
-resolve_full_httpd
+resolve_httpd
 mkdir -p "$fqgitdir/gitweb/$httpd_only"
 
 case "$httpd" in
@@ -590,6 +599,7 @@ webrick)
 	;;
 esac
 
+
 start_httpd
 url=http://127.0.0.1:$port
 
-- 
1.7.2

Re: git instaweb + webrick does not work

[Eric Wong]

Jakub Narebski <jnareb@gmail.com> wrote:
> Michael Dippery <mdippery@gmail.com> writes:
> > I _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?
> 
> To be more exact commit be5347b (git-instaweb: Put httpd logs in a
> "$httpd_only" subdirectory, 2010-05-28) by Pavan Kumar Sunkara added
> resolve_full_httpd before running *_config (webrick_config in this
> case).  But resolve_full_httpd() beside setting $httpd_only needed
> later for functionality provided by this commit, does also setting
> $full_httpd and checking if given web server can be run.
> 
> The `webrick' support in git-instaweb is peculiar in that webrick_conf
> creates 'webrick' shell script in "$GIT_DIR/gitweb/".  The code that
> checks if web server is available in resolve_full_httpd() searches
> also in "$GIT_DIR/gitweb/"... but it is run before webrick_conf
> function in git-instaweb has a chance to generate 'webrick' script.

I wouldn't mind making it more like what we do with plackup and
having a single Ruby script, eventually.

> The solution would be to either split resolve_full_httpd() into one
> function generating $httpd and $httpd_only, and second function
> generating $full_httpd and checing for web server existence, or create
> a separate check for 'webrick'.

I just split out the check and started modelling things after the code
for plackup.  Unfortunately, I haven't had any luck getting gitweb.cgi
to respect $GITWEB_CONFIG environment with webrick so the following
patch just ensures webrick is properly started, not useful.

Unfortunately I'm barely awake now, but I'll revisit this in ~16 hours
if nobody beats me to it.

>From 70b1773fc0bcb550788b26f2fda6ad6423960115 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Wed, 4 Aug 2010 09:51:25 +0000
Subject: [PATCH] instaweb: fix webrick server startup

This has been broken since commit be5347b
("httpd logs in a "$httpd_only" subdirectory")

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-instaweb.sh |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index b7342e2..1282395 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -57,6 +57,13 @@ resolve_full_httpd () {
 		httpd_only="${httpd%% *}" # cut on first space
 		return
 		;;
+	*webrick*)
+		# server is started by running via generated gitweb.rb in
+		# $fqgitdir/gitweb
+		full_httpd="$fqgitdir/gitweb/webrick.sh"
+		httpd_only="${httpd%% *}" # cut on first space
+		return
+		;;
 	esac
 
 	httpd_only="$(echo $httpd | cut -f1 -d' ')"
@@ -209,11 +216,11 @@ EOF
 	# which assumes _ruby_ is in the user's $PATH. that's _one_
 	# portable way to run ruby, which could be installed anywhere,
 	# really.
-	cat >"$fqgitdir/gitweb/$httpd" <<EOF
+	cat >"$fqgitdir/gitweb/$httpd.sh" <<EOF
 #!/bin/sh
 exec ruby "$fqgitdir/gitweb/$httpd.rb" \$*
 EOF
-	chmod +x "$fqgitdir/gitweb/$httpd"
+	chmod +x "$fqgitdir/gitweb/$httpd.sh"
 
 	cat >"$conf" <<EOF
 :Port: $port
-- 
Eric Wong

[RFC/PATCH 0/3] instaweb: fix and improve WEBrick support

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Jakub Narebski <jnareb@gmail.com> wrote:
> > Michael Dippery <mdippery@gmail.com> writes:
> > > I _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?
> > 
> > To be more exact commit be5347b (git-instaweb: Put httpd logs in a
> > "$httpd_only" subdirectory, 2010-05-28) by Pavan Kumar Sunkara added
> > resolve_full_httpd before running *_config (webrick_config in this
> > case).  But resolve_full_httpd() beside setting $httpd_only needed
> > later for functionality provided by this commit, does also setting
> > $full_httpd and checking if given web server can be run.
> > 
> I wouldn't mind making it more like what we do with plackup and
> having a single Ruby script, eventually.

Done in 2/3 of my instaweb patch series.
3/3 also adds logging support to WEBrick so it should support
everything other web servers support.

> > The solution would be to either split resolve_full_httpd() into one
> > function generating $httpd and $httpd_only, and second function
> > generating $full_httpd and checing for web server existence, or create
> > a separate check for 'webrick'.
> 
> I just split out the check and started modelling things after the code
> for plackup.  Unfortunately, I haven't had any luck getting gitweb.cgi
> to respect $GITWEB_CONFIG environment with webrick so the following
> patch just ensures webrick is properly started, not useful.

I had to use the undocumented :CGIInterpreter option of WEBrick
along with a shell script wrapper to pass environment variables.

CGI support in WEBrick is implemented strangely: it executes a new Ruby
interpreter after forking (but before executing the actual gitweb.cgi).
Thus I couldn't just neuter the "ENV.delete" method in the webrick.rb
file as the child process would just restore the default behavior.

Pushed out to the "webrick" branch of git://git.bogomips.org/git-svn

Eric Wong (3):
      instaweb: fix WEBrick server support
      instaweb: minimize moving parts for WEBrick
      instaweb: add access+error logging for WEBrick

-- 
Eric Wong