Thanks for sending this out and sorry for not paying attention to it for so long.

The relevant parts of the sprint discussion, I think, are in the sections "Friendly HG Breakout" and " Experimental as a Tag instead of a Name" (those are the two places where "registry" came up. There's also some related discussion in the " Flags and defaults breakout" breakout, but that stuff has mostly been sent in patches by Rodrigo and myself already.

I have to take a closer look at this patch and at Jun's immutable config stuff to see if these interact at all. I also think that David SP was thinking about some config registry stuff as well. I may play with this idea during the freeze to see if I can come up with a nice incremental conversion as well.

Again, thanks for the RFC. I'll try to dive in deeper soon.

~Ryan


On 3/24/17 5:41 PM, Gregory Szorc wrote:
On Fri, Mar 24, 2017 at 10:24 AM, Martin von Zweigbergk <martinv...@google.com <mailto:martinv...@google.com>> wrote:

    On Sun, Mar 12, 2017 at 12:18 PM, Gregory Szorc
    <gregory.sz...@gmail.com <mailto:gregory.sz...@gmail.com>> wrote:
    > # HG changeset patch
    > # User Gregory Szorc <gregory.sz...@gmail.com
    <mailto:gregory.sz...@gmail.com>>
    > # Date 1489346234 25200
    > #      Sun Mar 12 12:17:14 2017 -0700
    > # Node ID dd26bc2a3056879181851aaa3ff4accbfc42e1ad
    > # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
    > configoptions: introduce registrar for config options

    I like the direction. I missed the discussion at the sprint. Was the
    consensus that this is the way to go?


There were some side discussions. Not anything formal IIRC.


    What's the next step? Will you send a non-RFC series adding a few
    users as well? I'm also curious to see the "Actually hooking it up to
    config loading" step.


I have no plans to work on this.

I put the RFC out there because there were a number of conversations around overhauling configs and I wanted a tangible prototype for a formal config declaration mechanism to be in people's minds so they could consider benefits that a more formal config mechanism would have. (I think it can solve a lot of problems around things like environment variable mappings, config aliases, stronger type checking, mapping configs to command arguments, etc.)


    >
    > Various talks at the sprint have revolved around establishing more
    > formalization around config options. Specific problems we'd like
    > to solve or are thinking about solving include:
    >
    > * Experimental config options not documented and are not
    discoverable
    >   to end-users.
    > * Config options aren't strongly typed (it depends how they are
    >   accessed).
    > * Config options for extensions don't appear in `hg help config`.
    > * There is no formal mechanism to map a config option to command
    >   argument behavior. e.g. have a config option imply a command
    >   argument. Instead, logic is done in the command implementation,
    >   which results in inconsistent behavior, error messages, weird
    >   `hg help <command>` output.
    > * Config option validation is done at the call site and not as part
    >   of config loading or command dispatching.
    > * Config options are declared by side-effect all over the repo. It
    >   might be nicer to have a single "registry" so the full context of
    >   all options is easily referenced.
    > * No mechanism to "alias" an old config option to a new one. e.g.
    >   carrying over "experimental.feature" to its final value.
    >
    > This patch introduces a very eary proof of concept for improving
    > the situation. It adds config options to the "registrar" mechanism,
    > which allows their declaration to be formalized and recorded in
    > a central location. This is conceptually similar to populating a
    > central dict with the data. I chose to use decorators and (for now)
    > empty functions for declaring config options. This allows docstrings
    > to be used for writing the config help.
    >
    > In the future, one could imagine actually calling the function
    > declaring the config option. It could receive a ui instance and
    > an object defining the command being invoked. The function could
    > then look for conflicting options, adjust command arguments, etc.
    > It could do so in a way that is consistent across commands. e.g.
    > a ConfigOptionConflict exception could be raised and the ui or
    > dispatcher could consistently format that error condition rather
    > than leaving it to individual command functions to raise on their
    > own.
    >
    > It's worth noting that we need all the *core* options defined in
    > a central file because of lazy module loading. If a module isn't
    > loaded, the config option declarations wouldn't be called!
    >
    > There are several things missing from this patch and open issues to
    > resolve:
    >
    > * i18n of help text
    > * Actually using docstrings in `hg help`
    > * Hooking up strong typing or hinted typing
    > * Figuring out how to declare config options with sub-options
    > * Better solution for declaring config options that have both global
    >   options and per-item sub-options (like hostsecurity.ciphers)
    > * Actually hooking it up to config loading
    > * Mechanism for declaring config options in extensions
    >
    > diff --git a/mercurial/configoptions.py b/mercurial/configoptions.py
    > new file mode 100644
    > --- /dev/null
    > +++ b/mercurial/configoptions.py
    > @@ -0,0 +1,102 @@
    > +# configoptions.py -- Declaration of configuration options
    > +#
    > +# Copyright 2017 Gregory Szorc <gregory.sz...@gmail.com
    <mailto:gregory.sz...@gmail.com>>
    > +#
    > +# This software may be used and distributed according to the
    terms of the
    > +# GNU General Public License version 2 or any later version.
    > +
    > +from . import (
    > +    registrar,
    > +)
    > +
    > +configoption = registrar.configoption()
    > +
    > +@configoption('hostsecurity.ciphers')
    > +def optionciphers():
    > +    """Defines the cryptographic ciphers to use for connections.
    > +
    > +    Value must be a valid OpenSSL Cipher List Format as
    documented at
    > +
    https://www.openssl.org/docs/manmaster/apps/ciphers.html#CIPHER-LIST-FORMAT
    
<https://urldefense.proofpoint.com/v2/url?u=https-3A__www.openssl.org_docs_manmaster_apps_ciphers.html-23CIPHER-2DLIST-2DFORMAT&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=6FYVT5Po5N2vyslmzAdZmMlUq8oQi1a-uYeozyLWArw&s=JbBgutXmiKYWcHYWQaz5_9SUBel8KfUtwNVbTj_RPRA&e=>.
    > +
    > +    This setting is for advanced users only. Setting to
    incorrect values
    > +    can significantly lower connection security or decrease
    performance.
    > +    You have been warned.
    > +
    > +    This option requires Python 2.7.
    > +    """
    > +
    > +@configoption('hostsecurity.minimumprotocol')
    > +def optionminimumprotocol():
    > +    """Defines the minimum channel encryption protocol to use.
    > +
    > +    By default, the highest version of TLS supported by both
    client and
    > +    server is used.
    > +
    > +    Allowed values are: ``tls1.0``, ``tls1.1``, ``tls1.2``.
    > +
    > +    When running on an old Python version, only ``tls1.0`` is
    allowed since
    > +    old versions of Python only support up to TLS 1.0.
    > +
    > +    When running a Python that supports modern TLS versions,
    the default is
    > +    ``tls1.1``. ``tls1.0`` can still be used to allow TLS 1.0.
    However, this
    > +    weakens security and should only be used as a feature of
    last resort if
    > +    a server does not support TLS 1.1+.
    > +    """
    > +
    > +@configoption('hostsecurity.*:ciphers')
    > +def perhostciphers():
    > +    """Per host version of ``hostsecurity.ciphers``."""
    > +
    > +@configoption('hostsecurity.*:fingerprints')
    > +def perhostfingerprints():
    > +    """A list of hashes of the DER encoded peer/remote
    certificate. Values
    > +    have the form ``algorithm``:``fingerprint``. e.g.
> + ``sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2``.
    > +
    > +    The following algorithms/prefixes are supported: ``sha1``,
    ``sha256``,
    > +    ``sha512``.
    > +
    > +    Use of ``sha256`` or ``sha512`` is preferred.
    > +
    > +    If a fingerprint is specified, the CA chain is not
    validated for this
    > +    host and Mercurial will require the remote certificate to
    match one
    > +    of the fingerprints specified. This means if the server
    updates its
    > +    certificate, Mercurial will abort until a new fingerprint
    is defined.
    > +    This can provide stronger security than traditional
    CA-based validation
    > +    at the expense of convenience.
    > +
    > +    This option takes precedence over ``verifycertsfile``.
    > +    """
    > +
    > +@configoption('hostsecurity.*:minimumprotocol')
    > +def perhostminimumprotocol():
    > +    """This behaves like ``minimumprotocol`` as described above
    except it
    > +    only applies to the host on which it is defined.
    > +    """
    > +
    > +@configoption('hostsecurity.*:verifycertsfile')
    > +def perhostverifycertsfile():
    > +    """Path to file a containing a list of PEM encoded
    certificates used to
    > +    verify the server certificate. Environment variables and
    ``~user``
    > +    constructs are expanded in the filename.
    > +
    > +    The server certificate or the certificate's certificate
    authority (CA)
    > +    must match a certificate from this file or certificate
    verification
    > +    will fail and connections to the server will be refused.
    > +
    > +    If defined, only certificates provided by this file will be
    used:
    > +    ``web.cacerts`` and any system/default certificates will not be
    > +    used.
    > +
    > +    This option has no effect if the per-host ``fingerprints``
    option
    > +    is set.
    > +
    > +    The format of the file is as follows::
    > +
    > +        -----BEGIN CERTIFICATE-----
    > +        ... (certificate in base64 PEM encoding) ...
    > +        -----END CERTIFICATE-----
    > +        -----BEGIN CERTIFICATE-----
    > +        ... (certificate in base64 PEM encoding) ...
    > +        -----END CERTIFICATE-----
    > +    """
    > diff --git a/mercurial/registrar.py b/mercurial/registrar.py
    > --- a/mercurial/registrar.py
    > +++ b/mercurial/registrar.py
    > @@ -251,4 +251,20 @@ class templatefunc(_templateregistrarbas
    >
    >      Otherwise, explicit 'templater.loadfunction()' is needed.
    >      """
    >      _getname = _funcregistrarbase._parsefuncdecl
    > +
    > +class configoption(_funcregistrarbase):
    > +    """Decorator to register a config option."""
    > +    def _getname(self, decl):
    > +        return decl
    > +
    > +    def _formatdoc(self, decl, doc):
    > +        return pycompat.sysstr('.'.join(decl))
    > +
    > +    def _extrasetup(self, name, func, *args, **kwargs):
    > +        section, option = name.split('.', 1)
    > +
    > +        self._section = section
    > +        self._name = name
    > +
    > +        self._extra = kwargs
    > diff --git a/mercurial/ui.py b/mercurial/ui.py
    > --- a/mercurial/ui.py
    > +++ b/mercurial/ui.py
    > @@ -27,8 +27,9 @@ from .node import hex
    >
    >  from . import (
    >      color,
    >      config,
    > +    configoptions,
    >      encoding,
    >      error,
    >      formatter,
    >      progress,
    > @@ -135,8 +136,11 @@ class ui(object):
    >          Use uimod.ui.load() to create a ui which knows global
    and user configs.
    >          In most cases, you should use ui.copy() to create a
    copy of an existing
    >          ui object.
    >          """
    > +        # Ensure config options are imported as a side-effect.
    > +        options = configoptions.configoption
    > +
    >          # _buffers: used for temporary capture of output
    >          self._buffers = []
    >          # 3-tuple describing how each buffer in the stack behaves.
    >          # Values are (capture stderr, capture subprocesses,
    apply labels).


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

Reply via email to