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

Jeff King [off-list ref] writes:

quoted

quoted

+	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.