[git-buildpackage] could gbp.config.GbpOptionParser.get_config_files handle GBP_CONF_FILES=''

[Guido Günther]

Hi,
On Thu, Jan 11, 2018 at 08:22:13AM -0800, Nish Aravamudan wrote:
> On 11.01.2018 [08:39:10 +0100], Guido Günther wrote:
> > Hi Nish,
> > On Wed, Jan 10, 2018 at 02:21:52PM -0800, Nish Aravamudan wrote:
> > > For the reproducibility/debuggability of the git-ubuntu snap, we want to
> > > ensure that everyone will end up running certain commands (e.g., quilt,
> > > gbp) the same, no matter what their local configuration is. This is
> > > especially true for the importer itself, but is also true for other
> > > subcommands in our case.
> > > 
> > > I wonder if it might be reasonable to do:
> > > 
> > > diff --git a/gbp/config.py b/gbp/config.py
> > > index 8acf15da..c2feb2ab 100644
> > > --- a/gbp/config.py
> > > +++ b/gbp/config.py
> > > @@ -409,7 +409,7 @@ class GbpOptionParser(OptionParser):
> > >          >>> if conf_backup is not None: os.environ['GBP_CONF_FILES'] = conf_backup
> > >          """
> > >          envvar = os.environ.get('GBP_CONF_FILES')
> > > -        files = envvar.split(':') if envvar else [f for (f, _) in cls.def_config_files]
> > > +        files = envvar.split(':') if envvar is not None else [f for (f, _) in cls.def_config_files]
> > >          files = [os.path.expanduser(fname) for fname in files]
> > >          if no_local:
> > >              files = [fname for fname in files if fname.startswith('/')]
> > > 
> > > This would allow environments that wish to disable config file parsing
> > > (and thus behavioral variability) to juset set GBP_CONF_FILES='' rather
> > > than to something like '/dev/null' (which seems to be the only choice
> > > currently)?
> > 
> > Isn't it be more explicit to have GBP_CONF_FILES=/does/not/exist or
> > GBP_CONF_FILES=/dev/null than to make a distinction between unset or
> > set to the empty string? I'd rather not change the current behaviour.
> 
> I'm not sure, right now the behavior with GBP_CONF_FILES='' is
> surprising -- it seems like it should mean no files are parsed, but
> currently it means all the normal files are parsed.

Is this spelled out somewhere? When thinking in shell I would assume to
that:

   if [ -z "${GBP_CONF_FILES}" ]; then
     do something
   fi

do the same regardless if GBP_CONF_FILES is unset or
GBP_CONF_FILES=''. This wouldn't be true anymore. (and in fact that's
the only reason I have to justify the current behavior).

Cheers,
 -- Guido