Re: [PATCHv2] Add options to specify snapshot file name, prefix

[Jakub Narebski <jnareb@gmail.com>]

On Sat, 5 Nov 2011, Thomas Guyot-Sionnest wrote:
> On 11-11-04 12:10 PM, Jakub Narebski wrote:
> > Thomas Guyot-Sionnest <dermoth@aei.ca> writes:
> >
> > > commit b629275 implemented "smart" snapshot names and prefixes. I have
> > > scripts that used to rely on the old behaviour which allowed in some
> > > cases to have fixed prefix, and would require modifications to work with
> > > newer Gitweb.
[...]
> > > This patch adds two parameters for overriding the snapshot name and
> > > prefix, sn and sp respectively. For example, to get a snapshot
> > > named "myproject.[suffix]" with no prefix one can add this query string:
> > >   ?sn=myproject;sp=
> >
> > Would you need support for expandable parameters in both (a la
> > 'action' feature)?
> 
> I'm not sure what you mean... I never tinkered with gitweb.pl directly
> before.

I'm sorry I didn't make it clear what I meant here.

What I wanted to ask is if you would need support for snapshot names
like for example

   myproject-<full sha1>.tar.gz
 
It means that snapshot name depends on commit / tag / tree being archived;
for that you would need e.g.

   ...?sn=myproject-%H.tar.gz;sp=

But even if there would be need for this, it should be anyway put into
separate commit.


In short: forget about this comment.

[...]
> > > @@ -6684,11 +6686,19 @@ sub git_snapshot {
> > >  	}
> > >
> > >  	my ($name, $prefix) = snapshot_name($project, $hash);
> > > +	if (defined($input_params{'snapshot_name'})) {
> > > +		$name = $input_params{'snapshot_name'};
> > > +	}
> > > +	if (defined($input_params{'snapshot_prefix'})) {
> > > +		$prefix = $input_params{'snapshot_prefix'};
> > > +	} else {
> > > +		$prefix .= '/';
> > > +	}
> > >  	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
> > >  	my $cmd = quote_command(
> > >  		git_cmd(), 'archive',
> > >  		"--format=$known_snapshot_formats{$format}{'format'}",
> > > -		"--prefix=$prefix/", $hash);
> > > +		"--prefix=$prefix", $hash);
> > >  	if (exists $known_snapshot_formats{$format}{'compressor'}) {
> > >  		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
> > >  	}
> >
> > I wonder if you really want to allow prefix which do not end in '/'
> 
> I kind of agree, yet considering its lack of "front-end" visibility it
> made me think of plumbing commands like git-checkout-index (which I use
> sometimes to replace the "missing" git export) where prefix is nothing
> more than an appended string to file names.
> 
> And now I remember, this is also exactly how git-archive works.

Right.

I wonder if anybody is using "git archive" with prefix that DOESN'T
end with a '/'...
 
> > (which would be suprising, isn't it), or just allow empty prefix too.
> >
> >
> > For example
> >
> >   @@ -6684,11 +6686,19 @@ sub git_snapshot {
> >    	}
> >
> >    	my ($name, $prefix) = snapshot_name($project, $hash);
> >   +	if (defined($input_params{'snapshot_name'})) {
> >   +		$name = $input_params{'snapshot_name'};
> >   +	}
> >   +	if (defined($input_params{'snapshot_prefix'})) {
> >   +		$prefix = $input_params{'snapshot_prefix'};
> >   +	}
> >    	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
> >    	my $cmd = quote_command(
> >    		git_cmd(), 'archive',
> >    		"--format=$known_snapshot_formats{$format}{'format'}",
> >   -		"--prefix=$prefix/", $hash);
> >   +		($prefix eq "" ? () : "--prefix=$prefix"), $hash);
> >    	if (exists $known_snapshot_formats{$format}{'compressor'}) {
> >    		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
> >    	}
> >
> 
> You still have to add the /, i.e.:
> 
> >   +		($prefix eq "" ? () : "--prefix=$prefix/"), $hash);

True.  Sorry about that.
 
> And personally, I think git-archive is the one that should add a / - it
> has much more visibility to end-users than this obscure query-string.

So should we expect a re-roll?


P.S. I'd like to point out that git is now in pre-release feature freeze,
so please don't expect for these changes to appear in 'master' soon, though
most probably they would be available in 'pu' for the time being.

-- 
Jakub Narebski
Poland