threads / discuss / 24623

git instaweb + webrick does not work

Subject: git instaweb + webrick does not work

## tl;dr

5 messages between Aug 3, 2010 and Aug 5, 2010.

replies: 4people: 4as markdown or json

Michael Dippery· Aug 3, 2010, 21:37 UTC · lore
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

Jakub Narebski· Aug 3, 2010, 22:07 UTC · re: Michael Dippery · lore

Re: git instaweb + webrick does not work

Michael Dippery <mdippery@gmail.com> writes:
Show 12 quoted lines
> 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
Jared Hance· Aug 3, 2010, 22:34 UTC · re: Jakub Narebski · lore

[RFC/PATCH] Split resolve_full_httpd to prevent bug.

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
Eric Wong· Aug 4, 2010, 10:25 UTC · re: Jakub Narebski · lore

Re: git instaweb + webrick does not work

Jakub Narebski <jnareb@gmail.com> wrote:
Show 17 quoted lines
> 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
Eric Wong· Aug 5, 2010, 09:14 UTC · re: Eric Wong · lore

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

Eric Wong <normalperson@yhbt.net> wrote:
Show 15 quoted lines
> 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.

Show 9 quoted lines
> > 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

← back to recent threads