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

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.


Reply via email to