threads / patch / 23933

patch, 3 partsgit-instaweb: Remove pidfile after stopping web server

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

## tl;dr

9 messages between May 28, 2010 and May 31, 2010. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Jakub Narebski· May 28, 2010, 19:11 UTC · lore

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

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
Jakub Narebski· May 28, 2010, 19:11 UTC · re: Jakub Narebski · lore

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(-)
Show changes to git-instaweb.sh +1 −0
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
Jakub Narebski· May 28, 2010, 19:11 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to git-instaweb.sh +15 −2
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
Pavan Kumar Sunkara· May 28, 2010, 19:31 UTC · re: Jakub Narebski · lore

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

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

Show 13 quoted lines
> +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.

Jakub Narebski· May 28, 2010, 19:58 UTC · re: Pavan Kumar Sunkara · lore

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

On Fri, 28 May 2010, Pavan Kumar Sunkara wrote:
Show 6 quoted lines
> 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.
Show 20 quoted lines
> 
> > +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
Jakub Narebski· May 28, 2010, 19:11 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to 2 files +158 −3

Documentation/git-instaweb.txt, git-instaweb.sh

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
Eric Wong· May 29, 2010, 02:32 UTC · re: Jakub Narebski · lore

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

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
Eric Wong· May 29, 2010, 07:21 UTC · re: Eric Wong · lore

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

Eric Wong <normalperson@yhbt.net> wrote:
Show 9 quoted lines
> 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
Petr Baudis· May 31, 2010, 21:15 UTC · re: Jakub Narebski · lore

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

On Fri, May 28, 2010 at 09:11:22PM +0200, Jakub Narebski wrote:
Show 11 quoted lines
> 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>

← back to recent threads