[RFC/PATCH] git-instaweb: Add support for running gitweb via 'plackup'

[RFC/PATCH] 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.

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; it uses Plack::Runner module.  It uses HTTP::Server::PSGI as a web
server by default.

git-instaweb generates gitweb.psgi wrapper, which uses Plack::App::WrapCGI
to compile a CGI script into a PSGI application using CGI::Compile and
CGI::Emulate::PSGI, and runs it with any PSGI server as a PSGI
application.

To run git-instaweb with '--httpd=plackup', you need to have instaled
Plack core, CGI::Emulate::PSGI, CGI::Compile, and available for a Perl
script (e.g. by setting PERL5LIB environment variable).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Compared to earlier version (only mentioned, but not sent to git
mailing list, if I remember correctly), instead of configuring plackup
from command line (by modifying $httpd variable), the configuration is
embedded in generated gitweb.psgi file.  Also instead of using plackup,
the generated gitweb.psgi is made into standalone app, running server
(so $full_httpd is full path to gitweb.psgi).

RFC is because when when running

  $ ./git-instaweb --httpd=plackup --browser=lynx

I get the following error

  Looking up 127.0.0.1:1234
  Making HTTP connection to 127.0.0.1:1234
  Alert!: Unable to connect to remote host.

  lynx: Can't access startfile http://127.0.0.1:1234/
  http://127.0.0.1:1234

But running 'lynx http://127.0.0.1:1234/' after this works correctly
without problems.  Running './.git/gitweb/gitweb.psgi' doesn't cause
any problems either.  WTF?

HELP!!!

 Documentation/git-instaweb.txt |    2 +-
 git-instaweb.sh                |   52 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 4 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 f608014..dd82955 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -46,6 +46,13 @@ resolve_full_httpd () {
 			httpd="$httpd -f"
 		fi
 		;;
+	*plackup*)
+		# Plack::Runner etc must be in $PERL5LIB of user, currently
+		# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb
+		bind=
+		full_httpd="$fqgitdir/gitweb/gitweb.psgi"
+		return
+		;;
 	esac
 
 	httpd_only="$(echo $httpd | cut -f1 -d' ')"
@@ -83,9 +90,9 @@ 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
-		$full_httpd "$fqgitdir/gitweb/httpd.conf" &
+	*mongoose*|*plackup*)
+		#The mongoose server and plackup don't have a daemon mode so we'll have to fork it
+		$full_httpd "$fqgitdir/gitweb/httpd.conf" 2>"$fqgitdir/error_log" &
 		#Save the pid before doing anything else (we'll print it later)
 		pid=$!
 
@@ -370,6 +377,42 @@ mime_types	.gz=application/x-gzip,.tar.gz=application/x-tgz,.tgz=application/x-t
 EOF
 }
 
+plackup_conf () {
+	cat > "$fqgitdir/gitweb/gitweb.psgi" <<GITWEB_PSGI_EOF
+#!$PERL
+
+# gitweb - simple web interface to track changes in git repositories
+#          PSGI wrapper (see http://plackperl.org)
+
+use strict;
+
+use Plack::Builder;
+use Plack::App::WrapCGI;
+use CGI::Emulate::PSGI 0.07; # minimum version required to work with gitweb
+
+my \$app = builder {
+	# serve static files, i.e. stylesheet, images, script
+	enable 'Static',
+		path => sub { m!\.(js|css|png)\$! && s!^/gitweb/!! },
+		root => "$fqgitdir/gitweb/";
+	# convert CGI application to PSGI app
+	Plack::App::WrapCGI->new(script => "$fqgitdir/gitweb/gitweb.cgi")->to_app;
+};
+
+# make it runnable as standalone app, like via plackup
+if (__FILE__ eq \$0) {
+	require Plack::Runner;
+
+	my \$runner = Plack::Runner->new();
+	\$runner->parse_options(qw(--port $port),
+	                       "$local" ? qw(--host 127.0.0.1) : ());
+	\$runner->run(\$app);
+}
+GITWEB_PSGI_EOF
+
+	chmod a+x "$fqgitdir/gitweb/gitweb.psgi"
+	rm -f "$fqgitdir/gitweb/gitweb.psgi.tmp" "$conf"
+}
 
 script='
 s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
@@ -419,6 +462,9 @@ webrick)
 *mongoose*)
 	mongoose_conf
 	;;
+*plackup*)
+	plackup_conf
+	;;
 *)
 	echo "Unknown httpd specified: $httpd"
 	exit 1
-- 
1.7.0.1

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

[Jakub Narebski]

On Wed, 19 May 2010, Jakub Narebski wrote:

> ---
[...]
> RFC is because when when running
> 
>   $ ./git-instaweb --httpd=plackup --browser=lynx
> 
> I get the following error
> 
>   Looking up 127.0.0.1:1234
>   Making HTTP connection to 127.0.0.1:1234
>   Alert!: Unable to connect to remote host.
> 
>   lynx: Can't access startfile http://127.0.0.1:1234/
>   http://127.0.0.1:1234
> 
> But running 'lynx http://127.0.0.1:1234/' after this works correctly
> without problems.  Running './.git/gitweb/gitweb.psgi' doesn't cause
> any problems either.  WTF?

I think I know what might be happening here.  The plackup / Plack::Runner
needs some time to setup and start running HTTP::Server::PSGI based web
server.  When it is ready to serve requests (web pages), it prints to stderr

  <server>: Accepting connections at http://<host>:<port>/

In the case of .git/gitweb/gitweb.psgi script generated by git-instaweb it
is:

  HTTP::Server::PSGI: Accepting connections at http://0:1234/

But plackup / Plack::Runner is run in background (HTTP::Server::PSGI does
not support '--daemonize'), and most probably the web browser is started
before web server is not ready.


Do you have any idea on how to synchronize those two processes, the web
server and the web client (web browser), so that web browser is run only
after web server prints that it is ready to accept connection?

TIA.

> @@ -83,9 +90,9 @@ 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
> -		$full_httpd "$fqgitdir/gitweb/httpd.conf" &
> +	*mongoose*|*plackup*)
> +		#The mongoose server and plackup don't have a daemon mode so we'll have to fork it
> +		$full_httpd "$fqgitdir/gitweb/httpd.conf" 2>"$fqgitdir/error_log" &
>  		#Save the pid before doing anything else (we'll print it later)
>  		pid=$!
>  

-- 
Jakub Narebski
Poland

[PATCHv2 (RFC)] 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.

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.

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>
---
It lives^W works!  I have added 'httpd_is_ready' function, which
ensures that server is fully started before running web browser.
Currently it is non-trivial only for '--httpd=plackup'.


Currently gitweb.psgi wrapper uses Perl she-bang line, and
Plack::Runner module (if gitweb.psgi is run as a standalone script) to
configure used PSGI server in a server-agnostic way.

Attempts which didn't work or wouldn't work:

* Configuring plackup via command line options, i.e. setting $httpd
  to e.g.

     httpd="plackup --port=$port --host=127.0.0.1"

  But this wouldn't preserve options across server restart.

* Using

    #!/usr/bin/env plackup --port=$port --host=127.0.0.1

  as a she-bang line for gitweb.psgi wrapper.  It doesn't work because
  (in most cases) system passes everything after interpreter name as
  a _single_ option; in this case it would try to run

    /usr/bin/env 'plackup --port=$port --host=127.0.0.1' ...

  It also requires for plackup to be in $PATH.

* Using

    #!$PLACKUP --port=$port --host=127.0.0.1

  as a she-bang line for gitweb.psgi wrapper, after finding full path
  to 'plackup' and putting it in $PLACKUP variable (similar to $PERL).
  It doesn't work because (in most cases) system passes everything
  after interpreter name as a _single_ option; in this case it would
  try to run

    /path/to/plackup '--port=$port --host=127.0.0.1' ...

  Well, it would work if git-instaweb is run without '--local'... 

 Documentation/git-instaweb.txt |    2 +-
 git-instaweb.sh                |   71 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 67 insertions(+), 6 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 f608014..f69cb71 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -46,6 +46,13 @@ resolve_full_httpd () {
 			httpd="$httpd -f"
 		fi
 		;;
+	*plackup*)
+		# 'plackup must be in $PATH of user, currently
+		# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb
+		bind=
+		full_httpd="$fqgitdir/gitweb/gitweb.psgi"
+		return
+		;;
 	esac
 
 	httpd_only="$(echo $httpd | cut -f1 -d' ')"
@@ -83,9 +90,9 @@ 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
-		$full_httpd "$fqgitdir/gitweb/httpd.conf" &
+	*mongoose*|*plackup*)
+		#The mongoose server and plackup don't have a daemon mode so we'll have to fork it
+		$full_httpd "$fqgitdir/gitweb/httpd.conf" 2>"$fqgitdir/error_log" &
 		#Save the pid before doing anything else (we'll print it later)
 		pid=$!
 
@@ -112,6 +119,21 @@ stop_httpd () {
 	test -f "$fqgitdir/pid" && kill $(cat "$fqgitdir/pid")
 }
 
+httpd_is_ready () {
+	case "$httpd" in
+	*plackup*)
+		echo -n "Waiting for '$httpd' to start"
+		while ! grep 'Accepting connections' "$fqgitdir/error_log" >/dev/null 2>&1
+		do
+			sleep 1
+			echo -n "."
+		done
+		echo " (done)"
+		;;
+	esac
+	true
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -370,6 +392,42 @@ mime_types	.gz=application/x-gzip,.tar.gz=application/x-tgz,.tgz=application/x-t
 EOF
 }
 
+plackup_conf () {
+	cat > "$fqgitdir/gitweb/gitweb.psgi" <<GITWEB_PSGI_EOF
+#!$PERL
+
+# gitweb - simple web interface to track changes in git repositories
+#          PSGI wrapper (see http://plackperl.org)
+
+use strict;
+
+use Plack::Builder;
+use Plack::App::WrapCGI;
+use CGI::Emulate::PSGI 0.07; # minimum version required to work with gitweb
+
+my \$app = builder {
+	# serve static files, i.e. stylesheet, images, script
+	enable 'Static',
+		path => sub { m!\.(js|css|png)\$! && s!^/gitweb/!! },
+		root => "$fqgitdir/gitweb/";
+	# convert CGI application to PSGI app
+	Plack::App::WrapCGI->new(script => "$fqgitdir/gitweb/gitweb.cgi")->to_app;
+};
+
+# make it runnable as standalone app, like via plackup
+if (__FILE__ eq \$0) {
+	require Plack::Runner;
+
+	my \$runner = Plack::Runner->new();
+	\$runner->parse_options(qw(--port $port),
+	                       "$local" ? qw(--host 127.0.0.1) : ());
+	\$runner->run(\$app);
+}
+GITWEB_PSGI_EOF
+
+	chmod a+x "$fqgitdir/gitweb/gitweb.psgi"
+	rm -f "$fqgitdir/gitweb/gitweb.psgi.tmp" "$conf"
+}
 
 script='
 s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
@@ -419,6 +477,9 @@ webrick)
 *mongoose*)
 	mongoose_conf
 	;;
+*plackup*)
+	plackup_conf
+	;;
 *)
 	echo "Unknown httpd specified: $httpd"
 	exit 1
@@ -429,7 +490,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

[RFC/PATCHv3] 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.

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.

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>
---
This is an RFC because it should be split into three patches:
* a patch which makes git-instaweb remove pid-file after stopping server,
* a patch adding and using httpd_is_ready function,
* finally, a patch adding support for 'plackup' Perl web server.

Also, it somwehat lacks documentation.


Differences from previous version (v2):
* Make 'plackup' / Plack::Runner run in 'deployment' mode, and not in
  default 'development' mode.

* Use generic httpd_is_ready function, which checks if web server is
  available by checking if it connect to INET port $port on localhost.

* Instead of running 'plackup' / Plack::Runner in the default development
  mode, and redirecting its stderr to "$fqgitdir/error_log", use the same
  files for access log and for error log as for other web servers.

* Make if possible to run 'plackup' on HTTP::Server::Simple::PSGI, by
  setting PLACK_SERVER environment variable to HTTP::Server::Simple,
  even though it sets $SIG{CHLD} to 'IGNORE', which would be
  incompatibile with gitweb (see comments in code).

* A few cleanups.


 Documentation/git-instaweb.txt |    2 +-
 git-instaweb.sh                |  102 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 99 insertions(+), 5 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 f608014..a198bd2 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -46,6 +46,13 @@ resolve_full_httpd () {
 			httpd="$httpd -f"
 		fi
 		;;
+	*plackup*)
+		# 'plackup must be in $PATH of user, currently
+		# server is started by running via generated gitweb.psgi in $fqgitdir/gitweb
+		bind=
+		full_httpd="$fqgitdir/gitweb/gitweb.psgi"
+		return
+		;;
 	esac
 
 	httpd_only="$(echo $httpd | cut -f1 -d' ')"
@@ -83,8 +90,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*)
+		#The mongoose server and plackup 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=$!
@@ -110,6 +117,20 @@ EOF
 
 stop_httpd () {
 	test -f "$fqgitdir/pid" && kill $(cat "$fqgitdir/pid")
+	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
@@ -370,6 +391,76 @@ mime_types	.gz=application/x-gzip,.tar.gz=application/x-tgz,.tgz=application/x-t
 EOF
 }
 
+plackup_conf () {
+	cat > "$fqgitdir/gitweb/gitweb.psgi" <<GITWEB_PSGI_EOF
+#!$PERL
+
+# gitweb - simple web interface to track changes in git repositories
+#          PSGI wrapper (see http://plackperl.org)
+
+use strict;
+
+use IO::Handle;
+use Plack::Builder;
+use Plack::App::WrapCGI;
+use CGI::Emulate::PSGI 0.07; # minimum version required to work with gitweb
+
+
+my \$app = builder {
+	open my \$access_log_fh, '>', "$fqgitdir/gitweb/access.log"
+		or die "Couldn't open access log '$fqgitdir/gitweb/access.log': \$!";
+	open my \$error_log_fh,  '>', "$fqgitdir/gitweb/error.log"
+		or die "Couldn't open error log '$fqgitdir/gitweb/error.log': \$!";
+
+	\$access_log_fh->autoflush(1);
+	\$error_log_fh->autoflush(1);
+
+	# 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 pipe
+	# (which in turn uses wait / waitpid).
+	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 => "$fqgitdir/gitweb/";
+	# convert CGI application to PSGI app
+	Plack::App::WrapCGI->new(script => "$fqgitdir/gitweb/gitweb.cgi")->to_app;
+};
+
+# make it runnable as standalone app, like via plackup
+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);
+}
+GITWEB_PSGI_EOF
+
+	chmod a+x "$fqgitdir/gitweb/gitweb.psgi"
+	rm -f "$conf"
+}
 
 script='
 s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
@@ -419,6 +510,9 @@ webrick)
 *mongoose*)
 	mongoose_conf
 	;;
+*plackup*)
+	plackup_conf
+	;;
 *)
 	echo "Unknown httpd specified: $httpd"
 	exit 1
@@ -429,7 +523,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: [RFC/PATCHv3] git-instaweb: Add support for running gitweb via 'plackup'

[Jakub Narebski]

On Thu, 20 May 2010, Jakub Narebski wrote:

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

A question: is this solution acceptable?  Or should the configuration
(port, address if git-instaweb was run with '--local', root) be in
a separate file.

Webrick, which is in similar situation, uses httpd.conf config file in 
YAML.  Unfortunately there is no config / serialization reader in Perl
core, so if we have to go this route either the config file would have
to be in Perl, or gitweb.psgi wrapper would have to include some simple
config file parsing.

-- 
Jakub Narebski
Poland

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

[Eric Wong]

Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 20 May 2010, Jakub Narebski wrote:
> 
> > The configuration for 'plackup' is currently embedded in generated
> > gitweb.psgi wrapper, instead of using httpd.conf ($conf).
> 
> A question: is this solution acceptable?  Or should the configuration
> (port, address if git-instaweb was run with '--local', root) be in
> a separate file.

Hi Jakub,

Yes, the psgi wrapper is acceptable and probably ideal.

My understanding is that the .psgi file is analogous to a .ru file in
the (Ruby) Rack world[1], and having only a .ru file is probably how I
would add Rack support to git-instaweb if WEBrick weren't in Ruby
core[2].

> Webrick, which is in similar situation, uses httpd.conf config file in 
> YAML.  Unfortunately there is no config / serialization reader in Perl
> core, so if we have to go this route either the config file would have
> to be in Perl, or gitweb.psgi wrapper would have to include some simple
> config file parsing.

I don't think we need parsing in the gitweb.psgi wrapper, we'll just
consider gitweb.psgi the config file.  This is also the case for
Rack .ru files, which can be made executable and have a shebang
pointing to "rackup".


[1] - I'm not familiar with Plack/PSGI at all, but I've been dabbling
      quite a bit in Rack over the past year or two.

[2] - of course since WEBrick is in Ruby core and WEBrick supports CGI
      scripts out-of-the-box (unlike Rack), adding Rack support to
      instaweb would be a waste of time.

-- 
Eric Wong

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

[Eric Wong]

Jakub Narebski <jnareb@gmail.com> wrote:
> Do you have any idea on how to synchronize those two processes, the web
> server and the web client (web browser), so that web browser is run only
> after web server prints that it is ready to accept connection?

Your polling http_is_ready() function is fine, and really the only
portable solution across all servers (and OSes).

But if your webserver lets you run arbitrary code hooks after it
binds the listen socket, what I do when writing integration tests
is have it open a FIFO for writing.

Adapted and abridged from the Rainbows! web server test suite, the
following config snippet is from the config file for the Rainbows! web
server:
---------------- rainbows.conf.rb -----------------
after_fork do |server, worker|
  # test script will block while reading from the FIFO,
  # so notify the script on the first worker we spawn
  # by opening the FIFO
  if worker.nr == 0
    File.open("/path/to/fifo", "wb") { |fp| fp.syswrite "START" }
  end
end
---------------------------------------------------

And then in the test script:
----------- my-integration-test.sh ----------------
set -e
mkfifo /path/to/fifo
# start the server
rainbows --daemonize --config-file rainbows.conf.rb

# "cat /path/to/fifo" will block until the after_fork hook is called
# in rainbows.conf.rb
test xSTART = x"$(cat /path/to/fifo)"

# server is up, run the rest of the tests...
---------------------------------------------------

-- 
Eric Wong

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

[Jakub Narebski]

On Sun, 23 May 2010, Eric Wong wrote:
> Jakub Narebski <jnareb@gmail.com> wrote:
> > On Thu, 20 May 2010, Jakub Narebski wrote:
> > 
> > > The configuration for 'plackup' is currently embedded in generated
> > > gitweb.psgi wrapper, instead of using httpd.conf ($conf).
> > 
> > A question: is this solution acceptable?  Or should the configuration
> > (port, address if git-instaweb was run with '--local', root) be in
> > a separate file.
> 
> Hi Jakub,
> 
> Yes, the psgi wrapper is acceptable and probably ideal.
> 
> My understanding is that the .psgi file is analogous to a .ru file in
> the (Ruby) Rack world[1], and having only a .ru file is probably how I
> would add Rack support to git-instaweb if WEBrick weren't in Ruby
> core[2].

Yes, .psgi file in the (Perl) PSGI/Plack world is, as I understand it,
analogous to .ru file in the (Ruby) Rack world.  PSGI/Plack was inspired
by WSGI from Python world and Rack from Ruby world.

There is no web server in Perl core, although probably HTTP::Server::Simple
and HTTP::Engine comes close.  Well, there is HTTP::Server::Brick inspired
by WEBrick...

Using PSGI/Plack (via wrapper around CGI) allows easily to serve
gitweb's static files (stylesheet, images, etc.), gives persistent
environment (although this probably doesn't matter much for
git-instaweb), and allows for easy switching between different
PSGI-capable (e.g. via PSGI handlers) web servers, including the default
standalone HTTP::Server::PSGI (aka Standalone), HTTP::Server::Simple,
preforking Starlet or Starman, currently via PLACK_SERVER environment
variable.

> > Webrick, which is in similar situation, uses httpd.conf config file in 
> > YAML.  Unfortunately there is no config / serialization reader in Perl
> > core, so if we have to go this route either the config file would have
> > to be in Perl, or gitweb.psgi wrapper would have to include some simple
> > config file parsing.
> 
> I don't think we need parsing in the gitweb.psgi wrapper, we'll just
> consider gitweb.psgi the config file.  

Well, if it is all right, I would keep it as it is now (rebased on top
of Pavan's work related to GSoC 2010).  If we would want to keep config
separate from wrapper (e.g. by installing wrappers in $(gitwebdir)
together with gitweb.cgi, but keeping config files in ~/.gitweb or
.git/gitweb), we can split it later.

> This is also the case for Rack .ru files, which can be made executable
> and have a shebang pointing to "rackup".

Well, generated gitweb.psgi wrapper could have e.g.

  #!/usr/bin/env plackup

or

  #!$PLACKUP

(after putting full path to 'plackup' in $PLACKUP) as a she-bang line
('plackup' is PSGI/Plack equivalent inspired by 'rackup')... if not for
the fact that (as I wrote earlier) different web servers supported by
'plackup' utility have different ways of configuring them.  The common
API is 'plackup' command line options.

However in most operating systems everything after the interpreter in
the she-bang line is passed as *single* option, so for example
generating first line of gitweb.psgi wrapper as

  #!/usr/bin/evn plackup --port=1234 --host=127.0.0.1

wouldn't work.  That is why I used Plack::Runner instead, the module
behind 'plackup'.

As a nice side effect now you don't need to have 'plackup' in $PATH; you
only need to have Plack::Runner in $PERL5LIB (or equivalent), but you
need to have other required modules there anyway.

> [1] - I'm not familiar with Plack/PSGI at all, but I've been dabbling
>       quite a bit in Rack over the past year or two.
> 
> [2] - of course since WEBrick is in Ruby core and WEBrick supports CGI
>       scripts out-of-the-box (unlike Rack), adding Rack support to
>       instaweb would be a waste of time.

-- 
Jakub Narebski
Poland

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

[Ævar Arnfjörð Bjarmason]

2010/5/22 Jakub Narebski <jnareb@gmail.com>:
> Webrick, which is in similar situation, uses httpd.conf config file in
> YAML.  Unfortunately there is no config / serialization reader in Perl
> core, so if we have to go this route either the config file would have
> to be in Perl, or gitweb.psgi wrapper would have to include some simple
> config file parsing.

You could easily do (pseudocode):

    unless (eval { require Config::Any; 1 }) {
        do $conf_file;
    } else {
        my $parser = Config::Any->new( ... );
         $parser->docnf( $conf_file );
     }

Then you'd read (eval) a perl config file by default, but remain
compatible with saner config formats for users prepared to install
things from the CPAN.

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

[Jakub Narebski]

On Sun, 23 May 2010, Eric Wong wrote:
> Jakub Narebski <jnareb@gmail.com> wrote:
> >
> > Do you have any idea on how to synchronize those two processes, the web
> > server and the web client (web browser), so that web browser is run only
> > after web server prints that it is ready to accept connection?
> 
> Your polling http_is_ready() function is fine, and really the only
> portable solution across all servers (and OSes).

Well, even if we use the solution below it would be good fallback for
those web servers, for which the solution below cannot be used.

> 
> But if your webserver lets you run arbitrary code hooks after it
> binds the listen socket, what I do when writing integration tests
> is have it open a FIFO for writing.

I was thinking about 'flock' function in Perl (and similar in other
languages), and '/usr/bin/flock' utility from util-linux.  But using
FIFO as a semaphore might be a better idea: it is slighly more portable,
I guess.

Anyway, Plack handlers have (undocumented!) 'server_ready' option to web
servers, containing callback (code reference).  It can be set via
parse_options method of Plack::Runner, for example.  It is used by
'plackup' to print

  $server: Accepting connections at $proto://$host:$port

in the (default) "development" mode.

> 
> Adapted and abridged from the Rainbows! web server test suite, the
> following config snippet is from the config file for the Rainbows! web
> server:
> ---------------- rainbows.conf.rb -----------------
> after_fork do |server, worker|
>   # test script will block while reading from the FIFO,
>   # so notify the script on the first worker we spawn
>   # by opening the FIFO
>   if worker.nr == 0
>     File.open("/path/to/fifo", "wb") { |fp| fp.syswrite "START" }
>   end
> end
> ---------------------------------------------------
> 
> And then in the test script:
> ----------- my-integration-test.sh ----------------
> set -e
> mkfifo /path/to/fifo
> # start the server
> rainbows --daemonize --config-file rainbows.conf.rb
> 
> # "cat /path/to/fifo" will block until the after_fork hook is called
> # in rainbows.conf.rb
> test xSTART = x"$(cat /path/to/fifo)"
> 
> # server is up, run the rest of the tests...
> ---------------------------------------------------

I wonder how portable is that technique...

It certainly could be done in gitweb.psgi; the question is if it is it
worth it...

-- 
Jakub Narebski
Poland

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

[Jakub Narebski]

On Sun, 23 May 2010, Ævar Arnfjörð Bjarmason wrote:
> 2010/5/22 Jakub Narebski <jnareb@gmail.com>:
> >
> > Webrick, which is in similar situation, uses httpd.conf config file in
> > YAML.  Unfortunately there is no config / serialization reader in Perl
> > core, so if we have to go this route either the config file would have
> > to be in Perl, or gitweb.psgi wrapper would have to include some simple
> > config file parsing.
> 
> You could easily do (pseudocode):
> 
>     unless (eval { require Config::Any; 1 }) {
>     	do $conf_file;
>     } else {
>     	my $parser = Config::Any->new( ... );
>     	$parser->docnf( $conf_file );
>     }
> 
> Then you'd read (eval) a perl config file by default, but remain
> compatible with saner config formats for users prepared to install
> things from the CPAN.

I was thinking more about something like that:

  my @opts = qw(--env deployment);
  while (<>) {
  	chomp;
  	next unless $_;  # skip empty lines
  	next if /^\s*#/; # skip comments
  	if (/^\s*(.*?)\s*=\s*(.*)\s*$/) {
  		push @opts, '--'.lc($1), $2;
  	}
  }

where httpd.conf would look like this:

  port = 1234
  host = 127.0.0.1

Anyway, if such code is to be be added, it would be added in a separate
commit.
-- 
Jakub Narebski
Poland

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

[Jakub Narebski]

On Thu, 20 May 2010, Jakub Narebski wrote:
> ---
> This is an RFC because it should be split into three patches:
> * a patch which makes git-instaweb remove pid-file after stopping server,
> * a patch adding and using httpd_is_ready function,
> * finally, a patch adding support for 'plackup' Perl web server.

I'll wait with resending it as a patch series for Pavan work on
git-instaweb to be in, to not force him to do additional work on
the new 'plackup'-related code in git-instaweb.

> 
> Also, it somwehat lacks documentation.

Hmmm... where should the documentation about requirements etc.
of specific supported by git-instaweb web servers be added?

-- 
Jakub Narebski
Poland