interpreter: don't create new Interpreter instances for subprojects

Instead, replace the state with new state. This means we don't have to
re-calculate any of the methods or tell the sub-interpreter about the
state of the super-interpreter, it's already there. Since all of the
state that is not an implementation detail of the Interpreter itself is
no longer bound to the interpreter this is all very trivial.

This fixes a number of potential bugs created by side-effects from a
failing, optional, subproject (Assume A => B => C, where C configures
successfully but B fails after that)

 1. If B called `meson_override_dependency()` or
    `meson.override_find_program()` those overrides would remain, even
    though the targets would not exist.
 2. Targets from C would be built, even though B will not be, and A does
    not use them.
 3. If C called `meson.override_dependency()` or
    `meson.override_find_program()` these overrides would remain, and
    could affect A
 4. If B is optional, but a subproject D is called if B fails, and D
    calls C with different `default_options` than B, D will get a copy
    of C using those different options
 5. Summary options for C will be printed, even though it is unused

mesonbuild/interpreter/interpreter.py

@@ -269,38 +269,40 @@ def __init__(
                 ast: T.Optional[mparser.CodeBlockNode] = None,
                 relaxations: T.Optional[T.Set[InterpreterRuleRelaxation]] = None,
                 user_defined_options: T.Optional[coredata.SharedCMDOptions] = None,
-                world: T.Optional[GlobalInterpreterState] = None,
             ) -> None:
         self.state = InterpreterState(
             LocalInterpreterState(
                 subproject, subdir,
                 rule_relaxations=relaxations or set(),
             ),
-            world or GlobalInterpreterState(
+            GlobalInterpreterState(
                 _build.environment.get_source_dir(), _build,
                 user_defined_options, backend)
         )
         super().__init__()
         self.modules: T.Dict[str, NewExtensionModule] = {}
-        if ast is None:
-            self.load_root_meson_file()
-        else:
-            self.state.local.ast = ast
-        self.sanity_check_ast()
         self.builtin.update({'meson': MesonMain(self)})
         # Passed from the outside, only used in subprojects.
         if default_project_options:
             self.state.local.default_subproject_options.update(default_project_options)
         self.build_func_dict()
         self.build_holder_map()
+        self.__init(ast)
+        self._redetect_machines()
 
-        # build_def_files needs to be defined before parse_project is called
-        #
-        # For non-meson subprojects, we'll be using the ast. Even if it does
-        # exist we don't want to add a dependency on it, it's autogenerated
-        # from the actual build files, and is just for reference.
+    def __init(self, ast: T.Optional[mparser.CodeBlockNode]) -> None:
+        """Set and check the AST, then parse the `project()` function.
+
+        Helper used in initial setup and in subproject setup.
+
+        :param ast: A pre-calculated AST instance (from a translated build system) or nothing
+        """
+        if ast is None:
+            self.load_root_meson_file()
+        else:
+            self.state.local.ast = ast
+        self.sanity_check_ast()
         self.parse_project()
-        self._redetect_machines()
 
     @property
     def environment(self) -> environment.Environment:
@@ -922,13 +924,20 @@ def do_subproject(self, subp_name: str, kwargs: kwtypes.DoSubproject, force_meth
             'cargo': self._do_subproject_cargo,
         }
 
+        # snapshot the state, this allows us to rellback the state if the subproject fails
+        # The GlobalState may be altered by the subproject, and the local state
+        # may not have been returned to this project's state
+        snapshot = self.state.copy()
         try:
             return methods_map[method](subp_name, subdir, default_options, kwargs)
         # Invalid code is always an error
         except InvalidCode:
             raise
         except Exception as e:
             if not required:
+                # Restore the original global state
+                self.state = snapshot
+
                 with mlog.nested(subp_name):
                     # Suppress the 'ERROR:' prefix because this exception is not
                     # fatal and VS CI treat any logs with "ERROR:" as fatal.
@@ -943,61 +952,50 @@ def _do_subproject_meson(self, subp_name: str, subdir: str,
                              ast: T.Optional[mparser.CodeBlockNode] = None,
                              build_def_files: T.Optional[T.List[str]] = None,
                              relaxations: T.Optional[T.Set[InterpreterRuleRelaxation]] = None) -> SubprojectState:
-        with mlog.nested(subp_name):
-            if ast:
-                # Debug print the generated meson file
-                from ..ast import AstIndentationGenerator, AstPrinter
-                printer = AstPrinter(update_ast_line_nos=True)
-                ast.accept(AstIndentationGenerator())
-                ast.accept(printer)
-                printer.post_process()
-                meson_filename = os.path.join(self.state.world.build.environment.get_build_dir(), subdir, 'meson.build')
-                with open(meson_filename, "w", encoding='utf-8') as f:
-                    f.write(printer.result)
-                mlog.log('Generated Meson AST:', meson_filename)
-                mlog.cmd_ci_include(meson_filename)
-
-            old_build = self.state.world.build
-            self.state.world.build = old_build.copy()
-            subi = Interpreter(self.state.world.build, self.state.world.backend, subp_name, subdir,
-                               default_options, ast=ast,
-                               relaxations=relaxations,
-                               user_defined_options=self.state.world.user_defined_options,
-                               world=self.state.world)
-            # Those lists are shared by all interpreters. That means that
-            # even if the subproject fails, any modification that the subproject
-            # made to those lists will affect the parent project.
-            subi.state.world.subprojects = self.state.world.subprojects
-            subi.modules = self.modules
-            subi.holder_map = self.holder_map
-            subi.bound_holder_map = self.bound_holder_map
-
-            subi.state.local.subproject_stack = self.state.local.subproject_stack + [subp_name]
-            current_active = self.state.local.project_name
-            with mlog.nested_warnings():
-                subi.run()
-                subi_warnings = mlog.get_warning_count()
-            mlog.log('Subproject', mlog.bold(subp_name), 'finished.')
-
-        mlog.log()
-
-        if kwargs['version']:
-            pv = subi.state.local.project_version
-            wanted = kwargs['version']
-            if pv == 'undefined' or not mesonlib.version_compare_many(pv, wanted)[0]:
-                raise InterpreterException(f'Subproject {subp_name} version is {pv} but {wanted} required.')
-        self.state.local.project_name = current_active
-        self.state.world.subprojects.update(subi.state.world.subprojects)
-        self.state.world.subprojects[subp_name] = SubprojectState(
-            subi.state.local, subdir, warnings=subi_warnings, callstack=self.state.local.subproject_stack)
-        # Duplicates are possible when subproject uses files from project root
-        if build_def_files:
-            self.state.world.build_def_files.update(build_def_files)
-        # We always need the subi.build_def_files, to propagate sub-sub-projects
-        old_build.merge(subi.state.world.build)
-        self.state.world.build = old_build
-        self.state.world.build.subprojects[subp_name] = subi.state.local.project_version
-        return self.state.world.subprojects[subp_name]
+
+        # Create a new local state for the new subproject
+        local = LocalInterpreterState(
+            subp_name, subdir,
+            rule_relaxations=relaxations or set(),
+            default_subproject_options=default_options,
+            subproject_stack=self.state.local.subproject_stack + [subp_name],
+        )
+        with self.state.subproject(local) as current_state:
+            with mlog.nested(subp_name):
+                if ast:
+                    # Debug print the generated meson file
+                    from ..ast import AstIndentationGenerator, AstPrinter
+                    printer = AstPrinter(update_ast_line_nos=True)
+                    ast.accept(AstIndentationGenerator())
+                    ast.accept(printer)
+                    printer.post_process()
+                    meson_filename = os.path.join(self.state.world.build.environment.get_build_dir(), subdir, 'meson.build')
+                    with open(meson_filename, "w", encoding='utf-8') as f:
+                        f.write(printer.result)
+                    mlog.log('Generated Meson AST:', meson_filename)
+                    mlog.cmd_ci_include(meson_filename)
+
+                with mlog.nested_warnings():
+                    self.__init(ast)
+                    self.run()
+                    subi_warnings = mlog.get_warning_count()
+                mlog.log('Subproject', mlog.bold(subp_name), 'finished.')
+
+            mlog.log()
+
+            if kwargs['version']:
+                pv = self.state.local.project_version
+                wanted = kwargs['version']
+                if pv == 'undefined' or not mesonlib.version_compare_many(pv, wanted)[0]:
+                    raise InterpreterException(f'Subproject {subp_name} version is {pv} but {wanted} required.')
+            self.state.world.subprojects[subp_name] = SubprojectState(
+                self.state.local, subdir, warnings=subi_warnings, callstack=current_state.subproject_stack)
+            self.state.world.build.subprojects[subp_name] = self.state.local.project_version
+            # Duplicates are possible when subproject uses files from project root
+            if build_def_files:
+                self.state.world.build_def_files.update(build_def_files)
+
+            return self.state.world.subprojects[subp_name]
 
     def _do_subproject_cmake(self, subp_name: str, subdir: str,
                              default_options: T.Dict[OptionKey, str],

mesonbuild/interpreter/state.py

@@ -4,6 +4,7 @@
 """Implementation of Interpreter state for the primary Interpreter."""
 
 from __future__ import annotations
+import contextlib
 import dataclasses
 import typing as T
 
@@ -75,7 +76,7 @@ class LocalInterpreterState(LocalState):
     """
 
     default_subproject_options: T.Dict[OptionKey, str] = dataclasses.field(
-        default_factory=dict, init=False)
+        default_factory=dict)
     """Options passed to subprojects via the `dependency(default_options)` keyword argument.
 
     See also :attr:`project_default_options`.
@@ -119,9 +120,36 @@ def __post_init__(self) -> None:
         self.environment = self.build.environment
         self.coredata = self.environment.coredata
 
+    def copy(self) -> GlobalInterpreterState:
+        c = GlobalInterpreterState(
+            self.source_root, self.build.copy(),
+            self.user_defined_options, self.backend)
+        c.summary = self.summary.copy()
+        c.subprojects = self.subprojects.copy()
+        c.args_frozen = self.args_frozen
+        c.build_def_files = OrderedSet(self.build_def_files)
+        return c
+
 
 @dataclasses.dataclass
 class InterpreterState(State):
 
     local: LocalInterpreterState
     world: GlobalInterpreterState
+
+    def copy(self) -> InterpreterState:
+        return InterpreterState(self.local, self.world.copy())
+
+    @contextlib.contextmanager
+    def subproject(self, new: LocalInterpreterState) -> T.Iterator[LocalInterpreterState]:
+        """Replace the local state with a new one, and ensure it's set back
+
+        :param new: the new state to use
+        :yield: the old state
+        """
+        old = self.local
+        self.local = new
+        try:
+            yield old
+        finally:
+            self.local = old

test cases/common/267 default_options in find_program/meson.build

@@ -3,3 +3,10 @@ project('test default_options in find_program')
 dummy_exe = find_program('dummy', default_options: ['subproject_option=true'])
 
 test('test_dummy', dummy_exe)
+
+# Test that a suproject that fails does not alter global state, specifically if
+# A => B, where B alters that state, but A fails after successfully configuring B
+subproject('will_fail', required : false)
+
+p = find_program('python')
+assert(p.version().version_compare('< 1000'), 'failed subproject altered global state')

unittests/allplatformstests.py

@@ -3701,10 +3701,6 @@ def test_summary(self):
                 integer: 1
                 boolean: true
 
-            subsub undefined
-
-                Something: Some value
-
             My Project 1.0
 
               Configuration
@@ -3736,7 +3732,6 @@ def test_summary(self):
               Subprojects
                 sub            : YES
                 sub2           : NO Problem encountered: This subproject failed
-                subsub         : YES (from sub2)
 
               User defined options
                 backend        : ''' + self.backend_name + '''