[bootstrap] Add a --skip-cmake-bootstrap flag to use a prebuilt swift-build #3000

finagolfin · 2020-10-24T08:49:05Z

Rather than building with CMake and then using that freshly built swift-build to build it with itself, add this flag to check for a prebuilt swift-build next to swiftc and use that instead if it's there. The PackageDescription libraries are still built using CMake, as this repo's Package.swift only builds the latest 4_2 version.

This is part of my patches to cross-compile the corelibs and SPM for Android using the official prebuilt Swift toolchain for Ubuntu, as it doesn't make sense to rebuild a native SPM before cross-compiling it, since the official toolchain already has a native swift-build.

I tested this pull with the Oct. 22 source snapshot and official prebuilt snapshot toolchain after applying this small patch to build-script in the main Swift repo:

diff --git a/utils/build-script b/utils/build-script
index 639f790fe3..2d208781e1 100755
--- a/utils/build-script
+++ b/utils/build-script
@@ -321,7 +321,7 @@ def apply_default_arguments(toolchain, args):
     # SwiftPM and XCTest have a dependency on Foundation.
     # On OS X, Foundation is built automatically using xcodebuild.
     # On Linux, we must ensure that it is built manually.
-    if ((args.build_swiftpm or args.build_xctest) and
+    if ((args.build_swift or args.build_xctest) and
             platform.system() != "Darwin"):
         args.build_foundation = True
 
diff --git a/utils/build-script-impl b/utils/build-script-impl
index 185f6bf555..d6a4e01fd0 100755
--- a/utils/build-script-impl
+++ b/utils/build-script-impl
@@ -2456,7 +2456,7 @@ for host in "${ALL_HOSTS[@]}"; do
         # 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 [[ $(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.
diff --git a/utils/swift_build_support/swift_build_support/products/swiftpm.py b/utils/swift_build_support/swift_build_support/products/swiftpm.py
index 25e982e23f..18b626f3be 100644
--- a/utils/swift_build_support/swift_build_support/products/swiftpm.py
+++ b/utils/swift_build_support/swift_build_support/products/swiftpm.py
@@ -60,7 +60,7 @@ class SwiftPM(product.Product):
             helper_cmd.append("--release")
 
         helper_cmd += [
-            "--swiftc-path", swiftc,
+            #"--swiftc-path", swiftc,
             "--clang-path", self.toolchain.cc,
             "--cmake-path", self.toolchain.cmake,
             "--ninja-path", self.toolchain.ninja,

The first two were not strictly necessary, but allowed me to skip building LLVM and corelibs targets that aren't needed. I then added the prebuilt Swift to my path and built, tested, and installed SPM without bootstrapping without a problem, similar to how I did the same with the corelibs in swiftlang/swift#32922:

export PATH=/home/butta/swift/swift-DEVELOPMENT-SNAPSHOT-2020-10-22-a-ubuntu20.04/usr/bin:$PATH

./swift/utils/build-script -R --no-assertions --skip-build-cmark --skip-build-llvm --skip-build-swift -p -T --install-swiftpm

I~~t might make sense to~~ added a flag to ~~dis~~enable this, ~~and force~~defaulting to a bootstrap build even if a prebuilt swift-build is found, say if you made some changes to the SPM source that require bootstrapping with a modified SPM. I will submit a pull next so that a prebuilt Swift toolchain can be passed to the build-script and the single needed build-script patch above won't be.

Since some of this pull just avoids dependencies in the CMake build that are no longer needed when only building the PackageDescription libraries with CMake, it might be better to get the other PD library built with Package.swift and the prebuilt SPM instead, but I don't know if that's possible or why it isn't already done.

abertelrud · 2020-10-26T17:07:41Z

@swift-ci please smoke test

abertelrud · 2020-10-26T17:12:48Z

This seems reasonable but this should definitely be controlled by an option (and bootstrapping from source should likely be the default, with an option to skip it an use a prebuilt swift-build instead).

@aciidb0mb3r Do you have any concerns with this or know some of the background? One thing that concerns me is that the SwiftPM in the toolchain might be too old to build the current SwiftPM's main-branch sources, and that would result in some fairly subtle errors.

abertelrud · 2020-10-26T17:18:08Z

@compnerd How would this interact with the Windows support?

The more I think about this, the more it seems to me that this would be a great option to add to the bootstrap script, but that it shouldn't be on by default (for the reason I mentioned earlier). That should still enable the Android support that's mentioned, since those build scripts can presumably pass the flag.

finagolfin · 2020-10-26T18:35:33Z

this should definitely be controlled by an option (and bootstrapping from source should likely be the default, with an option to skip it an use a prebuilt swift-build instead).

Will do.

One thing that concerns me is that the SwiftPM in the toolchain might be too old to build the current SwiftPM's main-branch sources, and that would result in some fairly subtle errors.

I only pass in a toolchain that was built from the same unmodified commit, but obviously this feature will depend on the developer passing in a reasonable toolchain.

abertelrud · 2020-10-26T20:27:43Z

Great! Yes, my concern was just with people who might not realize that have an old toolchain and then try to figure out why they get errors while bootstrapping the mainline code. So if you're fine with making it an option without changing the default, that would be great. I think that there are already some options like that in the bootstrap script, such as --llbuild-link-framework, which optionally takes a prebuilt llbuild.framework instead of bootstrapping it (if I read the code right).

compnerd · 2020-10-26T22:06:04Z

This definitely will break the Windows CI - do we really need the bootstrap option for CMake? At least, can we make it enabled by default? The bootstrap script isn't used on Windows as it had issues due to rsync usage and path handling.

Removing the REQUIRED option on the find_package for the CONFIG variants of the repositories is also a viable option I think - that drops the hard requirement and should fall back to the swift flags to find the dependencies.

CMakeLists.txt

finagolfin · 2020-10-27T16:48:32Z

Updated so this is only enabled by a flag and made the CMake variable more descriptive and better documented.

…-build Rather than building with CMake and then using that freshly built swift-build to build it with itself, add this flag to check for a prebuilt swift-build next to swiftc and use that instead if it's there. The PackageDescription libraries are still built using CMake, as this repo's Package.swift only builds the latest 4_2 version.

Utilities/bootstrap

finagolfin · 2020-11-02T14:14:14Z

Ping, ready to merge?

abertelrud

Thanks @buttaface. This looks good to me.

abertelrud · 2020-11-02T17:08:31Z

Utilities/bootstrap

@@ -198,6 +202,8 @@ def parse_build_args(args):
    args.bootstrap_dir = os.path.join(args.target_dir, "bootstrap")
    args.conf = 'release' if args.release else 'debug'
    args.bin_dir = os.path.join(args.target_dir, args.conf)
+    args.bootstrap = not args.skip_cmake_bootstrap or \
+                     not os.path.exists(os.path.join(os.path.split(args.swiftc_path)[0], "swift-build"))


As a refinement, would it make sense to emit an error if --skip-cmake-bootstrap is true but there's no swift-build, rather than going through with the bootstrap?

I figured it's better to then try the bootstrap build as a backup, but could go either way. If you prefer, I can add a warning or error out in my next pull to cross-compile SPM for Android.

abertelrud · 2020-11-02T17:08:48Z

@swift-ci please smoke test

finagolfin · 2020-11-03T03:59:27Z

Thanks, now I need to clean up my patches cross-compiling SPM for Android and submit that pull.

finagolfin requested review from abertelrud, aciidgh, friedbunny and neonichu as code owners October 24, 2020 08:49

finagolfin force-pushed the no-bootstrap branch from 0beb536 to 1578327 Compare October 24, 2020 09:24

abertelrud self-assigned this Oct 26, 2020

compnerd reviewed Oct 26, 2020

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

finagolfin force-pushed the no-bootstrap branch 2 times, most recently from be856c4 to 6dd031a Compare October 27, 2020 16:39

finagolfin changed the title ~~[bootstrap] Use a prebuilt swift-build from the Swift toolchain if one exists~~ [bootstrap] Add a --skip-cmake-bootstrap flag to use a prebuilt swift-build Oct 27, 2020

finagolfin force-pushed the no-bootstrap branch from 6dd031a to c815b71 Compare October 27, 2020 16:51

compnerd reviewed Oct 28, 2020

View reviewed changes

Utilities/bootstrap Show resolved Hide resolved

abertelrud approved these changes Nov 2, 2020

View reviewed changes

abertelrud reviewed Nov 2, 2020

View reviewed changes

neonichu approved these changes Nov 2, 2020

View reviewed changes

abertelrud merged commit 19eaf12 into swiftlang:main Nov 2, 2020

finagolfin deleted the no-bootstrap branch November 3, 2020 04:02

finagolfin mentioned this pull request Nov 14, 2023

Disable CMake bootstrap by default #7089

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[bootstrap] Add a --skip-cmake-bootstrap flag to use a prebuilt swift-build #3000

[bootstrap] Add a --skip-cmake-bootstrap flag to use a prebuilt swift-build #3000

Uh oh!

finagolfin commented Oct 24, 2020 •

edited

Loading

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

finagolfin commented Oct 26, 2020

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

compnerd commented Oct 26, 2020 •

edited

Loading

Uh oh!

Uh oh!

finagolfin commented Oct 27, 2020

Uh oh!

Uh oh!

finagolfin commented Nov 2, 2020

Uh oh!

abertelrud left a comment

Uh oh!

abertelrud Nov 2, 2020

Uh oh!

finagolfin Nov 3, 2020

Uh oh!

abertelrud commented Nov 2, 2020

Uh oh!

finagolfin commented Nov 3, 2020

Uh oh!

Uh oh!

[bootstrap] Add a --skip-cmake-bootstrap flag to use a prebuilt swift-build #3000

[bootstrap] Add a --skip-cmake-bootstrap flag to use a prebuilt swift-build #3000

Uh oh!

Conversation

finagolfin commented Oct 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

finagolfin commented Oct 26, 2020

Uh oh!

abertelrud commented Oct 26, 2020

Uh oh!

compnerd commented Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

finagolfin commented Oct 27, 2020

Uh oh!

Uh oh!

finagolfin commented Nov 2, 2020

Uh oh!

abertelrud left a comment

Choose a reason for hiding this comment

Uh oh!

abertelrud Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

finagolfin Nov 3, 2020

Choose a reason for hiding this comment

Uh oh!

abertelrud commented Nov 2, 2020

Uh oh!

finagolfin commented Nov 3, 2020

Uh oh!

Uh oh!

finagolfin commented Oct 24, 2020 •

edited

Loading

compnerd commented Oct 26, 2020 •

edited

Loading