[PATCH 0/3] git-instaweb: Support for 'plackup' and improvements

[PATCH 0/3] git-instaweb: Support for 'plackup' and improvements

[Jakub Narebski]

This series is based on latest GSoC 2010 work by Pavan Kumar Sunkara,
on commit 72855f7 
  (git-instaweb: Configure it to work with new gitweb structure, 2010-05-21)
in the 'master' branch of git://repo.or.cz/git/gsoc2010-gitweb.git

The relevant patches were lately send to git mailing list as 4-patch
series:
 [PATCH GSoC 1/4] gitweb: Move static files into seperate subdirectory
 [PATCH GSoC 2/4] gitweb: Set default destination directory for
                  installing gitweb in Makefile
 [PATCH GSoC 3/4] git-instaweb: Put httpd logs in a "$httpd_only"
                  subdirectory
 [PATCH GSoC 4/4] git-instaweb: Configure it to work with new gitweb
                  structure
The first patch series is available as
  Message-Id: <1275027952-5057-4-git-send-email-pavan.sss1991@gmail.com>
  http://thread.gmane.org/gmane.comp.version-control.git/147913

The following patches were send as non-chained reply to the first
patch (without cover letter).


This series adds support for 'plackup' web server (PSGI/Plack based
server script) to git-instaweb.  Because the web server (by default
pure-Perl HTTP::Server::PSGI aka PLACK_SERVER=Standalone) can take a
while to start, it required adding waiting for web server to be ready
before starting web browser.

This series also contains somewhat unrelated fixup, namely that
git-instaweb removes pidfile after stopping web server.  This is
probably important only to web servers which do not have daemon mode,
and are "daemonized" (which includes generating pidfile) by
git-instaweb itself.

Shortlog:
~~~~~~~~~
Jakub Narebski (3):
  git-instaweb: Remove pidfile after stopping web server
  git-instaweb: Wait for server to start before running web browser
  git-instaweb: Add support for running gitweb via 'plackup'

Table of contents:
~~~~~~~~~~~~~~~~~~
 [PATCH 1/3] git-instaweb: Remove pidfile after stopping web server
 [PATCH 2/3] git-instaweb: Wait for server to start before running web browser
 [PATCH 3/3] git-instaweb: Add support for running gitweb via 'plackup'

Diffstat:
~~~~~~~~~
 Documentation/git-instaweb.txt |    2 +-
 git-instaweb.sh                |  177 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 174 insertions(+), 5 deletions(-)

-- 
Jakub Narebski

[PATCH 1/3] git-instaweb: Remove pidfile after stopping web server

[Jakub Narebski]

This way running e.g. "git instaweb" after "git instaweb --stop" would
not try to kill already stopped web server.

This is probably important only for those web servers that are
"daemonized" by git-instaweb itself, i.e. for those where it is
git-instaweb that creates pidfile.  Currently it is includes only
'mongoose' web server, but it would also include 'plackup' web server
(added in later commit).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 git-instaweb.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 5c700b6..a8c5dc0 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -114,6 +114,7 @@ EOF
 
 stop_httpd () {
 	test -f "$fqgitdir/pid" && kill $(cat "$fqgitdir/pid")
+	rm -f "$fqgitdir/pid"
 }
 
 while test $# != 0
-- 
1.7.0.1

[PATCH 2/3] git-instaweb: Wait for server to start before running web browser

[Jakub Narebski]

Add generic httpd_is_ready subroutine, which busy-waits for web server to
be started, by checking if $port is opened on localhost.  This is used to
avoid situation where web browser is started before web server is ready to
accept connection, and fails.

It uses IO::Socket::INET module, which is core Perl module since v5.6.0.


Alternate solution, possible for those web servers that can run arbitrary
code hooks after they bind the listen socket (after they start accepting
connections), would be to use some kind of blocking mechanism: FIFO or
lockfile, see
  http://thread.gmane.org/gmane.comp.version-control.git/147337/focus=147566

This can be always added later, as a web server specific branch in
httpd_is_ready function.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This was required to test support for 'plackup' web server in git-instaweb
(the next patch in this series), because default pure-Perl web server used
by Plack, namely HTTP::Server::PSGI, can take a while to start.

 git-instaweb.sh |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index a8c5dc0..dc8478f 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -117,6 +117,19 @@ stop_httpd () {
 	rm -f "$fqgitdir/pid"
 }
 
+httpd_is_ready () {
+	"$PERL" -MIO::Socket::INET -e "
+local \$| = 1; # turn on autoflush
+exit if (IO::Socket::INET->new('127.0.0.1:$port'));
+print 'Waiting for \'$httpd\' to start ..';
+do {
+	print '.';
+	sleep(1);
+} until (IO::Socket::INET->new('127.0.0.1:$port'));
+print qq! (done)\n!;
+"
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -414,7 +427,7 @@ start_httpd
 url=http://127.0.0.1:$port
 
 if test -n "$browser"; then
-	git web--browse -b "$browser" $url || echo $url
+	httpd_is_ready && git web--browse -b "$browser" $url || echo $url
 else
-	git web--browse -c "instaweb.browser" $url || echo $url
+	httpd_is_ready && git web--browse -c "instaweb.browser" $url || echo $url
 fi
-- 
1.7.0.1

Re: [PATCH 2/3] git-instaweb: Wait for server to start before running web browser

[Pavan Kumar Sunkara]

> It uses IO::Socket::INET module, which is core Perl module since v5.6.0.

Core module means it will be installed by default in v5.6. But what
happens in case of lower versions of perl ?

> +httpd_is_ready () {
> +       "$PERL" -MIO::Socket::INET -e "
> +local \$| = 1; # turn on autoflush
> +exit if (IO::Socket::INET->new('127.0.0.1:$port'));
> +print 'Waiting for \'$httpd\' to start ..';
> +do {
> +       print '.';
> +       sleep(1);
> +} until (IO::Socket::INET->new('127.0.0.1:$port'));
> +print qq! (done)\n!;
> +"
> +}
> +

One of the solution is to add a web server specific branch in httpd_is_ready().
So, if the server is plackup it loads the module and checks the port,
if not it will just continue.

Just an Idea for a common usage.

Thanks,
Pavan.

Re: [PATCH 2/3] git-instaweb: Wait for server to start before running web browser

[Jakub Narebski]

On Fri, 28 May 2010, Pavan Kumar Sunkara wrote:
> Jakub Narebski wrote;
> >
> > It uses IO::Socket::INET module, which is core Perl module since v5.6.0.
> 
> Core module means it will be installed by default in v5.6. But what
> happens in case of lower versions of Perl?

Gitweb itself requires even higher version of Perl for its utf-8
support: at least v5.8.0 for Encode module, and IIRC at least v5.8.3
or even v5.8.6 for correct handling of utf-8.

Perl v5.8.6 was released in 2006.

> 
> > +httpd_is_ready () {
> > +       "$PERL" -MIO::Socket::INET -e "
> > +local \$| = 1; # turn on autoflush
> > +exit if (IO::Socket::INET->new('127.0.0.1:$port'));
> > +print 'Waiting for \'$httpd\' to start ..';
> > +do {
> > +       print '.';
> > +       sleep(1);
> > +} until (IO::Socket::INET->new('127.0.0.1:$port'));
> > +print qq! (done)\n!;
> > +"
> > +}
> > +
> 
> One of the solution is to add a web server specific branch in httpd_is_ready().
> So, if the server is plackup it load the module and checks the port,
> if not it will just continue.
> 
> Just an idea for a common usage.

The above solution is universal, and works for any web server.  

We can add web server specific branch in httpd_is_ready() to use e.g. 
server_startup hook (and some blocking mechanism) for server(s) which
support it.

-- 
Jakub Narebski
Poland

[PATCHv4 3/3] git-instaweb: Add support for running gitweb via 'plackup'

[Jakub Narebski]

PSGI is an interface between Perl web applications and web servers, and
Plack is a Perl module and toolkit that contains PSGI middleware, helpers
and adapters to web servers; see http://plackperl.org

PSGI and Plack are inspired by Python's WSGI and Ruby's Rack (and
probably JavaScript's Jack/JSGI).

Plack core distribution includes HTTP::Server::PSGI, a reference PSGI
standalone web server implementation.  'plackup' is a command line
launcher to run PSGI applications from command line, connecting web
app to a web server via Plack::Runner module.  By default it uses
HTTP::Server::PSGI as a web server.

git-instaweb generates gitweb.psgi wrapper (in $GIT_DIR/gitweb).  This
wrapper uses Plack::App::WrapCGI to compile gitweb.cgi (which is a CGI
script) into a PSGI application using CGI::Compile and CGI::Emulate::PSGI.
git-instaweb then runs this wrapper, using by default HTTP::Server::PSGI
standalone Perl server, via Plack::Runner.

The configuration for 'plackup' is currently embedded in generated
gitweb.psgi wrapper, instead of using httpd.conf ($conf).


To run git-instaweb with '--httpd=plackup', you need to have instaled
Plack core, CGI::Emulate::PSGI, CGI::Compile.  Those modules have to be
available for Perl scripts (which can be done for example by setting
PERL5LIB environment variable).  This is currently not documented.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Differences from previous version (v3):
* resolve_full_httpd sets also $httpd_only, in addition to $full_httpd
* httpd_is_ready is moved to separate patch, earlier in series
* gitweb.psgi now includes (probably unnecessary) mimetype mapping
* gitweb.psgi puts access.log and error.log in $httpd_only subdirectory,
  like the rest of web servers (after 3rd patch by Pavan Kumar Sunkara)
* gitweb.psgi logs in error.log also build time warnings
* root dir in gitweb.psgi  for gitweb.cgi and for static files is $root,
  not $fqgitdir/gitweb (after 4th patch by Pavan Kumar Sunkara)

http://thread.gmane.org/gmane.comp.version-control.git/147337/focus=147415

 Documentation/git-instaweb.txt |    2 +-
 git-instaweb.sh                |  159 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 158 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-instaweb.txt b/Documentation/git-instaweb.txt
index a1f17df..2c3c4d2 100644
--- a/Documentation/git-instaweb.txt
+++ b/Documentation/git-instaweb.txt
@@ -29,7 +29,7 @@ OPTIONS
 	The HTTP daemon command-line that will be executed.
 	Command-line options may be specified here, and the
 	configuration file will be added at the end of the command-line.
-	Currently apache2, lighttpd, mongoose and webrick are supported.
+	Currently apache2, lighttpd, mongoose, plackup and webrick are supported.
 	(Default: lighttpd)
 
 -m::
diff --git a/git-instaweb.sh b/git-instaweb.sh
index dc8478f..8b6b02e 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -50,6 +50,12 @@ resolve_full_httpd () {
 			httpd="$httpd -f"
 		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' ')"
@@ -87,8 +93,8 @@ start_httpd () {
 
 	# don't quote $full_httpd, there can be arguments to it (-f)
 	case "$httpd" in
-	*mongoose*)
-		#The mongoose server doesn't have a daemon mode so we'll have to fork it
+	*mongoose*|*plackup*)
+		#These servers don't have a daemon mode so we'll have to fork it
 		$full_httpd "$fqgitdir/gitweb/httpd.conf" &
 		#Save the pid before doing anything else (we'll print it later)
 		pid=$!
@@ -390,6 +396,152 @@ mime_types	.gz=application/x-gzip,.tar.gz=application/x-tgz,.tgz=application/x-t
 EOF
 }
 
+plackup_conf () {
+	# generate a standalone 'plackup' server script in $fqgitdir/gitweb
+	# with embedded configuration; it does not use "$conf" file
+	cat > "$fqgitdir/gitweb/gitweb.psgi" <<EOF
+#!$PERL
+
+# gitweb - simple web interface to track changes in git repositories
+#          PSGI wrapper and server starter (see http://plackperl.org)
+
+use strict;
+
+use IO::Handle;
+use Plack::MIME;
+use Plack::Builder;
+use Plack::App::WrapCGI;
+use CGI::Emulate::PSGI 0.07; # minimum version required to work with gitweb
+
+# mimetype mapping (from lighttpd_conf)
+Plack::MIME->add_type(
+	".pdf"          =>      "application/pdf",
+	".sig"          =>      "application/pgp-signature",
+	".spl"          =>      "application/futuresplash",
+	".class"        =>      "application/octet-stream",
+	".ps"           =>      "application/postscript",
+	".torrent"      =>      "application/x-bittorrent",
+	".dvi"          =>      "application/x-dvi",
+	".gz"           =>      "application/x-gzip",
+	".pac"          =>      "application/x-ns-proxy-autoconfig",
+	".swf"          =>      "application/x-shockwave-flash",
+	".tar.gz"       =>      "application/x-tgz",
+	".tgz"          =>      "application/x-tgz",
+	".tar"          =>      "application/x-tar",
+	".zip"          =>      "application/zip",
+	".mp3"          =>      "audio/mpeg",
+	".m3u"          =>      "audio/x-mpegurl",
+	".wma"          =>      "audio/x-ms-wma",
+	".wax"          =>      "audio/x-ms-wax",
+	".ogg"          =>      "application/ogg",
+	".wav"          =>      "audio/x-wav",
+	".gif"          =>      "image/gif",
+	".jpg"          =>      "image/jpeg",
+	".jpeg"         =>      "image/jpeg",
+	".png"          =>      "image/png",
+	".xbm"          =>      "image/x-xbitmap",
+	".xpm"          =>      "image/x-xpixmap",
+	".xwd"          =>      "image/x-xwindowdump",
+	".css"          =>      "text/css",
+	".html"         =>      "text/html",
+	".htm"          =>      "text/html",
+	".js"           =>      "text/javascript",
+	".asc"          =>      "text/plain",
+	".c"            =>      "text/plain",
+	".cpp"          =>      "text/plain",
+	".log"          =>      "text/plain",
+	".conf"         =>      "text/plain",
+	".text"         =>      "text/plain",
+	".txt"          =>      "text/plain",
+	".dtd"          =>      "text/xml",
+	".xml"          =>      "text/xml",
+	".mpeg"         =>      "video/mpeg",
+	".mpg"          =>      "video/mpeg",
+	".mov"          =>      "video/quicktime",
+	".qt"           =>      "video/quicktime",
+	".avi"          =>      "video/x-msvideo",
+	".asf"          =>      "video/x-ms-asf",
+	".asx"          =>      "video/x-ms-asf",
+	".wmv"          =>      "video/x-ms-wmv",
+	".bz2"          =>      "application/x-bzip",
+	".tbz"          =>      "application/x-bzip-compressed-tar",
+	".tar.bz2"      =>      "application/x-bzip-compressed-tar",
+	""              =>      "text/plain"
+);
+
+my \$app = builder {
+	# to be able to override \$SIG{__WARN__} to log build time warnings
+	use CGI::Carp; # it sets \$SIG{__WARN__} itself
+
+	my \$logdir = "$fqgitdir/gitweb/$httpd_only";
+	open my \$access_log_fh, '>', "\$logdir/access.log"
+		or die "Couldn't open access log '\$logdir/access.log': \$!";
+	open my \$error_log_fh,  '>', "\$logdir/error.log"
+		or die "Couldn't open error log '\$logdir/error.log': \$!";
+
+	\$access_log_fh->autoflush(1);
+	\$error_log_fh->autoflush(1);
+
+	# redirect build time warnings to error.log
+	\$SIG{'__WARN__'} = sub {
+		my \$msg = shift;
+		# timestamp warning like in CGI::Carp::warn
+		my \$stamp = CGI::Carp::stamp();
+		\$msg =~ s/^/\$stamp/gm;
+		print \$error_log_fh \$msg;
+	};
+
+	# write errors to error.log, access to access.log
+	enable 'AccessLog',
+		format => "combined",
+		logger => sub { print \$access_log_fh @_; };
+	enable sub {
+		my \$app = shift;
+		sub {
+			my \$env = shift;
+			\$env->{'psgi.errors'} = \$error_log_fh;
+			\$app->(\$env);
+		}
+	};
+	# gitweb currently doesn't work with $SIG{CHLD} set to 'IGNORE',
+	# because it uses 'close $fd or die...' on piped filehandle $fh
+	# (which causes the parent process to wait for child to finish).
+	enable_if { \$SIG{'CHLD'} eq 'IGNORE' } sub {
+		my \$app = shift;
+		sub {
+			my \$env = shift;
+			local \$SIG{'CHLD'} = 'DEFAULT';
+			local \$SIG{'CLD'}  = 'DEFAULT';
+			\$app->(\$env);
+		}
+	};
+	# serve static files, i.e. stylesheet, images, script
+	enable 'Static',
+		path => sub { m!\.(js|css|png)\$! && s!^/gitweb/!! },
+		root => "$root/",
+		encoding => 'utf-8'; # encoding for 'text/plain' files
+	# convert CGI application to PSGI app
+	Plack::App::WrapCGI->new(script => "$root/gitweb.cgi")->to_app;
+};
+
+# make it runnable as standalone app,
+# like it would be run via 'plackup' utility
+if (__FILE__ eq \$0) {
+	require Plack::Runner;
+
+	my \$runner = Plack::Runner->new();
+	\$runner->parse_options(qw(--env deployment --port $port),
+	                       "$local" ? qw(--host 127.0.0.1) : ());
+	\$runner->run(\$app);
+}
+__END__
+EOF
+
+	chmod a+x "$fqgitdir/gitweb/gitweb.psgi"
+	# configuration is embedded in server script file, gitweb.psgi
+	rm -f "$conf"
+}
+
 gitweb_conf() {
 	cat > "$fqgitdir/gitweb/gitweb_config.perl" <<EOF
 #!/usr/bin/perl
@@ -417,6 +569,9 @@ webrick)
 *mongoose*)
 	mongoose_conf
 	;;
+*plackup*)
+	plackup_conf
+	;;
 *)
 	echo "Unknown httpd specified: $httpd"
 	exit 1
-- 
1.7.0.1

Re: [PATCHv4 3/3] git-instaweb: Add support for running gitweb via 'plackup'

[Eric Wong]

Jakub Narebski <jnareb@gmail.com> wrote:
> +	open my \$access_log_fh, '>', "\$logdir/access.log"
> +		or die "Couldn't open access log '\$logdir/access.log': \$!";
> +	open my \$error_log_fh,  '>', "\$logdir/error.log"
> +		or die "Couldn't open error log '\$logdir/error.log': \$!";

I believe '>>' (append) is preferable for log files and matches the
behavior of all other servers.  Other than that everything else looks
good.  I can make that change myself + Ack + push for Junio.

-- 
Eric Wong

Re: [PATCHv4 3/3] git-instaweb: Add support for running gitweb via 'plackup'

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Jakub Narebski <jnareb@gmail.com> wrote:
> > +	open my \$access_log_fh, '>', "\$logdir/access.log"
> > +		or die "Couldn't open access log '\$logdir/access.log': \$!";
> > +	open my \$error_log_fh,  '>', "\$logdir/error.log"
> > +		or die "Couldn't open error log '\$logdir/error.log': \$!";
> 
> I believe '>>' (append) is preferable for log files and matches the
> behavior of all other servers.  Other than that everything else looks
> good.  I can make that change myself + Ack + push for Junio.

Consider this series acked with the above change, since this series
depends on changes not ready for mainline yet, I've pushed my acks to
the "instaweb" branch at git://git.bogomips.org/git-svn.

-- 
Eric Wong

Re: [PATCH 0/3] git-instaweb: Support for 'plackup' and improvements

[Petr Baudis]

On Fri, May 28, 2010 at 09:11:22PM +0200, Jakub Narebski wrote:
> This series adds support for 'plackup' web server (PSGI/Plack based
> server script) to git-instaweb.  Because the web server (by default
> pure-Perl HTTP::Server::PSGI aka PLACK_SERVER=Standalone) can take a
> while to start, it required adding waiting for web server to be ready
> before starting web browser.
> 
> This series also contains somewhat unrelated fixup, namely that
> git-instaweb removes pidfile after stopping web server.  This is
> probably important only to web servers which do not have daemon mode,
> and are "daemonized" (which includes generating pidfile) by
> git-instaweb itself.

I did not test the series, but looking over it, all appears sane, so

Acked-by: Petr Baudis <pasky@suse.cz>