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

[Jeff King <peff@peff.net>]

On Thu, Mar 19, 2015 at 01:13:13PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > diff --git a/refs.c b/refs.c
> > index e23542b..7f0e7be 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -1934,6 +1934,11 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
> >  	data.fn = fn;
> >  	data.cb_data = cb_data;
> >  
> > +	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. For that case, we could add a "--ref-paranoia"
flag to pack-objects.

But there are also other cases where the _user_ might want to tell us to
be paranoid (e.g., the upload-pack example I gave earlier). Adding
--paranoia options to every command that might iterate, along with
support for other programs that call them to pass the option through,
seems like a large amount of maintenance burden for little benefit.

We could add a git-global "git --ref-paranoia <cmd>" option and leave
the environment variable as an implementation detail. That makes it hard
to turn on for server-side operations, though. git-daemon runs
git-upload-pack without room for options, though I suppose you could run
"git --ref-paranoia daemon". Smart-http is harder. I'm not sure Apache
lets you add random arguments to CGI programs.

Or is there something else I'm missing?

-Peff