[PATCH 05 of 10] chgserver: add a context manager to learn what to include for confighash

Jun Wu quark at fb.com
Thu Jun 30 12:59:00 EDT 2016


# HG changeset patch
# User Jun Wu <quark at fb.com>
# Date 1467292128 -3600
#      Thu Jun 30 14:08:48 2016 +0100
# Node ID 50d862642eb658dc28ca878a4ded89ab7303e3a9
# Parent  c02da7bd56e98cf4e905550859031079ed9beb18
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 50d862642eb6
chgserver: add a context manager to learn what to include for confighash

A pitfall about chg is developers need to be aware the ui object passed to
ui/extsetup can have wrong config and should not be depended on. This confuses
people, and causes inconvenience.

Instead of adding another set APIs letting extensions telling chg what need to
be hashed and fixing all problematic extensions one by one, I think it is
better just letting chg collecting these pieces of information automatically.

This patch adds a context manager wrapping common getters and adjusts
what to hash for confighash accordingly. It only includes configs currently
because supporting environment variables is while possible, not elegant
since we have to wrap stdlib.

Note this is not a solution for all cases, namely the following cases still
need extra work:

1. "if ui.config('x'): ui.setconfig('x', 'y')" in uisetup.
   This will overwrite config 'x' and chg will be unable to hash the value
   user provides. The pattern is mainly used in the evolve extension. It
   could be solved if we have clear layers about what are set by extensions
   and what are set by users.

2. Patterns like "self.ui = ui" in uisetup.
   This will keep a reference to a stale ui object. We probably want to
   add devel-warn for the case. It may also be reasonable to add a similar
   check to reposetup.

3. Interacting with filesystem or network in uisetup.
   In general, extensions doing these should be aware of race conditions.

We can also add a top-level extension interface like "chguisetup" called
per chg request. But that should not be a decision made lightly.

diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -64,6 +64,7 @@ from mercurial import (
     error,
     extensions,
     osutil,
+    ui as uimod,
     util,
 )
 
@@ -728,3 +729,37 @@ def _unwrapfuncs(wraplist):
         else:
             raise RuntimeError('%s cannot be restored' % name)
     wraplist[:] = []
+
+class _learnconfighash(object):
+    """learn what configs are accessed and add them to confighash"""
+    def __init__(self, ui, extname):
+        self._ui = ui
+        self._extname = extname
+        self._wraplist = []
+
+    def _uiconfigitems(self, orig, uiself, section, *args, **kwargs):
+        if section not in _configsections:
+            _log('add config section %s used by %s to confighash'
+                 % (section, self._extname))
+            _configsections.append(section)
+        return orig(uiself, section, *args, **kwargs)
+
+    def _uiconfig(self, orig, uiself, section, name, *args, **kwargs):
+        if section not in _configsections:
+            pair = (section, name)
+            if pair not in _configitems:
+                _log('add config item %s.%s used by %s to confighash'
+                     % (section, name, self._extname))
+                _configitems.append(pair)
+        return orig(uiself, section, name, *args, **kwargs)
+
+    def _wrapfunc(self, obj, name, newfunc):
+        _wrapfunc(obj, name, newfunc, self._wraplist)
+
+    def __enter__(self):
+        self._wrapfunc(uimod.ui, 'configitems', self._uiconfigitems)
+        self._wrapfunc(uimod.ui, 'config', self._uiconfig)
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        _unwrapfuncs(self._wraplist)
+        self._ui = None


More information about the Mercurial-devel mailing list