[staging-next] swift: bootstrap using system stdlib #211908
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofborg
bot
added
11.by: package-maintainer
stephank
changed the title
|
A staging-next PR was opened (#211923) so I'm retargeting that. |
|
|
||
| installPhase = '' | ||
| mkdir -p $out/lib/swift | ||
| cp -r \ |
There was a problem hiding this comment.
symlinking does not work?
There was a problem hiding this comment.
This is modelled after libs in apple_sdk.nix. I think the idea is that we try to avoid users having to download the full SDK.
Comment on lines
413
to
423
| ${lib.optionalString stdenv.isDarwin '' | ||
| # Add appleSwiftCore to the search paths. We can't simply add it to | ||
| # buildInputs, because it is potentially an older stdlib than the one we're | ||
| # building. We have to remove it again after the main Swift build, or later | ||
| # build steps may fail. (Specific case: Concurrency backdeploy uses the | ||
| # Sendable protocol, which appears to not be present in the macOS 11 SDK.) | ||
| OLD_NIX_SWIFTFLAGS_COMPILE="$NIX_SWIFTFLAGS_COMPILE" | ||
| OLD_NIX_LDFLAGS="$NIX_LDFLAGS" | ||
| export NIX_SWIFTFLAGS_COMPILE+=" -I ${appleSwiftCore}/lib/swift" | ||
| export NIX_LDFLAGS+=" -L ${appleSwiftCore}/lib/swift" | ||
| ''} |
There was a problem hiding this comment.
Suggested change
| ${lib.optionalString stdenv.isDarwin '' | |
| # Add appleSwiftCore to the search paths. We can't simply add it to | |
| # buildInputs, because it is potentially an older stdlib than the one we're | |
| # building. We have to remove it again after the main Swift build, or later | |
| # build steps may fail. (Specific case: Concurrency backdeploy uses the | |
| # Sendable protocol, which appears to not be present in the macOS 11 SDK.) | |
| OLD_NIX_SWIFTFLAGS_COMPILE="$NIX_SWIFTFLAGS_COMPILE" | |
| OLD_NIX_LDFLAGS="$NIX_LDFLAGS" | |
| export NIX_SWIFTFLAGS_COMPILE+=" -I ${appleSwiftCore}/lib/swift" | |
| export NIX_LDFLAGS+=" -L ${appleSwiftCore}/lib/swift" | |
| ''} | |
| '' + lib.optionalString stdenv.isDarwin '' | |
| # Add appleSwiftCore to the search paths. We can't simply add it to | |
| # buildInputs, because it is potentially an older stdlib than the one we're | |
| # building. We have to remove it again after the main Swift build, or later | |
| # build steps may fail. (Specific case: Concurrency backdeploy uses the | |
| # Sendable protocol, which appears to not be present in the macOS 11 SDK.) | |
| OLD_NIX_SWIFTFLAGS_COMPILE="$NIX_SWIFTFLAGS_COMPILE" | |
| OLD_NIX_LDFLAGS="$NIX_LDFLAGS" | |
| export NIX_SWIFTFLAGS_COMPILE+=" -I ${appleSwiftCore}/lib/swift" | |
| export NIX_LDFLAGS+=" -L ${appleSwiftCore}/lib/swift" | |
| '' + '' |
| # - Experimental features are OFF by default in CMake, but some are | ||
| # required to build the stdlib. | ||
| # - SWIFT_STDLIB_ENABLE_OBJC_INTEROP is set explicitely because its check | ||
| # is buggy. (Uses SWIFT_HOST_VARIANT_SDK before initialized.) | ||
| # Fixed in: https://github.com/apple/swift/commit/84083afef1de5931904d5c815d53856cdb3fb232 | ||
| cmakeFlags=" | ||
| -GNinja | ||
| -DBOOTSTRAPPING_MODE=BOOTSTRAPPING | ||
| -DBOOTSTRAPPING_MODE=${if stdenv.isDarwin then "BOOTSTRAPPING-WITH-HOSTLIBS" else "BOOTSTRAPPING"} |
There was a problem hiding this comment.
Suggested change
| -DBOOTSTRAPPING_MODE=${if stdenv.isDarwin then "BOOTSTRAPPING-WITH-HOSTLIBS" else "BOOTSTRAPPING"} | |
| -DBOOTSTRAPPING_MODE=BOOTSTRAPPING${lib.optionalStrin stdenv.isDarwin "-WITH-HOSTLIBS"} |
Comment on lines
450
to
455
| ${lib.optionalString stdenv.isDarwin '' | ||
| # Restore search paths to remove appleSwiftCore. | ||
| export NIX_SWIFTFLAGS_COMPILE="$OLD_NIX_SWIFTFLAGS_COMPILE" | ||
| export NIX_LDFLAGS="$OLD_NIX_LDFLAGS" | ||
| ''} | ||
|
|
There was a problem hiding this comment.
Suggested change
| ${lib.optionalString stdenv.isDarwin '' | |
| # Restore search paths to remove appleSwiftCore. | |
| export NIX_SWIFTFLAGS_COMPILE="$OLD_NIX_SWIFTFLAGS_COMPILE" | |
| export NIX_LDFLAGS="$OLD_NIX_LDFLAGS" | |
| ''} | |
| '' + lib.optionalString stdenv.isDarwin '' | |
| # Restore search paths to remove appleSwiftCore. | |
| export NIX_SWIFTFLAGS_COMPILE="$OLD_NIX_SWIFTFLAGS_COMPILE" | |
| export NIX_LDFLAGS="$OLD_NIX_LDFLAGS" | |
| '' + '' |
This reverts commit 511f21d. In apple_sdk_11_0, the xpc package contains only headers that are already part of libsystem, so this change did nothing. For Swift and `-fmodules`, this actually caused an error, because there was now a duplicate module in the search path.
`BOOTSTRAPPING-WITH-HOSTLIBS` is also what official builds use on Darwin. It's unclear why `BOOTSTRAPPING` was working before, but it is probably not supported on Darwin to begin with. It was now causing crashes because of mixing of two copies of stdlib at run-time.
stephank
force-pushed
the
fix/swift-darwin
branch
from
927b2ce to
7220d26
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
After the merge of #189977, there were new failures on staging.
db57f8c is a straight revert of #207123. As discussed there, we determined the change did not have any effect, and in fact broke the Swift build, because there are now 2 copies of xpc in the search path.
927b2ce is a little meatier.
There are multiple bootstrapping modes for the Swift compiler. The official upstream Linux builds use
-DBOOTSTRAPPING_MODE=BOOTSTRAPPING, which bootstraps compiler + stdlib, while the official Darwin builds use-DBOOTSTRAPPING_MODE=BOOTSTRAPPING-WITH-HOSTLIBS, which bootstraps compiler only. I hadBOOTSTRAPPINGworking on Darwin before, but it myseriously broke.The failure with
BOOTSTRAPPINGwas that both the bootstraplibswiftCore.dyliband the system/usr/lib/swift/libswiftCore.dylibwere being loaded at run-time, causing lots of warnings about duplicate ObjC classes, and eventually a segfault in the compiler itself. The system copy is not something we can easily eliminate; it's part of the stable macOS ABI, much like a system framework.I've not been able to determine an exact cause why this started failing now, but regardless, we already had to patch the Swift build to make
BOOTSTRAPPINGwork. It seems like it may not be supported on Darwin, so this change makes our build match upstream by usingBOOTSTRAPPING-WITH-HOSTLIBS, which is probably for the better.Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes