On Fri, Sep 30, 2016 at 03:36:30PM -0400, Jeff King wrote:
> @@ -1639,6 +1666,18 @@ static const char *unpack(int err_fd, struct shallow_info *si)
> argv_array_push(&child.args, alt_shallow_file);
> }
>
> + tmp_objdir = tmp_objdir_create();
> + if (!tmp_objdir)
> + return "unable to create temporary object directory";
> + child.env = tmp_objdir_env(tmp_objdir);
One thing to note here: this new code kicks in all the time. My
reasoning was that there's basically no time you _wouldn't_ want it to,
and certainly that was the case for us when I wrote it. But I tried to
think of user-visible changes. Here's what I came up with:
- we currently leave the tmp_pack_* for a failed push sitting around
(e.g., if the client hangs up halfway through, or index-pack rejects
the pack for some reason). But with this series, it would always be
cleaned up. That's a very good thing if you're running a git hosting
site. It might make things harder if you're debugging.
I don't think it's a good reason not to enable this by default, but
it _could_ be a reason to have a config switch to turn it off
temporarily (or just leave the "incoming-*" directory in place).
- the environment that pre-receive pack runs in has access to objects
that the rest of the repository doesn't. So if you were to do
something silly in your pre-receive like:
# reject the push, but log a copy of the objects
git update-ref refs/rejected/$(date +%s) $new_sha1
exit 1
Then your ref-update would succeed (you have $new_sha1), but the
objects would be deleted immediately afterwards. I find this a
somewhat questionable pattern, and I have no idea if anybody else
has thought of it. But it _does_ work today, and not with this
series.
I don't think it would be too hard to put a config conditional around
this tmp_objdir_create(). And then all of the tmp_objdir_env() calls
would just return NULL, and effectively become noops.
-Peff