Unknown variables from Variables file are now reported

Previously Update added everything from the file(s) to the local
values dict by doing "exec", and then unknown ones ended up silently
dropped since they weren't declared as options. Now the result
of the exec call is reprocessed so unknowns can be collected.

Fixes #4645

Signed-off-by: Mats Wichmann <mats@linux.com>

4 files changed, 39 insertions, 22 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 8ffb07d..3364b95 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..87b7c60 100644
--- a/SCons/Variables/VariablesTests.py
+++ b/SCons/Variables/VariablesTests.py
@@ -654,41 +654,42 @@ 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, one from a variables file.  Add one
+        of those later, make sure it gets removed from 'unknown' when added.
+        """
+        test = TestSCons.TestSCons()
+        var_file = test.workpath('vars.py')
+        test.write('vars.py', 'FROMFILE="notadded"')
+        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('notadded', 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")
         args = {
             'A'             : 'a',
             'ADDEDLATER'    : 'added',
         }
-
         opts.Update(env, args)
 
         r = opts.UnknownVariables()
         with self.subTest():
-            self.assertEqual(0, len(r))
+            self.assertEqual(1, len(r))  # should still have FROMFILE
+            self.assertNotIn('ADDEDLATER', r)
             self.assertEqual('added', env['ADDEDLATER'])
+            self.assertIn('FROMFILE', r)
 
     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: