interpreter: Warn when environment() ops are overriden

Warn when someone tries to use append() or prepend() on an env var
which already has an operation set on it. People seem to think that
multiple append/prepend operations stack, but they don't.

Closes https://github.com/mesonbuild/meson/issues/5087

5 files changed, 48 insertions, 1 deletions

diff --git a/mesonbuild/build.py b/mesonbuild/build.py
index 020c47b..dae94b6 100644
--- a/mesonbuild/build.py
+++ b/mesonbuild/build.py
@@ -296,11 +296,20 @@ class ExtractedObjects:
 class EnvironmentVariables:
     def __init__(self):
         self.envvars = []
+        # The set of all env vars we have operations for. Only used for self.has_name()
+        self.varnames = set()
 
     def __repr__(self):
         repr_str = "<{0}: {1}>"
         return repr_str.format(self.__class__.__name__, self.envvars)
 
+    def add_var(self, method, name, args, kwargs):
+        self.varnames.add(name)
+        self.envvars.append((method, name, args, kwargs))
+
+    def has_name(self, name):
+        return name in self.varnames
+
     def get_value(self, values, kwargs):
         separator = kwargs.get('separator', os.pathsep)
 
diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py
index 1cf56a2..993ebbd 100644
--- a/mesonbuild/interpreter.py
+++ b/mesonbuild/interpreter.py
@@ -235,7 +235,13 @@ class EnvironmentVariablesHolder(MutableInterpreterObject, ObjectHolder):
             raise InterpreterException("EnvironmentVariablesHolder methods require at least"
                                        "2 arguments, first is the name of the variable and"
                                        " following one are values")
-        self.held_object.envvars.append((method, args[0], args[1:], kwargs))
+        # Warn when someone tries to use append() or prepend() on an env var
+        # which already has an operation set on it. People seem to think that
+        # multiple append/prepend operations stack, but they don't.
+        if method != self.held_object.set and self.held_object.has_name(args[0]):
+            mlog.warning('Overriding previous value of environment variable {!r} with a new one'
+                         .format(args[0]), location=self.current_node)
+        self.held_object.add_var(method, args[0], args[1:], kwargs)
 
     @stringArgs
     @permittedKwargs({'separator'})
diff --git a/run_unittests.py b/run_unittests.py
index 99e3c49..156e4b8 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -1672,6 +1672,17 @@ class AllPlatformTests(BasePlatformTests):
         self.build()
         self.run_tests()
 
+    def test_env_ops_dont_stack(self):
+        '''
+        Test that env ops prepend/append do not stack, and that this usage issues a warning
+        '''
+        testdir = os.path.join(self.unit_test_dir, '59 test env doesn\'t stack')
+        out = self.init(testdir)
+        self.assertRegex(out, r'WARNING: Overriding.*TEST_VAR_APPEND')
+        self.assertRegex(out, r'WARNING: Overriding.*TEST_VAR_PREPEND')
+        self.assertNotRegex(out, r'WARNING: Overriding.*TEST_VAR_SET')
+        self.run_tests()
+
     def test_testsetups(self):
         if not shutil.which('valgrind'):
             raise unittest.SkipTest('Valgrind not installed.')
diff --git a/test cases/unit/59 test env doesn't stack/meson.build b/test cases/unit/59 test env doesn't stack/meson.build
new file mode 100644
index 0000000..01f2637
--- /dev/null
+++ b/test cases/unit/59 test env doesn't stack/meson.build
@@ -0,0 +1,12 @@
+project('test env var stacking')
+
+testenv = environment()
+testenv.set('TEST_VAR_SET', 'some-value')
+testenv.set('TEST_VAR_APPEND', 'some-value')
+testenv.set('TEST_VAR_PREPEND', 'some-value')
+
+testenv.append('TEST_VAR_APPEND', 'another-value-append', separator: ':')
+testenv.prepend('TEST_VAR_PREPEND', 'another-value-prepend', separator: ':')
+testenv.set('TEST_VAR_SET', 'another-value-set')
+
+test('check env', find_program('script.py'), env: testenv)
diff --git a/test cases/unit/59 test env doesn't stack/script.py b/test cases/unit/59 test env doesn't stack/script.py
new file mode 100755
index 0000000..2a76673
--- /dev/null
+++ b/test cases/unit/59 test env doesn't stack/script.py
@@ -0,0 +1,9 @@
+#!/usr/bin/env python3
+
+import os
+
+for name in ('append', 'prepend', 'set'):
+    envname = 'TEST_VAR_' + name.upper()
+    value = 'another-value-' + name
+    envvalue = os.environ[envname]
+    assert (envvalue == value), (name, envvalue)