use the toolchain's clang when building libdispatch #2951
Conversation
Use the clang that is part of the toolchain being built when building libdispatch to ensure getting the appropriate version of clang. Also pass through SWIFTC_BIN to the configure command to prepare for a cleanup of the libdispatch side of the build process.
| call "${LIBDISPATCH_SOURCE_DIR}"/configure --prefix="$(get_host_install_destdir ${host})$(get_host_install_prefix ${host})" --with-swift-toolchain="${SWIFT_BUILD_PATH}" | ||
| call env CC="${LLVM_BIN}/clang" CCX="${LLVM_BIN}/clang" SWIFTC="${SWIFTC_BIN}" \ | ||
| "${LIBDISPATCH_SOURCE_DIR}"/configure --with-swift-toolchain="${SWIFT_BUILD_PATH}" \ | ||
| --prefix="$(get_host_install_destdir ${host})$(get_host_install_prefix ${host})" |
There was a problem hiding this comment.
is libdispatch looking for executables in SWIFT_BUILD_PATH (i.e. --with-swift-toolchain) or just the standard library? If executables, it should switch to using $(build_directory ${LOCAL_HOST} swift). I didn't know how this was used when factoring in cross-compiling support, but it seems like something we could fix here if it's not already correct. We might not be able to execute things from SWIFT_BUILD_PATH, but we should set the swift compiler's -resource-dir to $(build_directory ${host} swift)/lib/swift at any rate.
|
It's looking for the swiftc executable. I'm planning on reworking the libdispatch build so that it will find swiftc from the SWIFTC environment variable instead (like it does CC and CXX), but it will take a couple of steps since first I need to define SWIFTC here, then I need to change the libdispatch configure script to look for the SWIFTC environment variable, then I can get rid of the toolchain argument in build-script-impl |
|
ok, thanks. We'd still need to set the compiler's resource-dir, sdk (sysroot) and toolchain/linker. We don't have anything in the build-script for those last two yet, but resource-dir should be the This is actually something we really need to improve for the target libraries (Foundation, XCTest and libdispatch). |
|
I would expect the resource-dir and linker to be inferred correctly for non-cross-compilation, and I think it's okay to get cross-compilation in as a follow-up change. |
Sorry, but what is the motivation behind this? The toolchain that we are building does not include clang. It is true that the binary is built by default, but that's only because it would be more work for the build scripts to disable building it. We are not using, testing or shipping the clang binary that we are compiling as a part of the build process. |
|
The motivation is to get control over the version of clang that is used to compile libdispatch's C sources to avoid having to ifdef around usage of compiler intrinsics and builtins that are available in clang-3.9 but not in earlier versions. Ubuntu 15.10 has clang 3.6 by default; 16.04 has clang 3.8. Note that a few lines above this change, the same thing is already being done when building Foundation. I understand the point about the built clang not being tested, but if that is a blocking problem for this change, the usage by foundation's build is also an issue. |
I think it is. |
|
We already have to use the just-built Clang to build our runtime, right? How is this any different? |
|
This change greatly expands the extent to which we will be using the compiler which we are not qualifying. |
|
That seems silly to me. We rely on most of Clang and all of LLVM to compile Swift things. If Clang is busted in pretty much any way, the Swift side will have the same problem. |
|
We need this change to be merged in order to build libdispatch. |
|
Although this shouldn't affect current projects, I'm kicking off a test run. |
|
@swift-ci please test |
|
I took a quick look at the Linux failure and I can't see how a change to the compiler that is used to build dispatch (which is not used for anything) could cause a swiftpm test to fail. Transient / latent failure? |
|
Yah that's a different bug. I'm merging this. |
What's in this pull request?
As discussed offline with @MadCoder, we now want to be sure that we are using the clang from the toolchain that is being built to compile libdispatch. This pull request changes the invocation of libdispatch's configure script from build-script-impl to pass down the location of clang in the CC and CXX environment variables.
Resolved bug number: (SR-)
None.
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.
Use the clang that is part of the toolchain being built
when building libdispatch to ensure getting the appropriate
version of clang.
Also pass through SWIFTC_BIN to the configure command to
prepare for a cleanup of the libdispatch side of the build process.