Merge pull request #9185 from dcbaker/submit/env-object-as-holder

Make EnvironmentVariablesObject a  holder

6 files changed, 125 insertions, 90 deletions

diff --git a/mesonbuild/build.py b/mesonbuild/build.py
index 0143022..bc4065b 100644
--- a/mesonbuild/build.py
+++ b/mesonbuild/build.py
@@ -426,12 +426,16 @@ class ExtractedObjects(HoldableObject):
         ]
 
 class EnvironmentVariables(HoldableObject):
-    def __init__(self) -> None:
-        self.envvars = []
+    def __init__(self, values: T.Optional[T.Dict[str, str]] = None) -> None:
+        self.envvars: T.List[T.Tuple[T.Callable[[T.Dict[str, str], str, T.List[str], str], str], str, T.List[str], str]] = []
         # The set of all env vars we have operations for. Only used for self.has_name()
-        self.varnames = set()
+        self.varnames: T.Set[str] = set()
 
-    def __repr__(self):
+        if values:
+            for name, value in values.items():
+                self.set(name, [value])
+
+    def __repr__(self) -> str:
         repr_str = "<{0}: {1}>"
         return repr_str.format(self.__class__.__name__, self.envvars)
 
@@ -450,14 +454,17 @@ class EnvironmentVariables(HoldableObject):
         self.varnames.add(name)
         self.envvars.append((self._prepend, name, values, separator))
 
-    def _set(self, env: T.Dict[str, str], name: str, values: T.List[str], separator: str) -> str:
+    @staticmethod
+    def _set(env: T.Dict[str, str], name: str, values: T.List[str], separator: str) -> str:
         return separator.join(values)
 
-    def _append(self, env: T.Dict[str, str], name: str, values: T.List[str], separator: str) -> str:
+    @staticmethod
+    def _append(env: T.Dict[str, str], name: str, values: T.List[str], separator: str) -> str:
         curr = env.get(name)
         return separator.join(values if curr is None else [curr] + values)
 
-    def _prepend(self, env: T.Dict[str, str], name: str, values: T.List[str], separator: str) -> str:
+    @staticmethod
+    def _prepend(env: T.Dict[str, str], name: str, values: T.List[str], separator: str) -> str:
         curr = env.get(name)
         return separator.join(values if curr is None else values + [curr])
 
diff --git a/mesonbuild/interpreter/interpreter.py b/mesonbuild/interpreter/interpreter.py
index d104f74..c21b90e 100644
--- a/mesonbuild/interpreter/interpreter.py
+++ b/mesonbuild/interpreter/interpreter.py
@@ -42,7 +42,6 @@ from .mesonmain import MesonMain
 from .dependencyfallbacks import DependencyFallbacksHolder
 from .interpreterobjects import (
     SubprojectHolder,
-    EnvironmentVariablesObject,
     ConfigurationDataObject,
     Test,
     RunProcess,
@@ -51,6 +50,7 @@ from .interpreterobjects import (
     NullSubprojectInterpreter,
 )
 from .type_checking import (
+    ENV_KW,
     INSTALL_MODE_KW,
     LANGUAGE_KW,
     NATIVE_KW,
@@ -189,7 +189,7 @@ TEST_KWARGS: T.List[KwargInfo] = [
               listify=True, default=[], since='0.46.0'),
     KwargInfo('priority', int, default=0, since='0.52.0'),
     # TODO: env needs reworks of the way the environment variable holder itself works probably
-    KwargInfo('env', (EnvironmentVariablesObject, list, dict, str, NoneType)),
+    ENV_KW,
     KwargInfo('suite', ContainerTypeInfo(list, str), listify=True, default=['']),  # yes, a list of empty string
 ]
 
@@ -395,6 +395,7 @@ class Interpreter(InterpreterBase, HoldableObject):
             build.Data: OBJ.DataHolder,
             build.InstallDir: OBJ.InstallDirHolder,
             build.IncludeDirs: OBJ.IncludeDirsHolder,
+            build.EnvironmentVariables: OBJ.EnvironmentVariablesHolder,
             compilers.RunResult: compilerOBJ.TryRunResultHolder,
             dependencies.ExternalLibrary: OBJ.ExternalLibraryHolder,
             coredata.UserFeatureOption: OBJ.FeatureOptionHolder,
@@ -1722,19 +1723,14 @@ This will become a hard error in the future.''' % kwargs['input'], location=self
                   kwargs: 'kwargs.FuncTest') -> None:
         self.add_test(node, args, kwargs, True)
 
-    def unpack_env_kwarg(self, kwargs: T.Union[EnvironmentVariablesObject, T.Dict[str, str], T.List[str]]) -> build.EnvironmentVariables:
-        envlist = kwargs.get('env', EnvironmentVariablesObject())
-        if isinstance(envlist, EnvironmentVariablesObject):
-            env = envlist.vars
-        elif isinstance(envlist, dict):
-            FeatureNew.single_use('environment dictionary', '0.52.0', self.subproject)
-            env = EnvironmentVariablesObject(envlist)
-            env = env.vars
-        else:
-            # Convert from array to environment object
-            env = EnvironmentVariablesObject(envlist)
-            env = env.vars
-        return env
+    def unpack_env_kwarg(self, kwargs: T.Union[build.EnvironmentVariables, T.Dict[str, 'TYPE_var'], T.List['TYPE_var'], str]) -> build.EnvironmentVariables:
+        envlist = kwargs.get('env')
+        if envlist is None:
+            return build.EnvironmentVariables()
+        msg = ENV_KW.validator(envlist)
+        if msg:
+            raise InvalidArguments(f'"env": {msg}')
+        return ENV_KW.convertor(envlist)
 
     def make_test(self, node: mparser.BaseNode,
                   args: T.Tuple[str, T.Union[build.Executable, build.Jar, ExternalProgram, mesonlib.File]],
@@ -2360,17 +2356,17 @@ This will become a hard error in the future.''' % kwargs['input'], location=self
 
     @noKwargs
     @noArgsFlattening
-    def func_environment(self, node, args, kwargs):
-        if len(args) > 1:
-            raise InterpreterException('environment takes only one optional positional arguments')
-        elif len(args) == 1:
+    @typed_pos_args('environment', optargs=[(str, list, dict)])
+    def func_environment(self, node: mparser.FunctionNode, args: T.Tuple[T.Union[None, str, T.List['TYPE_var'], T.Dict[str, 'TYPE_var']]],
+                         kwargs: 'TYPE_kwargs') -> build.EnvironmentVariables:
+        init = args[0]
+        if init is not None:
             FeatureNew.single_use('environment positional arguments', '0.52.0', self.subproject)
-            initial_values = args[0]
-            if not isinstance(initial_values, dict) and not isinstance(initial_values, list):
-                raise InterpreterException('environment first argument must be a dictionary or a list')
-        else:
-            initial_values = {}
-        return EnvironmentVariablesObject(initial_values, self.subproject)
+            msg = ENV_KW.validator(init)
+            if msg:
+                raise InvalidArguments(f'"environment": {msg}')
+            return ENV_KW.convertor(init)
+        return build.EnvironmentVariables()
 
     @typed_pos_args('join_paths', varargs=str, min_varargs=1)
     @noKwargs
diff --git a/mesonbuild/interpreter/interpreterobjects.py b/mesonbuild/interpreter/interpreterobjects.py
index d524059..262ff87 100644
--- a/mesonbuild/interpreter/interpreterobjects.py
+++ b/mesonbuild/interpreter/interpreterobjects.py
@@ -17,7 +17,7 @@ from ..interpreterbase import (
                                ContainerTypeInfo, KwargInfo,
                                InterpreterObject, MesonInterpreterObject, ObjectHolder, MutableInterpreterObject,
                                FeatureCheckBase, FeatureNewKwargs, FeatureNew, FeatureDeprecated,
-                               typed_pos_args, typed_kwargs, stringArgs, permittedKwargs,
+                               typed_pos_args, typed_kwargs, permittedKwargs,
                                noArgsFlattening, noPosargs, noKwargs, permissive_unholder_return, TYPE_var, TYPE_kwargs, TYPE_nvar, TYPE_nkwargs,
                                flatten, resolve_second_level_holders, InterpreterException, InvalidArguments, InvalidCode)
 from ..interpreter.type_checking import NoneType
@@ -30,9 +30,14 @@ import typing as T
 if T.TYPE_CHECKING:
     from . import kwargs
     from .interpreter import Interpreter
-    from ..environment import Environment
     from ..envconfig import MachineInfo
 
+    from typing_extensions import TypedDict
+
+    class EnvironmentSeparatorKW(TypedDict):
+
+        separator: str
+
 
 def extract_required_kwarg(kwargs: 'kwargs.ExtractRequired',
                            subproject: str,
@@ -229,76 +234,52 @@ class RunProcess(MesonInterpreterObject):
     def stderr_method(self, args: T.List[TYPE_var], kwargs: TYPE_kwargs) -> str:
         return self.stderr
 
-# TODO: Parsing the initial values should be either done directly in the
-#       `Interpreter` or in `build.EnvironmentVariables`. This way, this class
-#       can be converted into a pure object holder.
-class EnvironmentVariablesObject(MutableInterpreterObject, MesonInterpreterObject):
-    # TODO: Move the type cheking for initial_values out of this class and replace T.Any
-    def __init__(self, initial_values: T.Optional[T.Any] = None, subproject: str = ''):
-        super().__init__(subproject=subproject)
-        self.vars = build.EnvironmentVariables()
+
+_ENV_SEPARATOR_KW = KwargInfo('separator', str, default=os.pathsep)
+
+
+class EnvironmentVariablesHolder(ObjectHolder[build.EnvironmentVariables], MutableInterpreterObject):
+
+    def __init__(self, obj: build.EnvironmentVariables, interpreter: 'Interpreter'):
+        super().__init__(obj, interpreter)
         self.methods.update({'set': self.set_method,
                              'append': self.append_method,
                              'prepend': self.prepend_method,
                              })
-        if isinstance(initial_values, dict):
-            for k, v in initial_values.items():
-                self.set_method([k, v], {})
-        elif initial_values is not None:
-            for e in mesonlib.listify(initial_values):
-                if not isinstance(e, str):
-                    raise InterpreterException('Env var definition must be a list of strings.')
-                if '=' not in e:
-                    raise InterpreterException('Env var definition must be of type key=val.')
-                (k, val) = e.split('=', 1)
-                k = k.strip()
-                val = val.strip()
-                if ' ' in k:
-                    raise InterpreterException('Env var key must not have spaces in it.')
-                self.set_method([k, val], {})
 
     def __repr__(self) -> str:
         repr_str = "<{0}: {1}>"
-        return repr_str.format(self.__class__.__name__, self.vars.envvars)
+        return repr_str.format(self.__class__.__name__, self.held_object.envvars)
 
-    def unpack_separator(self, kwargs: T.Dict[str, T.Any]) -> str:
-        separator = kwargs.get('separator', os.pathsep)
-        if not isinstance(separator, str):
-            raise InterpreterException("EnvironmentVariablesObject methods 'separator'"
-                                       " argument needs to be a string.")
-        return separator
+    def __deepcopy__(self, memo: T.Dict[str, object]) -> 'EnvironmentVariablesHolder':
+        # Avoid trying to copy the intepreter
+        return EnvironmentVariablesHolder(copy.deepcopy(self.held_object), self.interpreter)
 
     def warn_if_has_name(self, name: str) -> None:
         # Multiple append/prepend operations was not supported until 0.58.0.
-        if self.vars.has_name(name):
+        if self.held_object.has_name(name):
             m = f'Overriding previous value of environment variable {name!r} with a new one'
             FeatureNew('0.58.0', m).use(self.subproject)
 
-    @stringArgs
-    @permittedKwargs({'separator'})
     @typed_pos_args('environment.set', str, varargs=str, min_varargs=1)
-    def set_method(self, args: T.Tuple[str, T.List[str]], kwargs: T.Dict[str, T.Any]) -> None:
+    @typed_kwargs('environment.set', _ENV_SEPARATOR_KW)
+    def set_method(self, args: T.Tuple[str, T.List[str]], kwargs: 'EnvironmentSeparatorKW') -> None:
         name, values = args
-        separator = self.unpack_separator(kwargs)
-        self.vars.set(name, values, separator)
+        self.held_object.set(name, values, kwargs['separator'])
 
-    @stringArgs
-    @permittedKwargs({'separator'})
     @typed_pos_args('environment.append', str, varargs=str, min_varargs=1)
-    def append_method(self, args: T.Tuple[str, T.List[str]], kwargs: T.Dict[str, T.Any]) -> None:
+    @typed_kwargs('environment.append', _ENV_SEPARATOR_KW)
+    def append_method(self, args: T.Tuple[str, T.List[str]], kwargs: 'EnvironmentSeparatorKW') -> None:
         name, values = args
-        separator = self.unpack_separator(kwargs)
         self.warn_if_has_name(name)
-        self.vars.append(name, values, separator)
+        self.held_object.append(name, values, kwargs['separator'])
 
-    @stringArgs
-    @permittedKwargs({'separator'})
     @typed_pos_args('environment.prepend', str, varargs=str, min_varargs=1)
-    def prepend_method(self, args: T.Tuple[str, T.List[str]], kwargs: T.Dict[str, T.Any]) -> None:
+    @typed_kwargs('environment.prepend', _ENV_SEPARATOR_KW)
+    def prepend_method(self, args: T.Tuple[str, T.List[str]], kwargs: 'EnvironmentSeparatorKW') -> None:
         name, values = args
-        separator = self.unpack_separator(kwargs)
         self.warn_if_has_name(name)
-        self.vars.prepend(name, values, separator)
+        self.held_object.prepend(name, values, kwargs['separator'])
 
 
 class ConfigurationDataObject(MutableInterpreterObject, MesonInterpreterObject):
diff --git a/mesonbuild/interpreter/kwargs.py b/mesonbuild/interpreter/kwargs.py
index 77826cb..317f7c6 100644
--- a/mesonbuild/interpreter/kwargs.py
+++ b/mesonbuild/interpreter/kwargs.py
@@ -11,7 +11,6 @@ from typing_extensions import TypedDict, Literal
 from .. import build
 from .. import coredata
 from ..mesonlib import MachineChoice, File, FileMode, FileOrString
-from .interpreterobjects import EnvironmentVariablesObject
 
 
 class FuncAddProjectArgs(TypedDict):
@@ -39,7 +38,7 @@ class BaseTest(TypedDict):
     workdir: T.Optional[str]
     depends: T.List[T.Union[build.CustomTarget, build.BuildTarget]]
     priority: int
-    env: T.Union[EnvironmentVariablesObject, T.List[str], T.Dict[str, str], str]
+    env: build.EnvironmentVariables
     suite: T.List[str]
 
 
diff --git a/mesonbuild/interpreter/mesonmain.py b/mesonbuild/interpreter/mesonmain.py
index 5355c47..9c4f50a 100644
--- a/mesonbuild/interpreter/mesonmain.py
+++ b/mesonbuild/interpreter/mesonmain.py
@@ -7,13 +7,13 @@ from .. import mlog
 
 from ..mesonlib import MachineChoice, OptionKey
 from ..programs import OverrideProgram, ExternalProgram
+from ..interpreter.type_checking import ENV_KW
 from ..interpreterbase import (MesonInterpreterObject, FeatureNew, FeatureDeprecated,
                                typed_pos_args, permittedKwargs, noArgsFlattening, noPosargs, noKwargs,
                                typed_kwargs, KwargInfo, MesonVersionString, InterpreterException)
 
 from .interpreterobjects import (ExecutableHolder, ExternalProgramHolder,
-                                 CustomTargetHolder, CustomTargetIndexHolder,
-                                 EnvironmentVariablesObject)
+                                 CustomTargetHolder, CustomTargetIndexHolder)
 from .type_checking import NATIVE_KW, NoneType
 
 import typing as T
@@ -415,9 +415,10 @@ class MesonMain(MesonInterpreterObject):
 
     @FeatureNew('add_devenv', '0.58.0')
     @noKwargs
-    @typed_pos_args('add_devenv', (str, list, dict, EnvironmentVariablesObject))
-    def add_devenv_method(self, args: T.Union[str, list, dict, EnvironmentVariablesObject], kwargs: T.Dict[str, T.Any]) -> None:
+    @typed_pos_args('add_devenv', (str, list, dict, build.EnvironmentVariables))
+    def add_devenv_method(self, args: T.Tuple[T.Union[str, list, dict, build.EnvironmentVariables]], kwargs: T.Dict[str, T.Any]) -> None:
         env = args[0]
-        if isinstance(env, (str, list, dict)):
-            env = EnvironmentVariablesObject(env)
-        self.build.devenv.append(env.vars)
+        msg = ENV_KW.validator(env)
+        if msg:
+            raise build.InvalidArguments(f'"add_devenv": {msg}')
+        self.build.devenv.append(ENV_KW.convertor(env))
diff --git a/mesonbuild/interpreter/type_checking.py b/mesonbuild/interpreter/type_checking.py
index d4e178d..4eed965 100644
--- a/mesonbuild/interpreter/type_checking.py
+++ b/mesonbuild/interpreter/type_checking.py
@@ -6,9 +6,11 @@
 import typing as T
 
 from .. import compilers
+from ..build import EnvironmentVariables
 from ..coredata import UserFeatureOption
+from ..interpreterbase import TYPE_var
 from ..interpreterbase.decorators import KwargInfo, ContainerTypeInfo
-from ..mesonlib import FileMode, MachineChoice
+from ..mesonlib import FileMode, MachineChoice, listify
 
 # Helper definition for type checks that are `Optional[T]`
 NoneType: T.Type[None] = type(None)
@@ -126,3 +128,52 @@ REQUIRED_KW: KwargInfo[T.Union[bool, UserFeatureOption]] = KwargInfo(
     default=True,
     # TODO: extract_required_kwarg could be converted to a convertor
 )
+
+def _env_validator(value: T.Union[EnvironmentVariables, T.List['TYPE_var'], T.Dict[str, 'TYPE_var'], str, None]) -> T.Optional[str]:
+    def _splitter(v: str) -> T.Optional[str]:
+        split = v.split('=', 1)
+        if len(split) == 1:
+            return f'"{v}" is not two string values separated by an "="'
+        return None
+
+    if isinstance(value, str):
+        v = _splitter(value)
+        if v is not None:
+            return v
+    elif isinstance(value, list):
+        for i in listify(value):
+            if not isinstance(i, str):
+                return f"All array elements must be a string, not {i!r}"
+            v = _splitter(i)
+            if v is not None:
+                return v
+    elif isinstance(value, dict):
+        # We don't need to spilt here, just do the type checking
+        for k, dv in value.items():
+            if not isinstance(dv, str):
+                return f"Dictionary element {k} must be a string not {dv!r}"
+    # We know that otherwise we have an EnvironmentVariables object or None, and
+    # we're okay at this point
+    return None
+
+
+def _env_convertor(value: T.Union[EnvironmentVariables, T.List[str], T.Dict[str, str], str, None]) -> EnvironmentVariables:
+    def splitter(input: str) -> T.Tuple[str, str]:
+        a, b = input.split('=', 1)
+        return (a.strip(), b.strip())
+
+    if isinstance(value, (str, list)):
+        return EnvironmentVariables(dict(splitter(v) for v in listify(value)))
+    elif isinstance(value, dict):
+        return EnvironmentVariables(value)
+    elif value is None:
+        return EnvironmentVariables()
+    return value
+
+
+ENV_KW: KwargInfo[T.Union[EnvironmentVariables, T.List, T.Dict, str, None]] = KwargInfo(
+    'env',
+    (EnvironmentVariables, list, dict, str, NoneType),
+    validator=_env_validator,
+    convertor=_env_convertor,
+)