[PATCH 7 of 7] configitems: adds a developer warning when accessing undeclared configuration

[Boris Feld]

On Tue, 2017-10-17 at 11:27 -0400, Augie Fackler wrote:
> On Mon, Oct 16, 2017 at 06:53:32PM +0200, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld <boris.feld at octobus.net>
> > # Date 1508168487 -7200
> > #      Mon Oct 16 17:41:27 2017 +0200
> > # Node ID 7a2c3832349499f8b00b9db64e6b87ff644faa9d
> > # Parent  d64632aed1d71fd2750aca29fe09d8a2e86921cd
> > # EXP-Topic config.register.ready
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > #              hg pull https://bitbucket.org/octobus/mercurial-deve
> > l/ -r 7a2c38323494
> > configitems: adds a developer warning when accessing undeclared
> > configuration
> 
> I've queued patches 1-6. This one I'm inclined to wait until the 4.5
> cycle, given how annoyingly invasive it's going to be for third
> parties. Otherwise there's no released-hg transition window for
> extensions.
> 
> What do you think? What about others? Durham, I suspect this will be
> particularly annoying for y'all?

We would prefer to have it in this cycle. We want all extensions to
have their configuration registered to start beginning working on the
cool stuff.

For example, we could ship a `hg config --audit` that check user
configuration against the registered one and point out typos, bad types
or deprecated configuration.

As the warning is only for extension authors, it's not a blocker for
the release I think. I understand that most, if not all, extensions
tests will break, maybe we can provide a script to generate 80% of the
configuration registration for extensions, maybe using https://pypi.pyt
hon.org/pypi/redbaron.

However, these 3 months will delay the point where the nice things
(like `hg config --audit`) start to be useful.

> 
> > 
> > Now that all known options are declared, we setup a warning to make
> > sure it will
> > stay this way.
> > 
> > We disable the warning in two tests checking other behavior with
> > random options.
> > 
> > diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> > --- a/mercurial/configitems.py
> > +++ b/mercurial/configitems.py
> > @@ -247,6 +247,9 @@
> >  coreconfigitem('devel', 'user.obsmarker',
> >      default=None,
> >  )
> > +coreconfigitem('devel', 'warn-config-unknown',
> > +    default=None,
> > +)
> >  coreconfigitem('diff', 'nodates',
> >      default=False,
> >  )
> > diff --git a/mercurial/ui.py b/mercurial/ui.py
> > --- a/mercurial/ui.py
> > +++ b/mercurial/ui.py
> > @@ -477,6 +477,10 @@
> > 
> >          if item is not None:
> >              alternates.extend(item.alias)
> > +        else:
> > +            msg = ("accessing unregistered config item: '%s.%s'")
> > +            msg %= (section, name)
> > +            self.develwarn(msg, 2, 'warn-config-unknown')
> > 
> >          if default is _unset:
> >              if item is None:
> > diff --git a/tests/test-devel-warnings.t b/tests/test-devel-
> > warnings.t
> > --- a/tests/test-devel-warnings.t
> > +++ b/tests/test-devel-warnings.t
> > @@ -363,6 +363,8 @@
> >    >     repo.ui.config('test', 'some', 'foo')
> >    >     repo.ui.config('test', 'dynamic', 'some-required-default')
> >    >     repo.ui.config('test', 'dynamic')
> > +  >     repo.ui.config('test', 'unregistered')
> > +  >     repo.ui.config('unregistered', 'unregistered')
> >    > EOF
> > 
> >    $ hg --config "extensions.buggyconfig=${TESTTMP}/buggyconfig.py"
> > buggyconfig
> > @@ -372,5 +374,7 @@
> >    devel-warn: specifying a default value for a registered config
> > item: 'ui.interactive' 'None' at: $TESTTMP/buggyconfig.py:*
> > (cmdbuggyconfig) (glob)
> >    devel-warn: specifying a default value for a registered config
> > item: 'test.some' 'foo' at: $TESTTMP/buggyconfig.py:*
> > (cmdbuggyconfig) (glob)
> >    devel-warn: config item requires an explicit default value:
> > 'test.dynamic' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig)
> > (glob)
> > +  devel-warn: accessing unregistered config item:
> > 'test.unregistered' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig)
> > (glob)
> > +  devel-warn: accessing unregistered config item:
> > 'unregistered.unregistered' at: $TESTTMP/buggyconfig.py:*
> > (cmdbuggyconfig) (glob)
> > 
> >    $ cd ..
> > diff --git a/tests/test-trusted.py b/tests/test-trusted.py
> > --- a/tests/test-trusted.py
> > +++ b/tests/test-trusted.py
> > @@ -67,6 +67,13 @@
> >                                       trusted))
> > 
> >      u = uimod.ui.load()
> > +    # disable the configuration registration warning
> > +    #
> > +    # the purpose of this test is to check the old behavior, not
> > to validate the
> > +    # behavior from registered item. so we silent warning related
> > to unregisted
> > +    # config.
> > +    u.setconfig('devel', 'warn-config-unknown', False, 'test')
> > +    u.setconfig('devel', 'all-warnings', False, 'test')
> >      u.setconfig('ui', 'debug', str(bool(debug)))
> >      u.setconfig('ui', 'report_untrusted', str(bool(report)))
> >      u.readconfig('.hg/hgrc')
> > @@ -157,6 +164,13 @@
> >  print()
> >  print("# read trusted, untrusted, new ui, trusted")
> >  u = uimod.ui.load()
> > +# disable the configuration registration warning
> > +#
> > +# the purpose of this test is to check the old behavior, not to
> > validate the
> > +# behavior from registered item. so we silent warning related to
> > unregisted
> > +# config.
> > +u.setconfig('devel', 'warn-config-unknown', False, 'test')
> > +u.setconfig('devel', 'all-warnings', False, 'test')
> >  u.setconfig('ui', 'debug', 'on')
> >  u.readconfig(filename)
> >  u2 = u.copy()
> > diff --git a/tests/test-ui-config.py b/tests/test-ui-config.py
> > --- a/tests/test-ui-config.py
> > +++ b/tests/test-ui-config.py
> > @@ -6,6 +6,15 @@
> >  )
> > 
> >  testui = uimod.ui.load()
> > +
> > +# disable the configuration registration warning
> > +#
> > +# the purpose of this test is to check the old behavior, not to
> > validate the
> > +# behavior from registered item. so we silent warning related to
> > unregisted
> > +# config.
> > +testui.setconfig('devel', 'warn-config-unknown', False, 'test')
> > +testui.setconfig('devel', 'all-warnings', False, 'test')
> > +
> >  parsed = dispatch._parseconfig(testui, [
> >      'values.string=string value',
> >      'values.bool1=true',
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel