From: Jonathan Tan <jonathantanmy@google.com>
Date: Tue, 23 Feb 2021 16:51:20 GMT
Subject: Re: [PATCH v2 2/4] http-fetch: allow custom index-pack args
Message-ID: <20210223165120.1561659-1-jonathantanmy@google.com>
In-Reply-To: <874ki29b53.fsf@evledraar.gmail.com>

> > diff --git a/Documentation/git-http-fetch.txt b/Documentation/git-http-fetch.txt
> > index 4deb4893f5..9fa17b60e4 100644
> > --- a/Documentation/git-http-fetch.txt
> > +++ b/Documentation/git-http-fetch.txt
> > @@ -41,11 +41,17 @@ commit-id::
> >  		<commit-id>['\t'<filename-as-in--w>]
> >  
> >  --packfile=<hash>::
> > -	Instead of a commit id on the command line (which is not expected in
> > +	For internal use only. Instead of a commit id on the command
> > +	line (which is not expected in
> >  	this case), 'git http-fetch' fetches the packfile directly at the given
> >  	URL and uses index-pack to generate corresponding .idx and .keep files.
> >  	The hash is used to determine the name of the temporary file and is
> > -	arbitrary. The output of index-pack is printed to stdout.
> > +	arbitrary. The output of index-pack is printed to stdout. Requires
> > +	--index-pack-args.
> > +
> > +--index-pack-args=<args>::
> > +	For internal use only. The command to run on the contents of the
> > +	downloaded pack. Arguments are URL-encoded separated by spaces.
> >  
> >  --recover::
> >  	Verify that everything reachable from target is fetched.  Used after
> > diff --git a/fetch-pack.c b/fetch-pack.c
> > index 876f90c759..aeac010b0b 100644
> > --- a/fetch-pack.c
> > +++ b/fetch-pack.c
> > @@ -1645,6 +1645,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
> >  		strvec_pushf(&cmd.args, "--packfile=%.*s",
> >  			     (int) the_hash_algo->hexsz,
> >  			     packfile_uris.items[i].string);
> > +		strvec_push(&cmd.args, "--index-pack-arg=index-pack");
> > +		strvec_push(&cmd.args, "--index-pack-arg=--stdin");
> > +		strvec_push(&cmd.args, "--index-pack-arg=--keep");
> 
> The docs say --*-args, but the code checks --*arg, that seems like a
> mistake that should be fixed to make the code/tests use the plural form,
> no?

Thanks for catching that. Originally it was plural since this single
argument would give multiple arguments to index-pack, but now each
argument gives only a single argument, so "arg" is correct. I'll update
it in the next version.