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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> > +	if (ref_paranoia < 0)
>> > +		ref_paranoia = git_env_bool("GIT_REF_PARANOIA", 0);
>> > +	if (ref_paranoia)
>> > +		data.flags |= DO_FOR_EACH_INCLUDE_BROKEN;
>> 
>> I am not a big fan of proliferation of interfaces based on
>> environment variables, but hopefully this is isolated enough to
>> become an issue in the future.
>
> I'm not sure which part you don't like.
> 
> 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.