Merge pull request #4653 from mwichmann/issue/varfile-unknowns

Unknown variables from Variables file are now reported

4 files changed, 41 insertions, 21 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index cf7e383..e65ba36 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -150,6 +150,8 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       types depending on whether zero, one, or multiple construction
       variable names are given.
     - Update Clean and NoClean documentation.
+    - Make sure unknown variables from a Variables file are recognized
+      as such (issue #4645)
 
 
 RELEASE 4.8.1 -  Tue, 03 Sep 2024 17:22:20 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index 77afdef..a73e654 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -137,6 +137,9 @@ FIXES
 - Skip running a few validation tests if the user is root and the test is
   not designed to work for the root user.
 
+- Make sure unknown variables from a Variables file are recognized
+  as such (issue #4645)
+
 IMPROVEMENTS
 ------------
 
diff --git a/SCons/Variables/VariablesTests.py b/SCons/Variables/VariablesTests.py
index 0791ea2..80c4d11 100644
--- a/SCons/Variables/VariablesTests.py
+++ b/SCons/Variables/VariablesTests.py
@@ -654,41 +654,45 @@ class UnknownVariablesTestCase(unittest.TestCase):
         self.assertEqual('answer', env['ANSWER'])
 
     def test_AddOptionUpdatesUnknown(self) -> None:
-        """Test updating of the 'unknown' dict"""
-        opts = SCons.Variables.Variables()
-
-        opts.Add('A',
-                 'A test variable',
-                 "1")
+        """Test updating of the 'unknown' dict.
 
+        Get one unknown from args and one from a variables file.
+        Add these later, making sure they no longer appear in unknowns
+        after the subsequent Update().
+        """
+        test = TestSCons.TestSCons()
+        var_file = test.workpath('vars.py')
+        test.write('vars.py', 'FROMFILE="added"')
+        opts = SCons.Variables.Variables(files=var_file)
+        opts.Add('A', 'A test variable', "1")
         args = {
             'A'             : 'a',
             'ADDEDLATER'    : 'notaddedyet',
         }
-
         env = Environment()
         opts.Update(env,args)
 
         r = opts.UnknownVariables()
         with self.subTest():
-            self.assertEqual({'ADDEDLATER': 'notaddedyet'}, r)
+            self.assertEqual('notaddedyet', r['ADDEDLATER'])
+            self.assertEqual('added', r['FROMFILE'])
             self.assertEqual('a', env['A'])
 
-        opts.Add('ADDEDLATER',
-                 'An option not present initially',
-                 "1")
-
+        opts.Add('ADDEDLATER', 'An option not present initially', "1")
+        opts.Add('FROMFILE', 'An option from a file also absent', "1")
         args = {
             'A'             : 'a',
             'ADDEDLATER'    : 'added',
         }
-
         opts.Update(env, args)
 
         r = opts.UnknownVariables()
         with self.subTest():
             self.assertEqual(0, len(r))
+            self.assertNotIn('ADDEDLATER', r)
             self.assertEqual('added', env['ADDEDLATER'])
+            self.assertNotIn('FROMFILE', r)
+            self.assertEqual('added', env['FROMFILE'])
 
     def test_AddOptionWithAliasUpdatesUnknown(self) -> None:
         """Test updating of the 'unknown' dict (with aliases)"""
diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py
index 2d16007..1826c64 100644
--- a/SCons/Variables/__init__.py
+++ b/SCons/Variables/__init__.py
@@ -235,19 +235,30 @@ class Variables:
         for filename in self.files:
             # TODO: issue #816 use Node to access saved-variables file?
             if os.path.exists(filename):
-                # lint: W0622: Redefining built-in 'dir'
-                dir = os.path.split(os.path.abspath(filename))[0]
-                if dir:
-                    sys.path.insert(0, dir)
+                # issue #4645: don't exec directly into values,
+                #   so we can iterate through for unknown variables.
+                temp_values = {}
+                dirname = os.path.split(os.path.abspath(filename))[0]
+                if dirname:
+                    sys.path.insert(0, dirname)
                 try:
-                    values['__name__'] = filename
+                    temp_values['__name__'] = filename
                     with open(filename) as f:
                         contents = f.read()
-                    exec(contents, {}, values)
+                    exec(contents, {}, temp_values)
                 finally:
-                    if dir:
+                    if dirname:
                         del sys.path[0]
-                    del values['__name__']
+                    del temp_values['__name__']
+
+                for arg, value in temp_values.items():
+                    added = False
+                    for option in self.options:
+                        if arg in option.aliases + [option.key,]:
+                            values[option.key] = value
+                            added = True
+                    if not added:
+                        self.unknown[arg] = value
 
         # set the values specified on the command line
         if args is None: