Subject: Re: [BUGFIX PATCH 1/4] git-instaweb: Fix issue with static files for 'plackup' server

From: Jakub Narebski <[email-masked]>
Date: Thu, 30 Dec 2010 01:40:06 +0100

On Thu, 30 Dec 2010, Junio C Hamano wrote:
> Jakub Narebski <[email-masked]> writes:
> 
> > The default (in gitweb/Makefile) is to use relative paths for gitweb
> > static files, e.g. "static/gitweb.css" for GITWEB_CSS.  But the
> > configuration for Plack::Middleware::Static in plackup_conf assumed
> > that static files must be absolute paths starting with "/gitweb/"
> > prefix which had to be stripped, e.g. "/gitweb/static/gitweb.css".
> > This in turn caused web server run by "git instaweb --httpd=plackup"
> > to not access static files (e.g. CSS) correctly.
> >
> > This is a minimal fixup, making 'plackup' web server in git-instaweb
> > work with default gitweb build configuration.
> >
> > Reported-by: Tadeusz Sośnierz <[email-masked]>
> > Signed-off-by: Jakub Narebski <[email-masked]>
> > ---
> > The regexp is probably too strict: qr{^/static/} should be enough,
> > but I didn't want to change too much at once.
[...]
> > diff --git a/git-instaweb.sh b/git-instaweb.sh
> > index 10fcebb..bb57d81 100755
> > --- a/git-instaweb.sh
> > +++ b/git-instaweb.sh
> > @@ -549,7 +549,7 @@ my \$app = builder {
> >  	};
> >  	# serve static files, i.e. stylesheet, images, script
> >  	enable 'Static',
> > -		path => sub { m!\.(js|css|png)\$! && s!^/gitweb/!! },
> > +		path => qr{^/static/.*(?:js|css|png)\$},
> 
> I wonder if you meant qr{^/static/.*\\.(?:js|css|png)\$} here, to make
> sure that these three tokens are file suffixes, not just random
> substring.

Yes, it should be qr{^/static/.*\\.(?:js|css|png)\$} though as I said:
"The regexp is probably too strict: qr{^/static/} should be enough,"

-- 
Jakub Narebski
Poland