[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 <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/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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20171020/5ad150db/attachment.html>