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

[Boris Feld]

On Fri, 2017-10-20 at 00:45 +0200, Gregory Szorc wrote:
> On Thu, Oct 19, 2017 at 11:26 PM, Augie Fackler 
> wrote:
> > > On Oct 19, 2017, at 17:25, Gregory Szorc  > > wrote:
> > 
> > >
> > 
> > > On Wed, Oct 18, 2017 at 5:40 PM, Augie Fackler 
> > wrote:
> > 
> > >
> > 
> > > > On Oct 18, 2017, at 09:18, Boris Feld 
> > wrote:
> > 
> > > >
> > 
> > > > 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 
> > 
> > > >>> # 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/mercuria
> > l-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.
> > 
> > >
> > 
> > > Closing the loop from an irc conversation: my concern was that
> > there be a transitional release that has the registrar mechanism
> > but not the enforcement of using the registrar, so that you don't
> > have to have a "big bang" upgrade process where (in order to pass
> > tests) you have to upgrade all your extensions and hg
> > simultaneously. Fortunately, 4.3 includes the registrar mechanism,
> > so we're good, and I've queued patch 7. Thanks!
> > 
> > >
> > 
> > > I know this was already queued. But I share Boris's desire to
> > enable devel warnings as early as possible out of principle. It's
> > not difficult to suppress these warnings in tests. But if people
> > complain it is difficult, we could add a special flag to run-
> > tests.py to control devel warnings or something to make it even
> > easier. We should encourage extension authors to make code future
> > compatible sooner rather than later. So I generally support
> > enabling new devel warnings when they're ready.
> > 
> > 
> > 
> > My worry was mostly around having a transitional release where the
> > new option was available but not yet enforced, so upgrades don't
> > have to be atomic (which is often not practical, even for big
> > companies with centrally managed infrastructure). We've cleared
> > that bar. :)
> 
> Thinking about this some more, extensions will need to use the
> default argument on option lookup for compat with old Mercurial
> versions. This will print a devel "specifying a default value for a
> registered config option" warning on newer Mercurials. The recourse
> today to not emit warnings on any version is to conditionally pass
> the default argument depending on the Mercurial version. Yuck.
> 
> Should we offer an escape hatch so extensions can suppress this
> warning on a per-option (either in its registrar declaration or at
> lookup time) or global (all options registered to an extension
> basis)? Maybe registrar.configitem() could take a "suppress warnings"
> argument or something. We would then support this suppression
> mechanism for N releases before making it no-op and/or dropping it.
> 
> Or we could just tell extension authors to suppress the default
> argument warning via devel.warn-config-default=false config option.
> That's doable. But slightly annoying for testing since it could mask
> legitimate warnings. Or is there a way to make test output
> conditional on hghave features? I suppose .t lines could be qualified
> with e.g. hg44+ or something.
> 

It's indeed a nuisance for extensions that need to support old
Mercurial versions (Mercurial 4.2 and before).

One working workaround is to use `dynamicdefault` when registering the
configuration items and the warning will not be shown. It will be
working only if all default values are passed on config* calls.___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Gregory Szorc]

On Thu, Oct 19, 2017 at 11:26 PM, Augie Fackler  wrote:

>
> > On Oct 19, 2017, at 17:25, Gregory Szorc 
> wrote:
> >
> > On Wed, Oct 18, 2017 at 5:40 PM, Augie Fackler  wrote:
> >
> > > On Oct 18, 2017, at 09:18, Boris Feld  wrote:
> > >
> > > 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 
> > >>> # 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.
> >
> > Closing the loop from an irc conversation: my concern was that there be
> a transitional release that has the registrar mechanism but not the
> enforcement of using the registrar, so that you don't have to have a "big
> bang" upgrade process where (in order to pass tests) you have to upgrade
> all your extensions and hg simultaneously. Fortunately, 4.3 includes the
> registrar mechanism, so we're good, and I've queued patch 7. Thanks!
> >
> > I know this was already queued. But I share Boris's desire to enable
> devel warnings as early as possible out of principle. It's not difficult to
> suppress these warnings in tests. But if people complain it is difficult,
> we could add a special flag to run-tests.py to control devel warnings or
> something to make it even easier. We should encourage extension authors to
> make code future compatible sooner rather than later. So I generally
> support enabling new devel warnings when they're ready.
>
> My worry was mostly around having a transitional release where the new
> option was available but not yet enforced, so upgrades don't have to be
> atomic (which is often not practical, even for big companies with centrally
> managed infrastructure). We've cleared that bar. :)


Thinking about this some more, extensions will need to use the default
argument on option lookup for compat with old Mercurial versions. This will
print a devel "specifying a default value for a registered config option"
warning on newer Mercurials. The recourse today to not emit warnings on any
version is to conditionally pass the default argument depending on the
Mercurial version. Yuck.

Should we offer an escape hatch so extensions can suppress this warning on
a per-option (either in its registrar declaration or at lookup time) or
global (all options registered to an extension basis)? Maybe
registrar.configitem() could take a "suppress warnings" argument or
something. We would then support this suppression mechanism for N releases
before making it no-op and/or dropping it.

Or we could just tell extension authors to suppress the default argument
warning via devel.warn-config-default=false config option. That's doable.
But slightly annoying for testing since it could mask legitimate warnings.
Or is there a way to make test output conditional on hghave features? I
suppose .t lines could be qualified with e.g. hg44+ or something.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Augie Fackler]

> On Oct 19, 2017, at 17:25, Gregory Szorc  wrote:
> 
> On Wed, Oct 18, 2017 at 5:40 PM, Augie Fackler  wrote:
> 
> > On Oct 18, 2017, at 09:18, Boris Feld  wrote:
> >
> > 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 
> >>> # 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.
> 
> Closing the loop from an irc conversation: my concern was that there be a 
> transitional release that has the registrar mechanism but not the enforcement 
> of using the registrar, so that you don't have to have a "big bang" upgrade 
> process where (in order to pass tests) you have to upgrade all your 
> extensions and hg simultaneously. Fortunately, 4.3 includes the registrar 
> mechanism, so we're good, and I've queued patch 7. Thanks!
> 
> I know this was already queued. But I share Boris's desire to enable devel 
> warnings as early as possible out of principle. It's not difficult to 
> suppress these warnings in tests. But if people complain it is difficult, we 
> could add a special flag to run-tests.py to control devel warnings or 
> something to make it even easier. We should encourage extension authors to 
> make code future compatible sooner rather than later. So I generally support 
> enabling new devel warnings when they're ready.

My worry was mostly around having a transitional release where the new option 
was available but not yet enforced, so upgrades don't have to be atomic (which 
is often not practical, even for big companies with centrally managed 
infrastructure). We've cleared that bar. :)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Gregory Szorc]

On Wed, Oct 18, 2017 at 5:40 PM, Augie Fackler  wrote:

>
> > On Oct 18, 2017, at 09:18, Boris Feld  wrote:
> >
> > 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 
> >>> # 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.
>
> Closing the loop from an irc conversation: my concern was that there be a
> transitional release that has the registrar mechanism but not the
> enforcement of using the registrar, so that you don't have to have a "big
> bang" upgrade process where (in order to pass tests) you have to upgrade
> all your extensions and hg simultaneously. Fortunately, 4.3 includes the
> registrar mechanism, so we're good, and I've queued patch 7. Thanks!
>

I know this was already queued. But I share Boris's desire to enable devel
warnings as early as possible out of principle. It's not difficult to
suppress these warnings in tests. But if people complain it is difficult,
we could add a special flag to run-tests.py to control devel warnings or
something to make it even easier. We should encourage extension authors to
make code future compatible sooner rather than later. So I generally
support enabling new devel warnings when they're ready.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Martin von Zweigbergk via Mercurial-devel]

On Thu, Oct 19, 2017 at 1:49 PM, Ryan McElroy  wrote:

> On 10/18/17 11:40 AM, Augie Fackler wrote:
>
> 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.
>
> Closing the loop from an irc conversation: my concern was that there be a 
> transitional release that has the registrar mechanism but not the enforcement 
> of using the registrar, so that you don't have to have a "big bang" upgrade 
> process where (in order to pass tests) you have to upgrade all your 
> extensions and hg simultaneously. Fortunately, 4.3 includes the registrar 
> mechanism, so we're good, and I've queued patch 7. Thanks!
>
>
> It looks like this didn't fix the tests for anything in the svn convert
> area. I'll send follow-up patches to fix that.
>

Probably fixed by https://phab.mercurial-scm.org/D1180?

>
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Ryan McElroy]

On 10/18/17 11:40 AM, Augie Fackler wrote:

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.

Closing the loop from an irc conversation: my concern was that there be a transitional 
release that has the registrar mechanism but not the enforcement of using the registrar, 
so that you don't have to have a "big bang" upgrade process where (in order to 
pass tests) you have to upgrade all your extensions and hg simultaneously. Fortunately, 
4.3 includes the registrar mechanism, so we're good, and I've queued patch 7. Thanks!


It looks like this didn't fix the tests for anything in the svn convert 
area. I'll send follow-up patches to fix that.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Augie Fackler]

> On Oct 18, 2017, at 09:18, Boris Feld  wrote:
> 
> 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 
>>> # 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.

Closing the loop from an irc conversation: my concern was that there be a 
transitional release that has the registrar mechanism but not the enforcement 
of using the registrar, so that you don't have to have a "big bang" upgrade 
process where (in order to pass tests) you have to upgrade all your extensions 
and hg simultaneously. Fortunately, 4.3 includes the registrar mechanism, so 
we're good, and I've queued patch 7. Thanks!

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [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 
> > # 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()
> >  

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

[Ryan McElroy]

On 10/17/17 4:27 PM, Augie Fackler wrote:

On Mon, Oct 16, 2017 at 06:53:32PM +0200, Boris Feld wrote:

# HG changeset patch
# User Boris Feld 
# Date 1508168487 -7200
#  Mon Oct 16 17:41:27 2017 +0200
# Node ID 7a2c3832349499f8b00b9db64e6b87ff644faa9d
# Parent  d64632aed1d71fd2750aca29fe09d8a2e86921cd
# EXP-Topic config.register.ready

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?


My view is that this will cause a little pain which will cause us to 
quickly define all of the configs we use, which will be good for us in 
the long-run. It will be more painful for extensions that don't 
continuously integrate, but it's only a devel-warn so I feel the pain 
level is the right balance. I'd be in favor of queuing this and getting 
the pain over more quickly, myself.



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', 

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

[Augie Fackler]

On Mon, Oct 16, 2017 at 06:53:32PM +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld 
> # 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-devel/ -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?

>
> 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', 

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

[Boris Feld]

# HG changeset patch
# User Boris Feld 
# 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-devel/ -r 
7a2c38323494
configitems: adds a developer warning when accessing undeclared configuration

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@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel