[build-script] Improvements to CMakeOptions.

CMakeOptions was briefly used in the cmake module, but mostly unused in
the rest of the modules, however, several products were dealing with
what eventually will end as CMake options, but not using the already
existing code, and duplicating it.

The changes tries to unify all usage around CMakeOptions which avoids
code duplication. Additionally, it provides some more API surface in
CMakeOptions to make it more useful.

- The initializer can be passed a list of tuples or another CMakeOptions
  to copy into the newly created options.
- Boolean Python values are treated automatically as boolean CMake
  options and transformed into `TRUE` and `FALSE` automatically.
- Provides `extend`, which will add all the tuples from a list or all
  the options from another `CMakeOptions` into the receiving
  `CMakeOptions`.
- Provides `__contains__`, which makes checking for existing options a
  little easier (however, being `CMakeOptions` basically a list, the
  checking has to include also the value, which is not that helpful).
- Modify LLVM and Swift products to use `CMakeOptions`. It makes the
  boolean values clearer and avoid a lot of repetitive string
  formatting.
- Modify the tests to make them pass and provide new test for newly
  introduced features.

Author: drodriguez

utils/swift_build_support/swift_build_support/cmake.py

@@ -25,23 +25,33 @@ class CMakeOptions(object):
     """List like object used to define cmake options
     """
 
-    def __init__(self):
+    def __init__(self, initial_options=None):
         self._options = []
+        if initial_options is not None:
+            self.extend(initial_options)
 
     def define(self, var, value):
         """Utility to define cmake options in this object.
 
         opts.define("FOO", "BAR")       # -> -DFOO=BAR
         opts.define("FLAG:BOOL", True)  # -> -FLAG:BOOL=TRUE
         """
-        if var.endswith(':BOOL'):
+        if var.endswith(':BOOL') or isinstance(value, bool):
             value = self.true_false(value)
         if value is None:
             value = ""
         elif not isinstance(value, (str, Number)):
-            raise ValueError('define: invalid value: %s' % value)
+            raise ValueError('define: invalid value for key %s: %s (%s)' %
+                             (var, value, type(value)))
         self._options.append('-D%s=%s' % (var, value))
 
+    def extend(self, tuples_or_options):
+        if isinstance(tuples_or_options, CMakeOptions):
+            self += tuples_or_options
+        else:
+            for (variable, value) in tuples_or_options:
+                self.define(variable, value)
+
     @staticmethod
     def true_false(value):
         if hasattr(value, 'lower'):
@@ -58,6 +68,9 @@ def __len__(self):
     def __iter__(self):
         return self._options.__iter__()
 
+    def __contains__(self, item):
+        return self._options.__contains__(item)
+
     def __add__(self, other):
         ret = CMakeOptions()
         ret._options += self._options

utils/swift_build_support/swift_build_support/products/llvm.py

@@ -11,6 +11,7 @@
 # ----------------------------------------------------------------------------
 
 from . import product
+from ..cmake import CMakeOptions
 
 
 class LLVM(product.Product):
@@ -20,14 +21,12 @@ def __init__(self, args, toolchain, source_dir, build_dir):
                                  build_dir)
 
         # Add the cmake option for enabling or disabling assertions.
-        self.cmake_options.extend([
-            '-DLLVM_ENABLE_ASSERTIONS=%s' % str(args.llvm_assertions).upper()
-        ])
+        self.cmake_options.define(
+            'LLVM_ENABLE_ASSERTIONS:BOOL', args.llvm_assertions)
 
         # Add the cmake option for LLVM_TARGETS_TO_BUILD.
-        self.cmake_options.extend([
-            '-DLLVM_TARGETS_TO_BUILD=%s' % args.llvm_targets_to_build
-        ])
+        self.cmake_options.define(
+            'LLVM_TARGETS_TO_BUILD', args.llvm_targets_to_build)
 
         # Add the cmake options for vendors
         self.cmake_options.extend(self._compiler_vendor_flags)
@@ -44,17 +43,17 @@ def _compiler_vendor_flags(self):
             raise RuntimeError("Unknown compiler vendor?!")
 
         return [
-            "-DCLANG_VENDOR=Apple",
-            "-DCLANG_VENDOR_UTI=com.apple.compilers.llvm.clang",
+            ('CLANG_VENDOR', 'Apple'),
+            ('CLANG_VENDOR_UTI', 'com.apple.compilers.llvm.clang'),
             # This is safe since we always provide a default.
-            "-DPACKAGE_VERSION={}".format(self.args.clang_user_visible_version)
+            ('PACKAGE_VERSION', str(self.args.clang_user_visible_version))
         ]
 
     @property
     def _version_flags(self):
-        result = []
+        result = CMakeOptions()
         if self.args.clang_compiler_version is not None:
-            result.append("-DCLANG_REPOSITORY_STRING=clang-{}".format(
-                self.args.clang_compiler_version
-            ))
+            result.define(
+                'CLANG_REPOSITORY_STRING',
+                "clang-{}".format(self.args.clang_compiler_version))
         return result

utils/swift_build_support/swift_build_support/products/product.py

@@ -12,6 +12,8 @@
 
 import abc
 
+from .. import cmake
+
 
 class Product(object):
     @classmethod
@@ -59,7 +61,7 @@ def __init__(self, args, toolchain, source_dir, build_dir):
         self.toolchain = toolchain
         self.source_dir = source_dir
         self.build_dir = build_dir
-        self.cmake_options = []
+        self.cmake_options = cmake.CMakeOptions()
 
 
 class ProductBuilder(object):

utils/swift_build_support/swift_build_support/products/swift.py

@@ -11,6 +11,7 @@
 # ----------------------------------------------------------------------------
 
 from . import product
+from ..cmake import CMakeOptions
 
 
 class Swift(product.Product):
@@ -47,8 +48,7 @@ def _runtime_sanitizer_flags(self):
             sanitizer_list += ['Thread']
         if len(sanitizer_list) == 0:
             return []
-        return ["-DSWIFT_RUNTIME_USE_SANITIZERS=%s" %
-                ";".join(sanitizer_list)]
+        return [('SWIFT_RUNTIME_USE_SANITIZERS', ';'.join(sanitizer_list))]
 
     @property
     def _compiler_vendor_flags(self):
@@ -64,31 +64,27 @@ def _compiler_vendor_flags(self):
             swift_compiler_version = self.args.swift_compiler_version
 
         return [
-            "-DSWIFT_VENDOR=Apple",
-            "-DSWIFT_VENDOR_UTI=com.apple.compilers.llvm.swift",
+            ('SWIFT_VENDOR', 'Apple'),
+            ('SWIFT_VENDOR_UTI', 'com.apple.compilers.llvm.swift'),
 
             # This has a default of 3.0, so it should be safe to use here.
-            "-DSWIFT_VERSION={}".format(self.args.swift_user_visible_version),
+            ('SWIFT_VERSION', str(self.args.swift_user_visible_version)),
 
             # FIXME: We are matching build-script-impl here. But it seems like
             # bit rot since this flag is specified in another place with the
             # exact same value in build-script-impl.
-            "-DSWIFT_COMPILER_VERSION={}".format(swift_compiler_version),
+            ('SWIFT_COMPILER_VERSION', str(swift_compiler_version)),
         ]
 
     @property
     def _version_flags(self):
-        r = []
+        r = CMakeOptions()
         if self.args.swift_compiler_version is not None:
             swift_compiler_version = self.args.swift_compiler_version
-            r.append(
-                "-DSWIFT_COMPILER_VERSION={}".format(swift_compiler_version)
-            )
+            r.define('SWIFT_COMPILER_VERSION', str(swift_compiler_version))
         if self.args.clang_compiler_version is not None:
             clang_compiler_version = self.args.clang_compiler_version
-            r.append(
-                "-DCLANG_COMPILER_VERSION={}".format(clang_compiler_version)
-            )
+            r.define('CLANG_COMPILER_VERSION', str(clang_compiler_version))
         return r
 
     @property
@@ -99,24 +95,20 @@ def _benchmark_flags(self):
         onone_iters = self.args.benchmark_num_onone_iterations
         o_iters = self.args.benchmark_num_o_iterations
         return [
-            "-DSWIFT_BENCHMARK_NUM_ONONE_ITERATIONS={}".format(onone_iters),
-            "-DSWIFT_BENCHMARK_NUM_O_ITERATIONS={}".format(o_iters)
+            ('SWIFT_BENCHMARK_NUM_ONONE_ITERATIONS', onone_iters),
+            ('SWIFT_BENCHMARK_NUM_O_ITERATIONS', o_iters)
         ]
 
     @property
     def _compile_db_flags(self):
-        return ['-DCMAKE_EXPORT_COMPILE_COMMANDS=TRUE']
+        return [('CMAKE_EXPORT_COMPILE_COMMANDS', True)]
 
     @property
     def _force_optimized_typechecker_flags(self):
-        if not self.args.force_optimized_typechecker:
-            return ['-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER=FALSE']
-        return ['-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER=TRUE']
+        return [('SWIFT_FORCE_OPTIMIZED_TYPECHECKER:BOOL',
+                 self.args.force_optimized_typechecker)]
 
     @property
     def _stdlibcore_exclusivity_checking_flags(self):
-        # This is just to get around 80 column limitations.
-        result = '-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING={}'
-        if not self.args.enable_stdlibcore_exclusivity_checking:
-            return [result.format("FALSE")]
-        return [result.format("TRUE")]
+        return [('SWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING:BOOL',
+                 self.args.enable_stdlibcore_exclusivity_checking)]

utils/swift_build_support/tests/products/test_llvm.py

@@ -89,15 +89,16 @@ def test_llvm_enable_assertions(self):
             toolchain=self.toolchain,
             source_dir='/path/to/src',
             build_dir='/path/to/build')
-        self.assertIn('-DLLVM_ENABLE_ASSERTIONS=TRUE', llvm.cmake_options)
+        self.assertIn('-DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE', llvm.cmake_options)
 
         self.args.llvm_assertions = False
         llvm = LLVM(
             args=self.args,
             toolchain=self.toolchain,
             source_dir='/path/to/src',
             build_dir='/path/to/build')
-        self.assertIn('-DLLVM_ENABLE_ASSERTIONS=FALSE', llvm.cmake_options)
+        self.assertIn('-DLLVM_ENABLE_ASSERTIONS:BOOL=FALSE',
+                      llvm.cmake_options)
 
     def test_compiler_vendor_flags(self):
         self.args.compiler_vendor = "none"

utils/swift_build_support/tests/products/test_swift.py

@@ -86,8 +86,8 @@ def test_by_default_no_cmake_options(self):
             build_dir='/path/to/build')
         expected = [
             '-DCMAKE_EXPORT_COMPILE_COMMANDS=TRUE',
-            '-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER=FALSE',
-            '-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING=FALSE'
+            '-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER:BOOL=FALSE',
+            '-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING:BOOL=FALSE'
         ]
         self.assertEqual(set(swift.cmake_options), set(expected))
 
@@ -101,8 +101,8 @@ def test_swift_runtime_tsan(self):
         flags_set = [
             '-DSWIFT_RUNTIME_USE_SANITIZERS=Thread',
             '-DCMAKE_EXPORT_COMPILE_COMMANDS=TRUE',
-            '-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER=FALSE',
-            '-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING=FALSE'
+            '-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER:BOOL=FALSE',
+            '-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING:BOOL=FALSE'
         ]
         self.assertEqual(set(swift.cmake_options), set(flags_set))
 
@@ -286,7 +286,7 @@ def test_force_optimized_typechecker_flags(self):
             source_dir='/path/to/src',
             build_dir='/path/to/build')
         self.assertEqual(
-            ['-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER=TRUE'],
+            ['-DSWIFT_FORCE_OPTIMIZED_TYPECHECKER:BOOL=TRUE'],
             [x for x in swift.cmake_options
              if 'SWIFT_FORCE_OPTIMIZED_TYPECHECKER' in x])
 
@@ -298,6 +298,7 @@ def test_exclusivity_checking_flags(self):
             source_dir='/path/to/src',
             build_dir='/path/to/build')
         self.assertEqual(
-            ['-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING=TRUE'],
+            ['-DSWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING:BOOL='
+             'TRUE'],
             [x for x in swift.cmake_options
              if 'SWIFT_STDLIB_ENABLE_STDLIBCORE_EXCLUSIVITY_CHECKING' in x])

utils/swift_build_support/tests/test_cmake.py

@@ -496,6 +496,47 @@ def test_operations(self):
             "-DOPT1_1=VAL1",
             "-DOPT1_2=VAL2"])
 
+    def test_initial_options_with_tuples(self):
+        options = CMakeOptions([('FOO', 'foo'), ('BAR', True)])
+        self.assertIn('-DFOO=foo', options)
+        self.assertIn('-DBAR=TRUE', options)
+
+    def test_initial_options_with_other_options(self):
+        options = CMakeOptions()
+        options.define('FOO', 'foo')
+        options.define('BAR', True)
+        derived = CMakeOptions(options)
+        self.assertIn('-DFOO=foo', derived)
+        self.assertIn('-DBAR=TRUE', derived)
+
+    def test_booleans_are_translated(self):
+        options = CMakeOptions()
+        options.define('A_BOOLEAN_OPTION', True)
+        options.define('ANOTHER_BOOLEAN_OPTION', False)
+        self.assertIn('-DA_BOOLEAN_OPTION=TRUE', options)
+        self.assertIn('-DANOTHER_BOOLEAN_OPTION=FALSE', options)
+
+    def test_extend_with_other_options(self):
+        options = CMakeOptions()
+        options.define('FOO', 'foo')
+        options.define('BAR', True)
+        derived = CMakeOptions()
+        derived.extend(options)
+        self.assertIn('-DFOO=foo', derived)
+        self.assertIn('-DBAR=TRUE', derived)
+
+    def test_extend_with_tuples(self):
+        options = CMakeOptions()
+        options.extend([('FOO', 'foo'), ('BAR', True)])
+        self.assertIn('-DFOO=foo', options)
+        self.assertIn('-DBAR=TRUE', options)
+
+    def test_contains(self):
+        options = CMakeOptions()
+        self.assertTrue('-DFOO=foo' not in options)
+        options.define('FOO', 'foo')
+        self.assertTrue('-DFOO=foo' in options)
+
 
 if __name__ == '__main__':
     unittest.main()