Merge pull request #8905 from mensinda/refactorFix

fix: Fix set_variable not holderifying (fixes #8904)

mesonbuild/interpreter/interpreter.py

@@ -752,14 +752,7 @@ external dependencies (including libraries) must go to "dependencies".''')
     def func_subproject(self, nodes, args, kwargs):
         if len(args) != 1:
             raise InterpreterException('Subproject takes exactly one argument')
-        subp_name = args[0]
-        subp = self.do_subproject(subp_name, 'meson', kwargs)
-        # Update the holder maps from the subproject. Additional entries to the
-        # holder maps can be added through imported Meson modules
-        if isinstance(subp.held_object, Interpreter):
-            self.holder_map.update(subp.held_object.holder_map)
-            self.bound_holder_map.update(subp.held_object.bound_holder_map)
-        return subp
+        return self.do_subproject(args[0], 'meson', kwargs)
 
     def disabled_subproject(self, subp_name, disabled_feature=None, exception=None):
         sub = SubprojectHolder(NullSubprojectInterpreter(), os.path.join(self.subproject_dir, subp_name),
@@ -824,7 +817,7 @@ external dependencies (including libraries) must go to "dependencies".''')
             elif method == 'cmake':
                 return self._do_subproject_cmake(subp_name, subdir, subdir_abs, default_options, kwargs)
             else:
-                raise InterpreterException(f'The method {method} is invalid for the subproject {subp_name}')
+                raise mesonlib.MesonBugException(f'The method {method} is invalid for the subproject {subp_name}')
         # Invalid code is always an error
         except InvalidCode:
             raise
@@ -879,6 +872,10 @@ external dependencies (including libraries) must go to "dependencies".''')
         self.build.subprojects[subp_name] = subi.project_version
         self.coredata.initialized_subprojects.add(subp_name)
         self.summary.update(subi.summary)
+        # Update the holder maps from the subproject. Additional entries to the
+        # holder maps can be added through imported Meson modules
+        self.holder_map.update(subi.holder_map)
+        self.bound_holder_map.update(subi.bound_holder_map)
         return self.subprojects[subp_name]
 
     def _do_subproject_cmake(self, subp_name, subdir, subdir_abs, default_options, kwargs):
@@ -2671,7 +2668,7 @@ This will become a hard error in the future.''', location=self.current_node)
         if len(args) != 2:
             raise InvalidCode('Set_variable takes two arguments.')
         varname, value = args
-        self.set_variable(varname, value)
+        self.set_variable(varname, value, holderify=True)
 
     @noKwargs
     @noArgsFlattening

mesonbuild/interpreterbase/interpreterbase.py

@@ -16,7 +16,7 @@
 # or an interpreter-based tool.
 
 from .. import mparser, mesonlib, mlog
-from .. import environment, dependencies
+from .. import environment
 
 from .baseobjects import (
     InterpreterObject,
@@ -592,7 +592,7 @@ The result of this is undefined and will become a hard error in a future Meson r
         obj.current_node = node
         return self._holderify(obj.method_call(method_name, args, kwargs))
 
-    def _holderify(self, res: T.Optional[TYPE_var]) -> T.Union[TYPE_elementary, InterpreterObject]:
+    def _holderify(self, res: T.Union[TYPE_var, InterpreterObject, None]) -> T.Union[TYPE_elementary, InterpreterObject]:
         if res is None:
             return None
         if isinstance(res, (int, bool, str)):
@@ -799,7 +799,7 @@ The result of this is undefined and will become a hard error in a future Meson r
             index = posargs[0]
             fallback = None
             if len(posargs) == 2:
-                fallback = posargs[1]
+                fallback = self._holderify(posargs[1])
             elif len(posargs) > 2:
                 m = 'Array method \'get()\' only takes two arguments: the ' \
                     'index and an optional fallback value if the index is ' \
@@ -845,7 +845,7 @@ The result of this is undefined and will become a hard error in a future Meson r
                 return obj[key]
 
             if len(posargs) == 2:
-                fallback = posargs[1]
+                fallback = self._holderify(posargs[1])
                 if isinstance(fallback, mparser.BaseNode):
                     return self.evaluate_statement(fallback)
                 return fallback
@@ -909,20 +909,34 @@ To specify a keyword argument, use : instead of =.''')
             raise InvalidArguments('Tried to assign value to a non-variable.')
         value = self.evaluate_statement(node.value)
         if not self.is_assignable(value):
-            raise InvalidCode('Tried to assign an invalid value to variable.')
+            raise InvalidCode(f'Tried to assign the invalid value "{value}" of type {type(value).__name__} to variable.')
         # For mutable objects we need to make a copy on assignment
         if isinstance(value, MutableInterpreterObject):
             value = copy.deepcopy(value)
         self.set_variable(var_name, value)
         return None
 
-    def set_variable(self, varname: str, variable: T.Union[TYPE_var, InterpreterObject]) -> None:
+    def set_variable(self, varname: str, variable: T.Union[TYPE_var, InterpreterObject], *, holderify: bool = False) -> None:
         if variable is None:
             raise InvalidCode('Can not assign None to variable.')
+        if holderify:
+            variable = self._holderify(variable)
+        else:
+            # Ensure that we are never storing a HoldableObject
+            def check(x: T.Union[TYPE_var, InterpreterObject]) -> None:
+                if isinstance(x, mesonlib.HoldableObject):
+                    raise mesonlib.MesonBugException(f'set_variable in InterpreterBase called with a HoldableObject {x} of type {type(x).__name__}')
+                elif isinstance(x, list):
+                    for y in x:
+                        check(y)
+                elif isinstance(x, dict):
+                    for v in x.values():
+                        check(v)
+            check(variable)
         if not isinstance(varname, str):
             raise InvalidCode('First argument to set_variable must be a string.')
         if not self.is_assignable(variable):
-            raise InvalidCode('Assigned value not of assignable type.')
+            raise InvalidCode(f'Assigned value "{variable}" of type {type(variable).__name__} is not an assignable type.')
         if re.match('[_a-zA-Z][_0-9a-zA-Z]*$', varname) is None:
             raise InvalidCode('Invalid variable name: ' + varname)
         if varname in self.builtin:
@@ -937,8 +951,7 @@ To specify a keyword argument, use : instead of =.''')
         raise InvalidCode('Unknown variable "%s".' % varname)
 
     def is_assignable(self, value: T.Any) -> bool:
-        return isinstance(value, (InterpreterObject, dependencies.Dependency,
-                                  str, int, list, dict, mesonlib.File))
+        return isinstance(value, (InterpreterObject, str, int, list, dict))
 
     def validate_extraction(self, buildtarget: mesonlib.HoldableObject) -> None:
         raise InterpreterException('validate_extraction is not implemented in this context (please file a bug)')

test cases/common/242 set and get variable/meson.build

@@ -0,0 +1,63 @@
+project('set and get')
+
+var1 = 'test1.txt'
+var2 = files('test1.txt')[0]
+
+# Use is_disabler for accessing variables
+assert(var1 == 'test1.txt')
+assert(not is_disabler(var2))
+
+# Ensure that set variables behave correctly
+set_variable('var3', 'test2.txt')
+set_variable('var4', files('test2.txt')[0])
+
+assert(var3 == 'test2.txt')
+assert(not is_disabler(var4))
+
+# Test get_variable directly
+assert(get_variable('var1') == 'test1.txt')
+assert(not is_disabler(get_variable('var2')))
+assert(get_variable('var3') == 'test2.txt')
+assert(not is_disabler(get_variable('var4')))
+
+# Test get_variable indirectly
+
+var5 = get_variable('var1')
+var6 = get_variable('var2')
+var7 = get_variable('var3')
+var8 = get_variable('var4')
+set_variable('var9', get_variable('var7'))
+set_variable('var0', get_variable('var8'))
+
+assert(var5 == 'test1.txt')
+assert(not is_disabler(var6))
+assert(var7 == 'test2.txt')
+assert(not is_disabler(var8))
+assert(get_variable('var9') == 'test2.txt')
+assert(not is_disabler(get_variable('var0')))
+
+# test dict get
+dict = {'a': var2}
+
+dict_t1 = dict['a']
+dict_t2 = dict.get('a')
+dict_t3 = dict.get('a', var2)
+dict_t4 = dict.get('b', var2)
+
+assert(not is_disabler(dict_t1))
+assert(not is_disabler(dict_t2))
+assert(not is_disabler(dict_t3))
+assert(not is_disabler(dict_t4))
+
+# test lists
+list = [var2]
+
+list_t1 = list[0]
+list_t2 = list.get(0)
+list_t3 = list.get(0, var2)
+list_t4 = list.get(1, var2)
+
+assert(not is_disabler(list_t1))
+assert(not is_disabler(list_t2))
+assert(not is_disabler(list_t3))
+assert(not is_disabler(list_t4))