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

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)
 

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'})

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.')

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)

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)