[PATCH RFC] configoptions: introduce registrar for config options

Martin von Zweigbergk martinvonz at google.com
Fri Mar 24 13:24:28 EDT 2017


On Sun, Mar 12, 2017 at 12:18 PM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at 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?

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.

>
> 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.szorc at 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()
> +
> + at 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.
> +
> +    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.
> +    """
> +
> + at 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+.
> +    """
> +
> + at configoption('hostsecurity.*:ciphers')
> +def perhostciphers():
> +    """Per host version of ``hostsecurity.ciphers``."""
> +
> + at 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``.
> +    """
> +
> + at configoption('hostsecurity.*:minimumprotocol')
> +def perhostminimumprotocol():
> +    """This behaves like ``minimumprotocol`` as described above except it
> +    only applies to the host on which it is defined.
> +    """
> +
> + at 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 at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list