[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 <raf at durin42.com> wrote:

>
> > On Oct 19, 2017, at 17:25, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
> >
> > On Wed, Oct 18, 2017 at 5:40 PM, Augie Fackler <raf at durin42.com> wrote:
> >
> > > On Oct 18, 2017, at 09:18, Boris Feld <boris.feld at octobus.net> 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 <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.
> >
> > 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20171020/9246e271/attachment.html>