Re: [RFC PATCH] http: use --stdin and --keep when downloading pack
- From
Jeff King <peff@peff.net>
- Date
- Feb 22, 2019, 05:04 UTC
- Message-ID
- <20190222050406.GA6050@sigill.intra.peff.net>
- In-Reply-To
- <20190221184948.100083-1-jonathantanmy@google.com>
On Thu, Feb 21, 2019 at 10:49:48AM -0800, Jonathan Tan wrote:
Show 15 quoted lines
> > > - 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