From 4a123f102e7c4d0a0d19b9eca1a2fb552362c4bf Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 27 Jan 2020 20:52:50 -0800 Subject: [PATCH] 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 34848e6026c9c43d45efe97a8849effdc1a1c97a Author: Alex Langford 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. --- utils/build-script | 4 +- utils/build-script-impl | 49 ++++++++++--------- .../build_swift/driver_arguments.py | 23 --------- .../build_swift/test_driver_arguments.py | 24 --------- 4 files changed, 28 insertions(+), 72 deletions(-) diff --git a/utils/build-script b/utils/build-script index 436fb357bac59..87065cd34f09e 100755 --- a/utils/build-script +++ b/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. diff --git a/utils/build-script-impl b/utils/build-script-impl index 49239434d12e0..b5b3264410f36 100755 --- a/utils/build-script-impl +++ b/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 diff --git a/utils/build_swift/build_swift/driver_arguments.py b/utils/build_swift/build_swift/driver_arguments.py index 3c607ee18efbb..ed569ad8a8473 100644 --- a/utils/build_swift/build_swift/driver_arguments.py +++ b/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: diff --git a/utils/build_swift/tests/build_swift/test_driver_arguments.py b/utils/build_swift/tests/build_swift/test_driver_arguments.py index efbc469a28a3c..7811b6dd6d61e 100644 --- a/utils/build_swift/tests/build_swift/test_driver_arguments.py +++ b/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)