Fix the build-script --skip-build option.

This option configures the build directories without building any
targets. Splitting configuration from build allows for the decoupling
of build products. This decoupling is essential for the enlightened
way of developing Swift where the build-script is never actually used
to build anything, and build products can be independently
configured. When fully supported, this avoids many unnecessary
full/clean rebuilds and enables debugging by mixing-and-matching
configurations and rebuilding only select products after a change.

Sadly, the option has degraded, and a recent commit rendered it fully broken:

  commit 34848e6
  Author: Alex Langford <[email protected]>
  Date:   Wed Jan 22 19:27:44 2020

    [build] Unify logic to skip building projects in build-script-impl

The breaking commit was itself a reasonable cleanup. The underlying
problem was the original --skip-build was implemented using hacks that
conflated configuration with build.

This fix reinstates a reasonable situation:

--skip-build has no effect on configuration, as documented. It merely
  skips building the targets. This is how it must behave to work as
  intended.

--skip-build-{product} and its inverse --build-{product} controls
  which products will be configured. These options are in heavy use
  throughout the scripts, so changing the name (e.g. to --skip-config)
  would be disruptive and of questionable benefit.

None of this changes the fact that any required build logic that
people have dumped into build-script-impl still effectively breaks the
enlightened way of building Swift, particularly when building
toolchain components.

Author: atrick

utils/build-script

@@ -464,9 +464,7 @@ class BuildScriptInvocation(object):
             ]
 
         if args.skip_build:
-            impl_args += ["--skip-build-cmark",
-                          "--skip-build-llvm",
-                          "--skip-build-swift"]
+            impl_args += ["--skip-build"]
         if not args.build_benchmarks:
             impl_args += ["--skip-build-benchmarks"]
         # Currently we do not build external benchmarks by default.

utils/build-script-impl

@@ -30,12 +30,13 @@ umask 0022
 # A default value of "" indicates that the corresponding variable
 # will remain unset unless set explicitly.
 #
-# skip-* parameters do not affect the configuration (CMake parameters).
-# You can turn them on and off in different invocations of the script for the
-# same build directory.
+# The --skip-build parameter, with no product name, does not affect the
+# configuration (CMake parameters). You can turn this option on and
+# off in different invocations of the script for the same build
+# directory without affecting configutation.
 #
-# build-* parameters affect the CMake configuration (enable/disable those
-# components).
+# skip-build-* and build-* parameters affect the CMake configuration
+# (enable/disable those components).
 #
 # Each variable name is re-exported into this script in uppercase, where dashes
 # are substituted by underscores. For example, `swift-install-components` is
@@ -118,6 +119,7 @@ KNOWN_SETTINGS=(
     swift-stdlib-build-type                       "Debug"           "the CMake build variant for Swift"
 
     ## Skip Build ...
+    skip-build                                    ""                "set to configure as usual while skipping the build step"
     skip-build-android                            ""                "set to skip building Swift stdlibs for Android"
     skip-build-benchmarks                         ""                "set to skip building Swift Benchmark Suite"
     skip-build-clang-tools-extra                  ""                "set to skip building clang-tools-extra as part of llvm"
@@ -1041,13 +1043,10 @@ if [[ ! "${SKIP_BUILD_PLAYGROUNDSUPPORT}" && ! -d ${PLAYGROUNDSUPPORT_SOURCE_DIR
     exit 1
 fi
 
-# We cannot currently apply the normal rules of skipping here for LLVM. Even if
-# we are skipping building LLVM, we still need to at least build several tools
-# that swift relies on for building and testing. See the LLVM configure rules.
-PRODUCTS=(llvm)
 [[ "${SKIP_BUILD_CMARK}" ]] || PRODUCTS+=(cmark)
 [[ "${SKIP_BUILD_LIBCXX}" ]] || PRODUCTS+=(libcxx)
 [[ "${SKIP_BUILD_LIBICU}" ]] || PRODUCTS+=(libicu)
+[[ "${SKIP_BUILD_LLVM}" ]] || PRODUCTS+=(llvm)
 [[ "${SKIP_BUILD_SWIFT}" ]] || PRODUCTS+=(swift)
 [[ "${SKIP_BUILD_LLDB}" ]] || PRODUCTS+=(lldb)
 [[ "${SKIP_BUILD_LIBDISPATCH}" ]] || PRODUCTS+=(libdispatch)
@@ -1406,7 +1405,6 @@ for host in "${ALL_HOSTS[@]}"; do
     for product in "${PRODUCTS[@]}"; do
         [[ $(should_execute_action "${host}-${product/_static}-build") ]] || continue
 
-        unset skip_build
         source_dir_var="$(toupper ${product})_SOURCE_DIR"
         source_dir=${!source_dir_var}
         build_dir=$(build_directory ${host} ${product})
@@ -1436,7 +1434,7 @@ for host in "${ALL_HOSTS[@]}"; do
                 if [ "${BUILD_LLVM}" == "0" ] ; then
                     build_targets=(clean)
                 fi
-                if [ "${SKIP_BUILD_LLVM}" ] ; then
+                if [ "${SKIP_BUILD}" ] ; then
                     # We can't skip the build completely because the standalone
                     # build of Swift depend on these for building and testing.
                     build_targets=(llvm-tblgen clang-resource-headers intrinsics_gen clang-tablegen-targets)
@@ -2207,18 +2205,25 @@ for host in "${ALL_HOSTS[@]}"; do
         fi
 
         # Build.
-        if [[ "${CMAKE_GENERATOR}" == "Xcode" ]] ; then
-            # Xcode generator uses "ALL_BUILD" instead of "all".
-            # Also, xcodebuild uses -target instead of bare names.
-            build_targets=("${build_targets[@]/all/ALL_BUILD}")
-            build_targets=("${build_targets[@]/#/${BUILD_TARGET_FLAG} }")
-
-            # Xcode can't restart itself if it turns out we need to reconfigure.
-            # Do an advance build to handle that.
-            call "${CMAKE_BUILD[@]}" "${build_dir}" $(cmake_config_opt ${product})
-        fi
+        #
+        # Even if builds are skipped, Swift configuration relies on
+        # some LLVM tools like TableGen. In the LLVM configure rules
+        # above, a small subset of LLVM build_targets are selected
+        # when SKIP_BUILD is set.
+        if [[ $(not ${SKIP_BUILD}) || "${product}" == "llvm" ]]; then
+            if [[ "${CMAKE_GENERATOR}" == "Xcode" ]] ; then
+                # Xcode generator uses "ALL_BUILD" instead of "all".
+                # Also, xcodebuild uses -target instead of bare names.
+                build_targets=("${build_targets[@]/all/ALL_BUILD}")
+                build_targets=("${build_targets[@]/#/${BUILD_TARGET_FLAG} }")
+
+                # Xcode can't restart itself if it turns out we need to reconfigure.
+                # Do an advance build to handle that.
+                call "${CMAKE_BUILD[@]}" "${build_dir}" $(cmake_config_opt ${product})
+            fi
 
-        call "${CMAKE_BUILD[@]}" "${build_dir}" $(cmake_config_opt ${product}) -- "${BUILD_ARGS[@]}" ${build_targets[@]}
+            call "${CMAKE_BUILD[@]}" "${build_dir}" $(cmake_config_opt ${product}) -- "${BUILD_ARGS[@]}" ${build_targets[@]}
+        fi
 
         # When we are building LLVM copy over the compiler-rt
         # builtins for iOS/tvOS/watchOS to ensure that Swift's

utils/build_swift/build_swift/driver_arguments.py

@@ -131,29 +131,6 @@ def _apply_default_arguments(args):
         raise ValueError('error: --watchos-all is unavailable in open-source '
                          'Swift.\nUse --watchos to skip watchOS device tests.')
 
-    # Propagate global --skip-build
-    if args.skip_build:
-        args.build_linux = False
-        args.build_freebsd = False
-        args.build_cygwin = False
-        args.build_osx = False
-        args.build_ios = False
-        args.build_tvos = False
-        args.build_watchos = False
-        args.build_android = False
-        args.build_benchmarks = False
-        args.build_external_benchmarks = False
-        args.build_lldb = False
-        args.build_llbuild = False
-        args.build_libcxx = False
-        args.build_swiftpm = False
-        args.build_xctest = False
-        args.build_foundation = False
-        args.build_libdispatch = False
-        args.build_libicu = False
-        args.build_playgroundsupport = False
-        args.build_pythonkit = False
-
     # --skip-{ios,tvos,watchos} or --skip-build-{ios,tvos,watchos} are
     # merely shorthands for --skip-build-{**os}-{device,simulator}
     if not args.ios or not args.build_ios:

utils/build_swift/tests/build_swift/test_driver_arguments.py

@@ -540,30 +540,6 @@ def test_implied_defaults_build_variant(self):
         self.assertEqual(namespace.swift_build_variant, 'Debug')
         self.assertEqual(namespace.swift_stdlib_build_variant, 'Debug')
 
-    def test_implied_defaults_skip_build(self):
-        namespace = self.parse_default_args(['--skip-build'])
-
-        self.assertFalse(namespace.build_benchmarks)
-
-        self.assertFalse(namespace.build_linux)
-        self.assertFalse(namespace.build_android)
-        self.assertFalse(namespace.build_freebsd)
-        self.assertFalse(namespace.build_cygwin)
-        self.assertFalse(namespace.build_osx)
-        self.assertFalse(namespace.build_ios)
-        self.assertFalse(namespace.build_tvos)
-        self.assertFalse(namespace.build_watchos)
-
-        self.assertFalse(namespace.build_foundation)
-        self.assertFalse(namespace.build_libdispatch)
-        self.assertFalse(namespace.build_libicu)
-        self.assertFalse(namespace.build_lldb)
-        self.assertFalse(namespace.build_llbuild)
-        self.assertFalse(namespace.build_libcxx)
-        self.assertFalse(namespace.build_playgroundsupport)
-        self.assertFalse(namespace.build_swiftpm)
-        self.assertFalse(namespace.build_xctest)
-
     def test_implied_defaults_skip_build_ios(self):
         namespace = self.parse_default_args(['--skip-build-ios'])
         self.assertFalse(namespace.build_ios_device)