Merge GH #1265 Handle conditional includes correctly in amalgamation (see also #1264)

3 files changed, 77 insertions, 20 deletions

diff --git a/configure.py b/configure.py
index a4d2eadf0..88a21a6c7 100755
--- a/configure.py
+++ b/configure.py
@@ -83,7 +83,7 @@ class Version(object):
     @staticmethod
     def get_data():
         if not Version.data:
-            root_dir = os.path.dirname(sys.argv[0])
+            root_dir = os.path.dirname(os.path.realpath(__file__))
             Version.data = parse_version_file(os.path.join(root_dir, 'version.txt'))
         return Version.data
 
@@ -2440,13 +2440,18 @@ def portable_symlink(file_path, target_dir, method):
 
 
 class AmalgamationHelper(object):
-    _any_include_matcher = re.compile(r'#include <(.*)>$')
-    _botan_include_matcher = re.compile(r'#include <botan/(.*)>')
-    _std_include_matcher = re.compile(r'^#include <([^/\.]+|stddef.h)>$')
+    # All include types may have trailing comment like e.g. '#include <vector> // IWYU pragma: export'
+    _any_include = re.compile(r'#include <(.*)>')
+    _botan_include = re.compile(r'#include <botan/(.*)>')
+
+    # Only matches at the beginning of the line. By convention, this means that the include
+    # is not wrapped by condition macros
+    _unconditional_any_include = re.compile(r'^#include <(.*)>')
+    _unconditional_std_include = re.compile(r'^#include <([^/\.]+|stddef.h)>')
 
     @staticmethod
     def is_any_include(cpp_source_line):
-        match = AmalgamationHelper._any_include_matcher.search(cpp_source_line)
+        match = AmalgamationHelper._any_include.search(cpp_source_line)
         if match:
             return match.group(1)
         else:
@@ -2454,15 +2459,23 @@ class AmalgamationHelper(object):
 
     @staticmethod
     def is_botan_include(cpp_source_line):
-        match = AmalgamationHelper._botan_include_matcher.search(cpp_source_line)
+        match = AmalgamationHelper._botan_include.search(cpp_source_line)
+        if match:
+            return match.group(1)
+        else:
+            return None
+
+    @staticmethod
+    def is_unconditional_any_include(cpp_source_line):
+        match = AmalgamationHelper._unconditional_any_include.search(cpp_source_line)
         if match:
             return match.group(1)
         else:
             return None
 
     @staticmethod
-    def is_std_include(cpp_source_line):
-        match = AmalgamationHelper._std_include_matcher.search(cpp_source_line)
+    def is_unconditional_std_include(cpp_source_line):
+        match = AmalgamationHelper._unconditional_std_include.search(cpp_source_line)
         if match:
             return match.group(1)
         else:
@@ -2518,7 +2531,7 @@ class AmalgamationHeader(object):
                 for c in self.header_contents(header):
                     yield c
             else:
-                std_header = AmalgamationHelper.is_std_include(line)
+                std_header = AmalgamationHelper.is_unconditional_std_include(line)
 
                 if std_header:
                     self.all_std_includes.add(std_header)
@@ -2669,9 +2682,9 @@ class AmalgamationGenerator(object):
                 f.write('#endif\n')
 
         # target to include header map
-        headers_written = {}
+        unconditional_headers_written = {}
         for target, _ in amalgamation_sources.items():
-            headers_written[target] = included_in_headers.copy()
+            unconditional_headers_written[target] = included_in_headers.copy()
 
         for mod in sorted(self._modules, key=lambda module: module.basename):
             tgt = self._target_for_module(mod)
@@ -2679,17 +2692,17 @@ class AmalgamationGenerator(object):
                 with open(src, 'r', **encoding_kwords) as f:
                     for line in f:
                         if AmalgamationHelper.is_botan_include(line):
+                            # Botan headers are inlined in amalgamation headers
                             continue
 
-                        header = AmalgamationHelper.is_any_include(line)
-                        if header:
-                            if header in headers_written[tgt]:
-                                continue
+                        if AmalgamationHelper.is_any_include(line) in unconditional_headers_written[tgt]:
+                            # This include (conditional or unconditional) was unconditionally added before
+                            continue
 
-                            amalgamation_files[tgt].write(line)
-                            headers_written[tgt].add(header)
-                        else:
-                            amalgamation_files[tgt].write(line)
+                        amalgamation_files[tgt].write(line)
+                        unconditional_header = AmalgamationHelper.is_unconditional_any_include(line)
+                        if unconditional_header:
+                            unconditional_headers_written[tgt].add(unconditional_header)
 
         for f in amalgamation_files.values():
             f.close()
diff --git a/src/lib/utils/semaphore.h b/src/lib/utils/semaphore.h
index 84478a8df..431680cb3 100644
--- a/src/lib/utils/semaphore.h
+++ b/src/lib/utils/semaphore.h
@@ -11,7 +11,7 @@
 #include <botan/mutex.h>
 
 #if defined(BOTAN_TARGET_OS_HAS_THREADS)
-#include <condition_variable>
+  #include <condition_variable>
 #endif
 
 namespace Botan {
diff --git a/src/scripts/python_unittests.py b/src/scripts/python_unittests.py
index 37068ed1c..337f5369e 100755
--- a/src/scripts/python_unittests.py
+++ b/src/scripts/python_unittests.py
@@ -14,9 +14,53 @@ import sys
 import unittest
 
 sys.path.append("../..") # Botan repo root
+from configure import AmalgamationHelper # pylint: disable=wrong-import-position
 from configure import CompilerDetector # pylint: disable=wrong-import-position
 from configure import ModulesChooser # pylint: disable=wrong-import-position
 
+class AmalgamationHelperTests(unittest.TestCase):
+    def test_matcher_std_includes(self):
+        self.assertEqual(AmalgamationHelper.is_unconditional_std_include("#include <string>"), "string")
+        self.assertEqual(AmalgamationHelper.is_unconditional_std_include("#include <string> // comment"), "string")
+
+        self.assertEqual(AmalgamationHelper.is_unconditional_std_include("#include <myfile.h>"), None)
+        self.assertEqual(AmalgamationHelper.is_unconditional_std_include("#include <unistd.h>"), None)
+        self.assertEqual(AmalgamationHelper.is_unconditional_std_include("  #include <string>"), None)
+
+    def test_matcher_botan_include(self):
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <botan/oids.h>"),
+                         "oids.h")
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <botan/internal/socket.h>"),
+                         "internal/socket.h")
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <botan/oids.h> // comment"),
+                         "oids.h")
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <botan/internal/socket.h> // comment"),
+                         "internal/socket.h")
+        self.assertEqual(AmalgamationHelper.is_botan_include("  #include <botan/oids.h>"),
+                         "oids.h")
+        self.assertEqual(AmalgamationHelper.is_botan_include("  #include <botan/internal/socket.h>"),
+                         "internal/socket.h")
+
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <string>"), None)
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <myfile.h>"), None)
+        self.assertEqual(AmalgamationHelper.is_botan_include("#include <unistd.h>"), None)
+
+    def test_matcher_any_includes(self):
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <string>"), "string")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <myfile.h>"), "myfile.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <unistd.h>"), "unistd.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <botan/oids.h>"),
+                         "botan/oids.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("  #include <string>"), "string")
+        self.assertEqual(AmalgamationHelper.is_any_include("  #include <myfile.h>"), "myfile.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("  #include <unistd.h>"), "unistd.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("  #include <botan/oids.h>"),
+                         "botan/oids.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <string> // comment"), "string")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <myfile.h> // comment"), "myfile.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <unistd.h> // comment"), "unistd.h")
+        self.assertEqual(AmalgamationHelper.is_any_include("#include <botan/oids.h> // comment"),
+                         "botan/oids.h")
 
 class CompilerDetection(unittest.TestCase):