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

From: Jeff King <peff@peff.net>
Date: Mar 19, 2015, 21:00 UTC
Message-ID: <20150319210019.GB7775@peff.net>
In-Reply-To: <xmqqbnjoiw4m.fsf@gitster.dls.corp.google.com>

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

Show 18 quoted lines

> 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