patch, 4 partsgit-instaweb: Three fixes and one improvement

Actually only the first patch is a proper bugfix for issue noticed by Tadeusz Sośnierz ('tadzik' on github, https://github.com/tadzik) in GitHub private message; second and third are hardening git-instaweb against differences in configuration and environment.

Junio, could you at least apply the first one before 1.7.4? This is a fix for a real bug, if for not often used "git instaweb --httpd=plackup".

tadzik, does the first patch fixes the issue you noticed?

Jakub Narebski (4):
  git-instaweb: Fix issue with static files for 'plackup' server
  git-instaweb: Static files are under "static/" in gitweb_conf
  git-instaweb: Add checks that web server can be started and is
    started
  git-instaweb: Use "git-instaweb" as sitename (for page titles)

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

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 <tadzikes@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The regexp is probably too strict: qr{^/static/} should be enough,
but I didn't want to change too much at once.

This bug was not noticed because we don't have any test for git-instaweb, not mentioning tests for all web servers supported. And the fact that I was checking "git instaweb -httpd=plackup" against gitweb.cgi built with custom configuration (including the fact that GITWEB_CSS="/gitweb/static/gitweb.css").

tadzik, does that fix the issue you noticed?

Junio, could you apply this before 1.7.4?  Thanks in advance.

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

Jakub Narebski <jnareb@gmail.com> writes:

Hmph, I was kind of hoping that I can issue 1.7.3.5 tonight...

Jakub Narebski <jnareb@gmail.com> writes:

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.

On Thu, 30 Dec 2010, Junio C Hamano wrote:

[...]

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

The variables used to compose links for gitweb static files, i.e. @stylesheets / $stylesheet, $logo, etc. are configurable via GITWEB_CSS, GITWEB_LOGO, etc. gitweb build configuration variables. The installation directory of those static files is currently always set to $(gitwebdir)/static. Thus git-instaweb could rely on the place where those files are installed, but not on $logo, $favicon etc. variables.

Set @stylesheets, $logo, $favicon and $javascript to their default values in gitweb config file for git-instaweb (in gitweb_conf function), so that web servers invoked by git-instaweb (for example Plack::Middleware::Static component for 'plackup' web server) can correctly serve links to those static files.

We assume here that git-instaweb uses modern gitweb with names of gitweb config variables as mentioned above.

Note that we had to take care that possible optional minimizations of stylesheet and of javascript is correctly taken into account.

This is continuation of fixup for issue noticed by Tadeusz Sośnierz with --httpd=plackup and gitweb stylesheet (CSS).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The subject (commit description) is not the best, but that is what I
was able to come up with.

This doesn't matter for default configuration and setup, but it allows me to use my custom gitweb build configuration without any changes in result.

 git-instaweb.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Better to check that "$root/gitweb.cgi" exists, and that web server started, rather than have (for example) the following error message

  $ git instaweb --httpd=plackup --browser=mozilla
  Waiting for 'plackup' to start .../usr/share/gitweb/gitweb.cgi:
  No such file or directory at <somewhere>/perl5/CGI/Compile.pm line 97.
  ........

and keep waiting in httpd_is_ready for gitweb.cgi which would not start.

Note that the first check is not strictly necessary, but checking separately that gitweb.cgi script can be found at expected place leads to better error message.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It can be considered bugfix, or it can be considered hardening of
git-instaweb (making it more resilent).

 git-instaweb.sh |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

The default is to use "$ENV{'SERVER_NAME'} Git" or "Untitled Git"; use "git-instaweb" instead.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is new feature, and doesn't need to be in 1.7.4

It would be an RFC, if not for the fact that it is such trivial change.

 git-instaweb.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Jakub Narebski <jnareb@gmail.com> wrote:

Thanks Jakub.  This series all looks reasonable to me (untested).