From bdf4505a3ffb1e2aed13db51f130fc219a121ff9 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Tue, 19 Mar 2024 09:14:00 -0700 Subject: [PATCH] debugruntest: improve compatibility with extensions Summary: debugruntest executes sl commands in-process to eliminate process startup overhead. This is incompatible with extensions changing during the test since the extensions are only loaded once, installing their function wrappers. If a later command disables the extension, the function wrappers (and other side effects) will still remain, probably messing things up. In this diff I attempt to fix the function wrappers to be compatible with debugruntest. Basically, when wrapping things we install another layer of wrapping that checks at runtime whether the extension is still enabled (and doesn't run the wrapper if the extension is disabled). Another approach would be to track all the wrappers and unwrap/rewrap them as we notice extensions have been disbled/enabled, but this way seemed simpler and maybe more reliable. Reviewed By: quark-zju Differential Revision: D54839559 fbshipit-source-id: 071bc0e832e48c7049f33c720007ab958a51e933 --- eden/scm/sapling/dispatch.py | 2 +- eden/scm/sapling/extensions.py | 65 ++++++++++++++++++- eden/scm/tests/test-debugruntest-extensions.t | 41 ++++++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 eden/scm/tests/test-debugruntest-extensions.t diff --git a/eden/scm/sapling/dispatch.py b/eden/scm/sapling/dispatch.py index 1497a0de1ac5e..df84089c5f703 100644 --- a/eden/scm/sapling/dispatch.py +++ b/eden/scm/sapling/dispatch.py @@ -908,7 +908,7 @@ def _dispatch(req): progress.init() # Configure extensions in phases: uisetup, extsetup, cmdtable, and # reposetup - extensions.loadall(lui) + extensions.initialload(lui) # Propagate any changes to lui.__class__ by extensions ui.__class__ = lui.__class__ diff --git a/eden/scm/sapling/extensions.py b/eden/scm/sapling/extensions.py index 28434c4cd7e33..1c93339ed354f 100644 --- a/eden/scm/sapling/extensions.py +++ b/eden/scm/sapling/extensions.py @@ -341,6 +341,9 @@ def load(ui, name, path): shortname = name if shortname in _ignoreextensions: return None + + _loaded_extensions.add(shortname) + if shortname in _extensions: return _extensions[shortname] _extensions[shortname] = None @@ -382,11 +385,31 @@ def _getattr(obj, name, default): raise +_extension_being_loaded = None + + +class _current_extension: + def __init__(self, name): + self.name = name + + def __enter__(self): + global _extension_being_loaded + self.prev_value = _extension_being_loaded + _extension_being_loaded = self.name + return None + + def __exit__(self, exc_type, exc_value, traceback): + global _extension_being_loaded + _extension_being_loaded = self.prev_value + return None + + def _runuisetup(name, ui): uisetup = getattr(_extensions[name], "uisetup", None) if uisetup: try: - uisetup(ui) + with _current_extension(name): + uisetup(ui) except Exception as inst: ui.traceback(force=True) msg = util.forcebytestr(inst) @@ -395,6 +418,7 @@ def _runuisetup(name, ui): notice=_("warning"), ) return False + return True @@ -403,7 +427,8 @@ def _runextsetup(name, ui): if extsetup: try: try: - extsetup(ui) + with _current_extension(name): + extsetup(ui) except TypeError: # Try to use getfullargspec (Python 3) first, and fall # back to getargspec only if it doesn't exist so as to @@ -412,7 +437,8 @@ def _runextsetup(name, ui): extsetup ).args: raise - extsetup() # old extsetup with no ui argument + with _current_extension(name): + extsetup() # old extsetup with no ui argument except Exception as inst: ui.traceback(force=True) msg = util.forcebytestr(inst) @@ -424,6 +450,17 @@ def _runextsetup(name, ui): return True +# Track which extensions are enabled for the current command invocation. This is for +# debugruntest to disable extensions that were previously loaded in-process but are not +# currently active. +_loaded_extensions = set() + + +def initialload(ui): + _loaded_extensions.clear() + loadall(ui) + + def loadall(ui, include_list=None): result = ui.configitems("extensions") resultkeys = set([name for name, loc in result]) @@ -647,6 +684,8 @@ def wrapcommand(table, command, wrapper, synopsis=None, docstring=None): key = alias break + wrapper = maybe_wrap_wrapper(wrapper) + origfn = entry[0] wrap = functools.partial(util.checksignature(wrapper), util.checksignature(origfn)) _updatewrapper(wrap, origfn, wrapper) @@ -668,6 +707,8 @@ def wrapfilecache(cls, propname, wrapper): """ propname = propname assert callable(wrapper) + + wrapper = maybe_wrap_wrapper(wrapper) for currcls in cls.__mro__: if propname in currcls.__dict__: origfn = currcls.__dict__[propname].func @@ -683,6 +724,22 @@ def wrap(*args, **kwargs): raise AttributeError(r"type '%s' has no property '%s'" % (cls, propname)) +# For debugruntest, add an outer layer of wrapping so we can "disable" the inner wrapper +# if the extension that installed the wrapper is not currently enabled. +def maybe_wrap_wrapper(wrapper): + if not util.istest() or not _extension_being_loaded: + return wrapper + + extname = _extension_being_loaded + + def wrapperwrapper(origfn, *args, **kwargs): + if extname and not extname in _loaded_extensions: + return origfn(*args, **kwargs) + return wrapper(origfn, *args, **kwargs) + + return wrapperwrapper + + class wrappedfunction: """context manager for temporarily wrapping a function""" @@ -734,6 +791,8 @@ def whatever(self, *args, **kwargs): """ assert callable(wrapper) + wrapper = maybe_wrap_wrapper(wrapper) + origfn = getattr(container, funcname) assert callable(origfn) if inspect.ismodule(container): diff --git a/eden/scm/tests/test-debugruntest-extensions.t b/eden/scm/tests/test-debugruntest-extensions.t new file mode 100644 index 0000000000000..88fcd413d68d2 --- /dev/null +++ b/eden/scm/tests/test-debugruntest-extensions.t @@ -0,0 +1,41 @@ +#debugruntest-compatible + + $ cat > ex1.py < from sapling import commands, extensions + > def uisetup(ui): + > def files(orig, ui, *args, **kwargs): + > ui.status("ex1\n") + > return orig(ui, *args, **kwargs) + > extensions.wrapcommand(commands.table, "files", files) + > EOS + + $ cat > ex2.py < from sapling import commands, extensions + > def uisetup(ui): + > def files(orig, ui, *args, **kwargs): + > ui.status("ex2\n") + > return orig(ui, *args, **kwargs) + > extensions.wrapcommand(commands.table, "files", files) + > EOS + + $ newrepo + $ echo foo > foo + $ hg ci -Aqm foo + $ hg files + foo + + $ hg files --config extensions.ex1=~/ex1.py + ex1 + foo + + $ hg files --config extensions.ex2=~/ex2.py + ex2 + foo + + $ hg files --config extensions.ex2=~/ex2.py --config extensions.ex1=~/ex1.py + ex2 + ex1 + foo + + $ hg files + foo