threads / bug / 24954

git instaweb for Debian squeeze (Re: Bug#592733: git instaweb broken)

Subject: git instaweb for Debian squeeze (Re: Bug#592733: git instaweb broken)

## tl;dr

14 messages between Sep 2, 2010 and Sep 4, 2010.

replies: 13people: 3as markdown or json

Jonathan Nieder· Sep 2, 2010, 22:12 UTC · lore

Hi again,

Uwe Kleine-König wrote:

> when I start git instaweb on a git repository, I get a crippled view
> (no style sheets, no images) that doesn't include the current project.

Ok, so the proper fix is v1.7.2-rc0~55^2~3, but that seems too dramatic to try at the moment for squeeze (for which the plan is to basically go with gitweb and instaweb 1.7.1). Does this workaround help?

Eric, do you think it is worth duplicating the logo and favicon within pre-v1.7.2 instaweb? The script already takes 252 KiB, so an extra 10 KiB might not be such a big deal.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 0c6e103..95d3d0a 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -387,7 +387,11 @@ script='
 s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
 s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
 s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
-s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;'
+s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
+s#(my|our) \$logo =.*#$1 \$logo = "/usr/share/gitweb/git-logo.png";#;
+s#(my|our) \$favicon =.*#$1 \$favicon = "/usr/share/gitweb/git-favicon.png";#;
+s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'
+
 
 gitweb_cgi () {
 	cat > "$1.tmp" <<\EOFGITWEB

Eric Wong· Sep 2, 2010, 22:36 UTC · re: Jonathan Nieder · lore

Re: git instaweb for Debian squeeze (Re: Bug#592733: git instaweb broken)

Jonathan Nieder <jrnieder@gmail.com> wrote:

> Uwe Kleine-König wrote:
> > when I start git instaweb on a git repository, I get a crippled view
> > (no style sheets, no images) that doesn't include the current project.

> Eric, do you think it is worth duplicating the logo and favicon within
> pre-v1.7.2 instaweb?  The script already takes 252 KiB, so an extra
> 10 KiB might not be such a big deal.

The stylesheet is important and should be fixed. I don't care about the logo and favicon (warning: I've never been a fan of logos/icons, needless noise IMHO), but if you're willing to help maintain it then it's fine by me.

-- 
Eric Wong

Jonathan Nieder· Sep 2, 2010, 23:52 UTC · re: Eric Wong · lore

[PATCH/RFC maint-1.7.1 0/4] instaweb 1.7.1 fixes for Debian squeeze

Eric Wong wrote:

>> Uwe Kleine-König wrote:
>>> when I start git instaweb on a git repository, I get a crippled view
>>> (no style sheets, no images) that doesn't include the current project.

[...]

> The stylesheet is important and should be fixed.  I don't care about the
> logo and favicon (warning: I've never been a fan of logos/icons,
> needless noise IMHO), but if you're willing to help maintain it then
> it's fine by me.

Thanks, Eric; that makes sense.  How about this?

The purpose of this series is the first patch, which allows old-fashioned instaweb to run even if /etc/gitweb.conf specifies some layout it does not expect.

I suspect this problem is already fixed in maint by v1.7.2-rc0~55^2~3 (git-instaweb: Configure it to work with new gitweb structure, 2010-05-28; thanks, Pavan!), but users that do not like gitweb version numbers with a 2 in them will not be able to use that.

Patches 2-4 fix some spurious 404 errors due to missing images, noticed while debugging.

Patches are against Junio's maint-1.7.1.

Jonathan Nieder (4):
  instaweb: ignore GITWEB_CONFIG_SYSTEM
  gitweb: skip logo in atom feed when there is none
  gitweb: make logo optional
  instaweb: disable logo and favicon by default

 git-instaweb.sh    |    6 +++++-
 gitweb/gitweb.perl |   12 +++++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

-- 
1.7.2.2

Jonathan Nieder· Sep 2, 2010, 23:54 UTC · re: Jonathan Nieder · lore

[PATCH 1/4] instaweb: ignore GITWEB_CONFIG_SYSTEM

The configuration in /etc/gitweb.conf might specify any old layout; in particular, it is likely not to be identical to that which git instaweb sets up. Noticed by Uwe Kleine-König.

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 git-instaweb.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index f608014..6279f4d 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -375,7 +375,8 @@ script='
 s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
 s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
 s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
-s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;'
+s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
+s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'
 
 gitweb_cgi () {
 	cat > "$1.tmp" <<\EOFGITWEB

-- 
1.7.2.2

Junio C Hamano· Sep 3, 2010, 17:23 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/4] instaweb: ignore GITWEB_CONFIG_SYSTEM

Jonathan Nieder <jrnieder@gmail.com> writes:

> The configuration in /etc/gitweb.conf might specify any old
> layout; in particular, it is likely not to be identical to that
> which git instaweb sets up.  Noticed by Uwe Kleine-König.

I am not sure if this is a reasonable change by reading the above. In some cases, the file may stale, but in other cases, it may specify what the site administrator wants its users to be using, no?

Show 19 quoted lines

>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Eric Wong <normalperson@yhbt.net>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  git-instaweb.sh |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/git-instaweb.sh b/git-instaweb.sh
> index f608014..6279f4d 100755
> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
> @@ -375,7 +375,8 @@ script='
>  s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
>  s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
>  s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
> -s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;'
> +s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
> +s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'

Perhaps moving the last ' to its own line would help maintaining the script in the longer term?

Show 5 quoted lines

>  
>  gitweb_cgi () {
>  	cat > "$1.tmp" <<\EOFGITWEB
> -- 
> 1.7.2.2

Jonathan Nieder· Sep 3, 2010, 18:20 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] instaweb: ignore GITWEB_CONFIG_SYSTEM

Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 7 quoted lines

>> The configuration in /etc/gitweb.conf might specify any old
>> layout; in particular, it is likely not to be identical to that
>> which git instaweb sets up.  Noticed by Uwe Kleine-König.
>
> I am not sure if this is a reasonable change by reading the above.  In
> some cases, the file may stale, but in other cases, it may specify what
> the site administrator wants its users to be using, no?

Yes, I should have mentioned this.

By analogy with .gitconfig, one might want "git instaweb" to check GITWEB_CONFIG_LOCAL, GITWEB_CONFIG_GLOBAL, and GITWEB_CONFIG_SYSTEM in that order, taking values from all three. A GITWEB_CONFIG envvar could override them all.

But stepping back a moment, what would this accomplish? The gitweb configuration specifies

 - what git version to use;
 - site branding;
 - what directory projects are relative to (*);
 - how deep to search for projects;
 - where the list of projects is (*);
 - how to demonstrate project consent to be shown (*);
 - whether to be a stickler about hiding secrets;
 - where gitweb.css, git-logo.png, git-favicon.png, and gitweb.js are (*);
 - base URI for relative paths used above (*);
 - what extra features (e.g. "snapshot") to enable;
 - MIME types, diff options, etc.

The items marked with a (*) are, in the regime used by git 1.7.1, specific to the "git instaweb" instance, since it is all about showing the current git repo and provides its own copies of gitweb.cgi instead of relying on copies in /usr/share/gitweb.

The items not marked with (*) might indeed benefit from some global site defaults. But unfortunately, that is already not what /etc/gitweb.conf is: it is rather used as a unit as a fallback when a separate gitweb_config.perl is not present.

In git 1.7.2 and later, "git instaweb" provides the gitweb_config.perl and this discussion becomes academic.

Show 6 quoted lines

>> -s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;'
>> +s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
>> +s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'
>
> Perhaps moving the last ' to its own line would help maintaining the
> script in the longer term?

Good idea; thanks.

Jonathan Nieder· Sep 3, 2010, 23:58 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] instaweb: ignore GITWEB_CONFIG_SYSTEM

Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 6 quoted lines

>> The configuration in /etc/gitweb.conf might specify any old
>> layout; in particular, it is likely not to be identical to that
>> which git instaweb sets up.  Noticed by Uwe Kleine-König.
>
> I am not sure if this is a reasonable change by reading the above.  In
> some cases, the file may stale, but in other cases

Ah, I missed your point here. By "any old layout", I meant "an arbitrary layout"[1]. Sorry for the lack of clarity.

Jonathan

[1] http://www.google.com/search?q=any+old

Junio C Hamano· Sep 4, 2010, 01:36 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/4] instaweb: ignore GITWEB_CONFIG_SYSTEM

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 9 quoted lines

> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> The configuration in /etc/gitweb.conf might specify any old
>>> layout; in particular, it is likely not to be identical to that
>>> which git instaweb sets up.  Noticed by Uwe Kleine-König.
>> ...
> Ah, I missed your point here.  By "any old layout", I meant "an
> arbitrary layout"[1].  Sorry for the lack of clarity.

Ah, you mean "instaweb wants to use a specific layout but whatever is specified in /etc/gitweb.conf will interfere with it"?

Jonathan Nieder· Sep 4, 2010, 02:19 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] instaweb: ignore GITWEB_CONFIG_SYSTEM

Junio C Hamano wrote:

> Ah, you mean "instaweb wants to use a specific layout but whatever is
> specified in /etc/gitweb.conf will interfere with it"?

Exactly.

Jonathan Nieder· Sep 2, 2010, 23:55 UTC · re: Jonathan Nieder · lore

[PATCH 2/4] gitweb: skip logo in atom feed when there is none

With v1.5.0-rc0~169 (gitweb: Fix Atom feed <logo>: it is $logo, not $logo_url, 2006-12-04), the logo URI to be written to Atom feeds was corrected but the case of no logo forgotten.

Cc: Jakub Narebski <jnareb@gmail.com>
Cc: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Theoretical, untested.

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a97ce03..cc20e74 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6630,7 +6630,7 @@ XML
 		if (defined $favicon) {
 			print "<icon>" . esc_url($favicon) . "</icon>\n";
 		}
-		if (defined $logo_url) {
+		if (defined $logo) {
 			# not twice as wide as tall: 72 x 27 pixels
 			print "<logo>" . esc_url($logo) . "</logo>\n";
 		}

-- 
1.7.2.2

Jonathan Nieder· Sep 2, 2010, 23:56 UTC · re: Jonathan Nieder · lore

[PATCH 3/4] gitweb: make logo optional

Some sites may not want to have a logo at all. In particular, git instaweb can benefit from this.

Cc: Jakub Narebski <jnareb@gmail.com>
Cc: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 gitweb/gitweb.perl |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cc20e74..9a72562 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3269,10 +3269,12 @@ EOF
 		insert_file($site_header);
 	}
 
-	print "<div class=\"page_header\">\n" .
-	      $cgi->a({-href => esc_url($logo_url),
-	               -title => $logo_label},
-	              qq(<img src="$logo" width="72" height="27" alt="git" class="logo"/>));
+	print "<div class=\"page_header\">\n";
+	if (defined $logo) {
+		print $cgi->a({-href => esc_url($logo_url),
+		               -title => $logo_label},
+		              qq(<img src="$logo" width="72" height="27" alt="git" class="logo"/>));
+	}
 	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
 	if (defined $project) {
 		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));

-- 
1.7.2.2

Jonathan Nieder· Sep 2, 2010, 23:57 UTC · re: Jonathan Nieder · lore

[PATCH 4/4] instaweb: disable logo and favicon by default

Avoid 404 errors due to the missing git logo and favicon.

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Eric Wong <normalperson@yhbt.net>
Cc: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for reading.

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

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 6279f4d..1128b31 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -376,7 +376,9 @@ s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
 s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
 s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
 s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
-s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'
+s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;
+s#(my|our) \$favicon =.*#$1 \$favicon = undef;#;'
+s#(my|our) \$logo =.*#$1 \$logo = undef;#;'
 
 gitweb_cgi () {
 	cat > "$1.tmp" <<\EOFGITWEB

-- 
1.7.2.2

Jonathan Nieder· Sep 3, 2010, 02:19 UTC · re: Jonathan Nieder · lore

[PATCH v2 4/4] instaweb: disable logo and favicon by default

Avoid 404 errors due to the missing git logo and favicon.

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Eric Wong <normalperson@yhbt.net>
Cc: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:

Show 10 quoted lines

> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
> @@ -376,7 +376,9 @@ s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
>  s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
>  s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
>  s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
> -s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'
> +s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;
> +s#(my|our) \$favicon =.*#$1 \$favicon = undef;#;'
> +s#(my|our) \$logo =.*#$1 \$logo = undef;#;'

Stray apostrophe; sorry about that.  Here's a replacement.

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

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 6279f4d..0725596 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -376,7 +376,9 @@ s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
 s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
 s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
 s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;
-s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;'
+s#(my|our) \$GITWEB_CONFIG_SYSTEM =.*#$1 \$GITWEB_CONFIG_SYSTEM = "";#;
+s#(my|our) \$favicon =.*#$1 \$favicon = undef;#;
+s#(my|our) \$logo =.*#$1 \$logo = undef;#;'
 
 gitweb_cgi () {
 	cat > "$1.tmp" <<\EOFGITWEB

-- 
1.7.2.2

Eric Wong· Sep 3, 2010, 01:09 UTC · re: Jonathan Nieder · lore

Re: [PATCH/RFC maint-1.7.1 0/4] instaweb 1.7.1 fixes for Debian squeeze

Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 11 quoted lines

> Eric Wong wrote:
> >> Uwe Kleine-König wrote:
> >>> when I start git instaweb on a git repository, I get a crippled view
> >>> (no style sheets, no images) that doesn't include the current project.
> [...]
> > The stylesheet is important and should be fixed.  I don't care about the
> > logo and favicon (warning: I've never been a fan of logos/icons,
> > needless noise IMHO), but if you're willing to help maintain it then
> > it's fine by me.
> 
> Thanks, Eric; that makes sense.  How about this?

Untested, but everything looks reasonable to me, thanks!

-- 
Eric Wong