Merge pull request #4329 from mwichmann/bug/scan-dict-convert

Fix dictifyCPPDEFINES handling of macro strings

4 files changed, 141 insertions, 45 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index c6f35faff..c5949793d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -9,9 +9,12 @@ NOTE: 4.3.0 now requires Python 3.6.0 and above. Python 3.5.x is no longer suppo
 
 RELEASE  VERSION/DATE TO BE FILLED IN LATER
 
-      From John Doe:
-
-        - Whatever John Doe did.
+  From Mats Wichmann
+    - C scanner's dictifyCPPDEFINES routine did not understand the possible
+      combinations of CPPDEFINES - not aware of a "name=value" string either
+      embedded in a sequence, or by itself.  The conditional C scanner thus
+      did not always properly apply the defines. The regular C scanner does
+      not use these, so was not affected.  [fixes #4193]
 
 
 RELEASE 4.5.2 -  Sun, 21 Mar 2023 14:08:29 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index 769a12dbe..4f6f0a828 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -32,7 +32,11 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
 FIXES
 -----
 
-- List fixes of outright bugs
+- C scanner's dictifyCPPDEFINES routine did not understand the possible
+  combinations of CPPDEFINES - not aware of a "name=value" string either
+  embedded in a sequence, or by itself.  The conditional C scanner thus
+  did not always properly apply the defines. The regular C scanner does
+  not use these, so was not affected.  [fixes #4193]
 
 IMPROVEMENTS
 ------------
diff --git a/SCons/Scanner/C.py b/SCons/Scanner/C.py
index a066104e5..31ab7e62e 100644
--- a/SCons/Scanner/C.py
+++ b/SCons/Scanner/C.py
@@ -21,7 +21,12 @@
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-"""Dependency scanner for C/C++ code."""
+"""Dependency scanner for C/C++ code.
+
+Two scanners are defined here: the default CScanner, and the optional
+CConditionalScanner, which must be explicitly selected by calling
+add_scanner() for each affected suffix.
+"""
 
 import SCons.Node.FS
 import SCons.cpp
@@ -62,25 +67,60 @@ class SConsCPPScanner(SCons.cpp.PreProcessor):
             return ''
 
 def dictify_CPPDEFINES(env) -> dict:
-    """Returns CPPDEFINES converted to a dict."""
+    """Returns CPPDEFINES converted to a dict.
+
+    This should be similar to :func:`~SCons.Defaults.processDefines`.
+    Unfortunately, we can't do the simple thing of calling that routine and
+    passing the result to the dict() constructor, because it turns the defines
+    into a list of "name=value" pairs, which the dict constructor won't
+    consume correctly.  Also cannot just call dict on CPPDEFINES itself - it's
+    fine if it's stored in the converted form (currently deque of tuples), but
+    CPPDEFINES could be in other formats too.
+
+    So we have to do all the work here - keep concepts in sync with
+    ``processDefines``.
+    """
     cppdefines = env.get('CPPDEFINES', {})
+    result = {}
     if cppdefines is None:
-        return {}
+        return result
+
+    if SCons.Util.is_Tuple(cppdefines):
+        try:
+            return {cppdefines[0]: cppdefines[1]}
+        except IndexError:
+            return {cppdefines[0]: None}
+
     if SCons.Util.is_Sequence(cppdefines):
-        result = {}
         for c in cppdefines:
             if SCons.Util.is_Sequence(c):
                 try:
                     result[c[0]] = c[1]
                 except IndexError:
-                    # it could be a one-item sequence
+                    # could be a one-item sequence
                     result[c[0]] = None
+            elif SCons.Util.is_String(c):
+                try:
+                    name, value = c.split('=')
+                    result[name] = value
+                except ValueError:
+                    result[c] = None
             else:
+                # don't really know what to do here
                 result[c] = None
         return result
-    if not SCons.Util.is_Dict(cppdefines):
-        return {cppdefines : None}
-    return cppdefines
+
+    if SCons.Util.is_String(cppdefines):
+        try:
+            name, value = cppdefines.split('=')
+            return {name: value}
+        except ValueError:
+            return {cppdefines: None}
+
+    if SCons.Util.is_Dict(cppdefines):
+        return cppdefines
+
+    return {cppdefines: None}
 
 class SConsCPPScannerWrapper:
     """The SCons wrapper around a cpp.py scanner.
diff --git a/SCons/Scanner/CTests.py b/SCons/Scanner/CTests.py
index 14e156e71..0f62198d6 100644
--- a/SCons/Scanner/CTests.py
+++ b/SCons/Scanner/CTests.py
@@ -91,6 +91,27 @@ int main(void)
 }
 """)
 
+# include using a macro, defined in source file
+test.write('f9a.c', """\
+#define HEADER "f9.h"
+#include HEADER
+
+int main(void)
+{
+   return 0;
+}
+""")
+
+# include using a macro, not defined in source file
+test.write('f9b.c', """\
+#include HEADER
+
+int main(void)
+{
+   return 0;
+}
+""")
+
 
 # for Emacs -> "
 
@@ -99,7 +120,7 @@ test.subdir('d1', ['d1', 'd2'])
 headers = ['f1.h','f2.h', 'f3-test.h', 'fi.h', 'fj.h', 'never.h',
            'd1/f1.h', 'd1/f2.h', 'd1/f3-test.h', 'd1/fi.h', 'd1/fj.h',
            'd1/d2/f1.h', 'd1/d2/f2.h', 'd1/d2/f3-test.h',
-           'd1/d2/f4.h', 'd1/d2/fi.h', 'd1/d2/fj.h']
+           'd1/d2/f4.h', 'd1/d2/fi.h', 'd1/d2/fj.h', 'f9.h']
 
 for h in headers:
     test.write(h, " ")
@@ -224,7 +245,7 @@ def deps_match(self, deps, headers):
     global my_normpath
     scanned = list(map(my_normpath, list(map(str, deps))))
     expect = list(map(my_normpath, headers))
-    self.assertTrue(scanned == expect, "expect %s != scanned %s" % (expect, scanned))
+    self.assertTrue(scanned == expect, f"expect {expect} != scanned {scanned}")
 
 # define some tests:
 
@@ -443,7 +464,7 @@ class CScannerTestCase15(unittest.TestCase):
         env = DummyEnvironment(CPPSUFFIXES = suffixes)
         s = SCons.Scanner.C.CScanner()
         for suffix in suffixes:
-            assert suffix in s.get_skeys(env), "%s not in skeys" % suffix
+            assert suffix in s.get_skeys(env), f"{suffix} not in skeys"
 
 
 class CConditionalScannerTestCase1(unittest.TestCase):
@@ -490,42 +511,70 @@ class CConditionalScannerTestCase3(unittest.TestCase):
         headers = ['d1/f1.h']
         deps_match(self, deps, headers)
 
+
+class CConditionalScannerTestCase4(unittest.TestCase):
+    def runTest(self):
+        """Test that dependency is detected if #include uses a macro."""
+
+        with self.subTest("macro defined in the source file"):
+            env = DummyEnvironment()
+            s = SCons.Scanner.C.CConditionalScanner()
+            deps = s(env.File('f9a.c'), env, s.path(env))
+            headers = ['f9.h']
+            deps_match(self, deps, headers)
+
+        with self.subTest("macro defined on the command line"):
+            env = DummyEnvironment(CPPDEFINES='HEADER=\\"f9.h\\"')
+            #env = DummyEnvironment(CPPDEFINES=['HEADER=\\"f9.h\\"'])
+            s = SCons.Scanner.C.CConditionalScanner()
+            deps = s(env.File('f9b.c'), env, s.path(env))
+            headers = ['f9.h']
+            deps_match(self, deps, headers)
+
+
 class dictify_CPPDEFINESTestCase(unittest.TestCase):
     def runTest(self):
-        """Make sure single-item tuples convert correctly.
+        """Make sure CPPDEFINES converts correctly.
 
-        This is a regression test: AppendUnique turns sequences into
-        lists of tuples, and dictify could gack on these.
+        Various types and combinations of types could fail if not handled
+        specifically by dictify_CPPDEFINES - this is a regression test.
         """
-        env = DummyEnvironment(CPPDEFINES=(("VALUED_DEFINE", 1), ("UNVALUED_DEFINE", )))
-        d = SCons.Scanner.C.dictify_CPPDEFINES(env)
-        expect = {'VALUED_DEFINE': 1, 'UNVALUED_DEFINE': None}
-        assert d == expect
-
-def suite():
-    suite = unittest.TestSuite()
-    suite.addTest(CScannerTestCase1())
-    suite.addTest(CScannerTestCase2())
-    suite.addTest(CScannerTestCase3())
-    suite.addTest(CScannerTestCase4())
-    suite.addTest(CScannerTestCase5())
-    suite.addTest(CScannerTestCase6())
-    suite.addTest(CScannerTestCase8())
-    suite.addTest(CScannerTestCase9())
-    suite.addTest(CScannerTestCase10())
-    suite.addTest(CScannerTestCase11())
-    suite.addTest(CScannerTestCase12())
-    suite.addTest(CScannerTestCase13())
-    suite.addTest(CScannerTestCase14())
-    suite.addTest(CScannerTestCase15())
-    suite.addTest(CConditionalScannerTestCase1())
-    suite.addTest(CConditionalScannerTestCase2())
-    suite.addTest(CConditionalScannerTestCase3())
-    suite.addTest(dictify_CPPDEFINESTestCase())
-    return suite
+        with self.subTest("tuples in a sequence, including one without value"):
+            env = DummyEnvironment(CPPDEFINES=[("VALUED", 1), ("UNVALUED",)])
+            d = SCons.Scanner.C.dictify_CPPDEFINES(env)
+            expect = {"VALUED": 1, "UNVALUED": None}
+            self.assertEqual(d, expect)
+
+        with self.subTest("tuple-valued define by itself"):
+            env = DummyEnvironment(CPPDEFINES=("STRING", "VALUE"))
+            d = SCons.Scanner.C.dictify_CPPDEFINES(env)
+            expect = {"STRING": "VALUE"}
+            self.assertEqual(d, expect)
+
+        with self.subTest("string-valued define in a sequence"):
+            env = DummyEnvironment(CPPDEFINES=[("STRING=VALUE")])
+            d = SCons.Scanner.C.dictify_CPPDEFINES(env)
+            expect = {"STRING": "VALUE"}
+            self.assertEqual(d, expect)
+
+        with self.subTest("string-valued define by itself"):
+            env = DummyEnvironment(CPPDEFINES="STRING=VALUE")
+            d = SCons.Scanner.C.dictify_CPPDEFINES(env)
+            expect = {"STRING": "VALUE"}
+            self.assertEqual(d, expect)
+
+        from collections import deque
+        with self.subTest("compound CPPDEFINES in internal format"):
+            env = DummyEnvironment(
+                CPPDEFINES=deque([("STRING", "VALUE"), ("UNVALUED",)])
+            )
+            d = SCons.Scanner.C.dictify_CPPDEFINES(env)
+            expect = {"STRING": "VALUE", "UNVALUED": None}
+            self.assertEqual(d, expect)
+
 
 if __name__ == "__main__":
-    TestUnit.run(suite())
+    unittest.main()
 
 # Local Variables:
 # tab-width:4