Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats

Jakub Narebski [off-list ref] writes:

On Tue, 17 July 2007, Matt McCutchen napisał:
...

quoted

Alert for gitweb site administrators: This patch changes the format of
$feature{'snapshot'}{'default'} in gitweb_config.perl from a list of
three pieces of information about a single format to a list of one or
more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').
Update your gitweb_config.perl appropriately.  The preferred names for
gitweb.snapshot in repository configuration have also changed from
'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still
recognized for compatibility.

This alert/warning should probably be put in RelNotes for when it would
be in git.git

Does anybody else worry about the backward imcompatibility, I
wonder...  List?

I really hate to having to say something like that in the
RelNotes.  I do not think this is a good enough reason to break
existing configurations; I would not want to be defending that
change.

quoted

I thought of another incompatibility: previously bookmarked snapshot
URLs will no longer work because they lack the new "sf" parameter.  I
don't care about this; do any of you?

I think either having good error message, or using first format avaiable
would be good enough.

I doubt bookmarked snapshot URL would make sense to begin with,
so this would be Ok.

I am wondering if something like this patch (totally untested,
mind you) to convert the old style %feature in configuration at
the site at runtime would be sufficient.

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f17c983..cdec4d0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -236,9 +236,39 @@ our %feature = (
 		'default' => [0]},
 );
 
+# Functions to convert values from older gitweb configuration
+# into the current data format
+sub gitweb_bc_feature_snapshot {
+	my $def = $feature{'snapshot'}{'default'};
+	# Older definition was to have either undef (to disable), or
+	# a three-element array whose first element was content encoding
+	# without leading "application/".
+	return if (ref $def ne 'ARRAY');
+	if (!defined $def->[0] && @$def == 1) {
+		# Disabled -- the new way to spell it is to have an empty
+		# arrayref.
+		$feature{'snapshot'}{'default'} = [];
+		return;
+	}
+	return if (@$def != 3);
+	for ($def->[0]) {
+		if (/x-gzip/) {
+			$feature{'snapshot'}{'default'} = ['tgz'];
+		}
+		if (/x-bz2/) {
+			$feature{'snapshot'}{'default'} = ['tbz2'];
+		}
+		if (/x-zip/) {
+			$feature{'snapshot'}{'default'} = ['zip'];
+		}
+	}
+}
+
 sub gitweb_check_feature {
 	my ($name) = @_;
 	return unless exists $feature{$name};
+	eval "gitweb_bc_feature_$name()";
+
 	my ($sub, $override, @defaults) = (
 		$feature{$name}{'sub'},
 		$feature{$name}{'override'},