Set a reasonable default for SWIFT_INSTALL_COMPONENTS #21886
Conversation
|
CC @gottesmm |
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain |
|
Linux Toolchain (Ubuntu 16.04) Install command |
|
macOS Toolchain Install command |
| @@ -2361,6 +2360,13 @@ for host in "${ALL_HOSTS[@]}"; do | |||
| ) | |||
| fi | |||
|
|
|||
| if [ "${SWIFT_INSTALL_COMPONENTS}" ] ; then | |||
There was a problem hiding this comment.
I think generally we have been using [[ and -n. Can you change to use that?
There was a problem hiding this comment.
I agree w.r.t. [[, but we aren't really using -n. I did a quick and dirty search of build-script-impl and I see:
[[ $x ]]x 66[[ -n $x ]]x 4[[ $x != ""]x 4
There was a problem hiding this comment.
Changed to if [[ "${SWIFT_INSTALL_COMPONENTS}" ]] ; then.
benlangmuir
force-pushed
the
install-components-default
branch
from
921ffdb to
e9c05a3
Compare
|
@swift-ci please smoke test |
utils/build-script-impl
Outdated
| @@ -2361,6 +2360,13 @@ for host in "${ALL_HOSTS[@]}"; do | |||
| ) | |||
| fi | |||
|
|
|||
| if [[ "${SWIFT_INSTALL_COMPONENTS}" ]] ; then | |||
There was a problem hiding this comment.
Ok. I am fine with this. I prefer the -z -n slightly since it makes things more explicit. But if this works... +1!
Everything else looks fine to me!
Previously, build-script-impl would set this to "" by default, resulting in nothing being installed with --install-swift unless you explicitly set --swift-install-components as well. Now we defer to cmake by default. On the cmake side, change the default value to exclude * dev - uses a lot of disk space and usually not something you want to install into a toolchain * clang-resource-dir-symlink & clang-builtin-headers-in-clang-resource-dir - these are mutually exclusive with clang-builtin-headers * sourcekit inproc/xpc - these are currently mutually exclusive, so pick the best one for the current platform.
benlangmuir
force-pushed
the
install-components-default
branch
from
e9c05a3 to
aac6046
Compare
|
The test failures were because we didn't build the sourcekit XPC service correctly. It turns out installing the xpc and in-process variants is mutually exclusive. That's probably not what we want, but untangling that mess is not in scope for this PR, so just choose the best one for the current platform. Edit: filed https://bugs.swift.org/browse/SR-9680 to figure out what to do with the tangled libraries. |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@benlangmuir good job! |
|
Something about this change broke CMake 3.12.2.1—
—so I'm afraid I'm going to revert this for now. |
Previously, build-script-impl would set this to "" by default, resulting
in nothing being installed with --install-swift unless you explicitly
set --swift-install-components as well. Now we defer to cmake by
default.
On the cmake side, change the default value to exclude
install into a toolchain
these are mutually exclusive with clang-builtin-headers
the best one for the current platform.