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

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

quoted

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