Skip to content

[build-script] Move --native-{clang,llvm,swift}-tools-path flags into the Python build-script#34437

Merged
gottesmm merged 1 commit intoswiftlang:mainfrom
finagolfin:native-flags
Nov 9, 2020
Merged

[build-script] Move --native-{clang,llvm,swift}-tools-path flags into the Python build-script#34437
gottesmm merged 1 commit intoswiftlang:mainfrom
finagolfin:native-flags

Conversation

@finagolfin
Copy link
Copy Markdown
Member

@finagolfin finagolfin commented Oct 26, 2020

Also, fix two places where the LLVM path was wrongly employed to set up clang, and use the Swift path in install_toolchain_path().

This is a continuation of #32922, which enabled building all the corelibs with a prebuilt toolchain, to non-build-script-impl products like the package manager. I applied this small patch to completely skip building any LLVM or corelibs targets and I was able to build and test SPM from the Oct. 22 source snapshot with the official prebuilt snapshot toolchain for Ubuntu:

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.

Then I ran this build-script invocation:

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
--native-swift-tools-path swift-DEVELOPMENT-SNAPSHOT-2020-10-22-a-ubuntu20.04/usr/bin -p -T --install-swiftpm

This makes it very easy to build and test several Swift repos with a prebuilt Swift toolchain, rather than having to spend hours building the compiler for any given snapshot.

finagolfin
finagolfin commented Oct 26, 2020
View reviewed changes
stdlib/CMakeLists.txt Outdated
Copy link
Copy Markdown
Member Author

@finagolfin finagolfin Oct 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously this means those currently passing in these flags and giving a different LLVM path than the clang path may now have to change their flag, but since these flags were never documented in the build-script help till now, that's probably just @compnerd and me.

@finagolfin
[build-script] Move --native-{clang,llvm,swift}-tools-path flags into…
9ed6766 
… the Python build-script

Also, fix two places where the LLVM path was wrongly employed to set up clang,
and use the Swift path in install_toolchain_path().
@compnerd
Copy link
Copy Markdown
Member

compnerd commented Oct 26, 2020

I want to play with this change a bit, but, from a quick look, I really like this idea. A +1 on the idea at the very least. If @gottesmm beats me to the review, that's fine, don't hold this up on my behalf, Id rather get this cleanup merged.

@gottesmm
Copy link
Copy Markdown
Contributor

gottesmm commented Nov 6, 2020

I am fine with this in principle. I just want to check that it does not break the minimal config.

@gottesmm
Copy link
Copy Markdown
Contributor

gottesmm commented Nov 6, 2020

@swift-ci please test stdlib with toolchain

@gottesmm
Copy link
Copy Markdown
Contributor

gottesmm commented Nov 6, 2020

preset=stdlib_S_standalone_minimal_macho_x86_64,build,test
@swift-ci please test with toolchain and preset

@swift-ci
Copy link
Copy Markdown
Contributor

swift-ci commented Nov 6, 2020

Build failed
Swift Test OS X Platform
Git Sha - 9ed6766

@finagolfin
Copy link
Copy Markdown
Member Author

finagolfin commented Nov 6, 2020

macOS stdlib CI failure is unrelated to this patch:

/Users/buildnode/jenkins/workspace/swift-PR-stdlib-with-toolchain-osx/branch-main/swift/stdlib/public/Concurrency/Task.swift:136:7: error: module 'Builtin' has no member named 'cancelAsyncTask'

@gottesmm
Copy link
Copy Markdown
Contributor

gottesmm commented Nov 6, 2020

@swift-ci test

@finagolfin
Copy link
Copy Markdown
Member Author

finagolfin commented Nov 7, 2020

Windows CI failure is unrelated.

@gottesmm
Copy link
Copy Markdown
Contributor

gottesmm commented Nov 7, 2020

@shahmishal do you have any concerns about this?

@finagolfin finagolfin mentioned this pull request Nov 8, 2020
Merged
@shahmishal
Copy link
Copy Markdown
Member

shahmishal commented Nov 9, 2020

@swift-ci test Windows

shahmishal
shahmishal approved these changes Nov 9, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@shahmishal shahmishal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gottesmm
Copy link
Copy Markdown
Contributor

gottesmm commented Nov 9, 2020

Ok. This LGTM. Lets merge this!

@gottesmm gottesmm merged commit 8474332 into swiftlang:main Nov 9, 2020
@finagolfin finagolfin deleted the native-flags branch November 10, 2020 01:18
@shahmishal
Copy link
Copy Markdown
Member

shahmishal commented Nov 10, 2020

We are seeing toolchain bot failing, it might be related to this PR.

https://ci.swift.org/job/oss-swift-package-osx/5267/

23:50:48 /bin/sh: /Users/buildnode/jenkins/workspace/oss-swift-package-osx/build/buildbot_osx/llvm-macosx-arm64/./bin/clang++: Bad CPU type in executable

@shahmishal shahmishal mentioned this pull request Nov 10, 2020
Merged
@edymtt edymtt mentioned this pull request Nov 10, 2020
Closed
@shahmishal
Copy link
Copy Markdown
Member

shahmishal commented Nov 11, 2020

@buttaface When you are ready to re-commit this change please run @swift-ci Please build toolchain. Thanks!

@finagolfin finagolfin mentioned this pull request Nov 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahmishal shahmishal shahmishal approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@finagolfin @compnerd @gottesmm @swift-ci @shahmishal