Re: [PATCH 2/5] refs: introduce a "ref paranoia" flag

From: Jeff King <peff@peff.net>
Date: Mar 19, 2015, 21:51 UTC
Message-ID: <20150319215114.GB8680@peff.net>
In-Reply-To: <xmqqpp84hdxg.fsf@gitster.dls.corp.google.com>

On Thu, Mar 19, 2015 at 02:31:39PM -0700, Junio C Hamano wrote:

Show 20 quoted lines

> > We do have to have this variable cross some process boundaries. Only
> > "repack" knows whether to turn on paranoia, but "pack-objects" is the
> > one that must act on it.
> >
> > Or is there something else I'm missing?
> 
> In general, I do not like the pattern of program A setting an
> environment only because it wants to tell program B it spawns
> something, because we cannot tell program B that the environment
> should be dropped when it calls something else (e.g. user defined
> hooks, merge drivers, textconvs, etc.) to prevent end user
> invocation of Git commands from honoring it.  Setting GIT_DIR or
> GIT_WORK_TREE and having to know when to drop them is not very
> pleasant, for example.
> 
> I think the use of this pattern is OK in this codepath in which
> repack calls pack-objects, and I think I can be persuaded to buy an
> argument that there is no harm, or it may even be a good thing, to
> run such an end-user program under paranoia mode, if pack-objects
> wants to spawn one.

Ah, I see. Yeah, I consider that to be a _feature_ for REF_PARANOIA here. If you are running receive-pack under REF_PARANOIA, for example, you would want your pre-receive hooks to use the same rules as the rest of receive-pack.

If there is a misfeature, it is that we turn on REF_PARANOIA automatically behind the user's back in some cases, which could surprise them if we call through to custom code. But as you note, I think this code path is OK, because we don't spawn anything else from pack-objects (and if we did, arguably it is OK because our caller told us we are doing something dangerous; but we would have to evaluate that case-by-case, I would think).

-Peff