Re: [RFC PATCH] http: use --stdin and --keep when downloading pack

[Jeff King <peff@peff.net>]

On Thu, Feb 21, 2019 at 10:49:48AM -0800, Jonathan Tan wrote:

> > > -	if (finalize_object_file(preq->tmpfile.buf, sha1_pack_name(p->sha1))
> > > -	 || finalize_object_file(tmp_idx, sha1_pack_index_name(p->sha1))) {
> > > -		free(tmp_idx);
> > > -		return -1;
> > > +	if (finish_command(&ip)) {
> > > +		ret = -1;
> > > +		goto cleanup;
> > >  	}
> > 
> > If the command fails but we got something in *lockfile, should we clean
> > it up? Likewise, do we need to be installing a signal handler to clean
> > it up in case we die in other code paths (or by a signal)?
> 
> My inclination is not to do it - as far as I can tell, we don't have
> cleanup or signal handlers in fetch-pack.c either.

Hmm, yeah, you're right. Nor do we do it on fetch. At first I was
surprised that this doesn't cause more problems (we'd leave it in place
after seeing a pre-receive hook fail, for instance), but in practice it
gets cleaned up along with the rest of the quarantine area.

There's still a window where we may leave it around, though (and we
don't quarantine fetches at all), so it might be worth addressing at
some point. But I agree it is totally out of scope for this patch.

-Peff