Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

swift: fix build with the new SDK pattern #346947

Merged
merged 12 commits into from
Oct 11, 2024

Conversation

reckenrode
Copy link
Contributor

Swift depends on how the old SDK pattern was implemented (e.g., that only the requested framework dependencies would be available). This PR updates Swift to build in a way that’s compatible with the new pattern, uses the same SDK that Swift 5.8 expects to use, and propagates it to make sure packages using Swift have at least the right SDK version.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@reckenrode reckenrode force-pushed the reckenrode/darwin-sdk-refactor branch 2 times, most recently from a240201 to 4112f1a Compare October 9, 2024 02:16
Swift’s build makes a few assumptions about how the SDK is set up that
are not true anymore with the new SDK. Fix it to find the SDK at
`$SDKROOT` and copy additional stubs it needs to bootstrap.
libarclite is needed to support ARC on very old deployment targets
(10.10 and older). None of these deployment targets are supported
deployment targets in nixpkgs, especially for Swift. This removes the
need to package the command-line tools executables.
Swift releases are associated with particular Darwin SDK versions. They
don’t _have_ to use that version, but it makes sense to use that version
by default. The deployment target is set to the supported Swift minimum
versions. Unlike C and C++, Swift requires you to availability
annotations, so propagating a newer SDK should be safe.
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Not really qualified to review this, but generally LGTM. Love to see those comments about our weird build environment disappear.

Comment on lines +44 to +48
deploymentVersion =
if lib.versionOlder (targetPlatform.darwinMinVersion or "0") "10.15" then
"10.15"
else
targetPlatform.darwinMinVersion;
Copy link
Member

Choose a reason for hiding this comment

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

I think this is equivalent to deploymentVersion = "10.15";, but it’s fine (and will go away in 25.05 anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also defaults to 10.15 when building on Linux (or should).

Comment on lines +152 to +154
# Make empty to avoid adding the SDK’s modules in the bootstrap wrapper. Otherwise, the SDK conflicts with the
# shims the wrapper tries to build.
darwinMinVersion="" substituteAll '${../wrapper/wrapper.sh}' "$out"
Copy link
Member

Choose a reason for hiding this comment

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

I don’t really understand this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The wrapper uses darwinMinVersion to detect whether it is targeting Linux or Darwin. On Darwin, it will add the SDK to NIX_CFLAGS_COMPILE. Setting it to empty triggers the Linux behavior (no setting), which is what we want because the full SDK breaks the Swift bootstrap.

Copy link
Member

Choose a reason for hiding this comment

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

I see, I guess because they expect people to be using the system Swift for bootstrap on macOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upstream uses a Python script to manage the bootstrap. I don’t know the specifics, but the nixpkgs derivation is trying to recreate what it does.

pkgs/development/compilers/swift/default.nix Outdated Show resolved Hide resolved
@reckenrode reckenrode force-pushed the reckenrode/darwin-sdk-refactor branch from a1adff0 to e3f2829 Compare October 10, 2024 20:25
@emilazy emilazy marked this pull request as ready for review October 10, 2024 22:54
@emilazy emilazy deleted the branch NixOS:staging October 11, 2024 00:00
@emilazy emilazy closed this Oct 11, 2024
@reckenrode reckenrode reopened this Oct 11, 2024
@reckenrode reckenrode changed the base branch from reckenrode/darwin-sdk-refactor to staging October 11, 2024 00:04
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

I think I understand this about as well as I ever will.

@emilazy emilazy merged commit 3f6c2f2 into NixOS:staging Oct 11, 2024
27 of 28 checks passed
@reckenrode reckenrode deleted the push-pttrpnmvytwl branch October 11, 2024 13:27
azuwis added a commit to azuwis/nixpkgs that referenced this pull request Oct 23, 2024
…h64-darwin

- Switch to new appke SDK pattern, NixOS#346947
- Build from source for aarch64-darwin
  * Use llvmPackages_19 on aarch64-darwin for `ptrauth.h` support
  * Setup unwrapped clang to build the arm64e scripting addition
- Use nix-update-script for passthru.updateScript
azuwis added a commit to azuwis/nixpkgs that referenced this pull request Oct 23, 2024
…h64-darwin

- Switch to new appke SDK pattern, NixOS#346947
- Build from source for aarch64-darwin
  * Use llvmPackages_19 on aarch64-darwin for `ptrauth.h` support
  * Setup unwrapped clang to build the arm64e scripting addition
- Use nix-update-script for passthru.updateScript
- Add azuwis as maintainer
azuwis added a commit to azuwis/nixpkgs that referenced this pull request Oct 23, 2024
…h64-darwin

- Switch to new appke SDK pattern, NixOS#346947
- Build from source for aarch64-darwin
  * Use llvmPackages_19 on aarch64-darwin for `ptrauth.h` support
  * Setup unwrapped clang to build the arm64e scripting addition
- Use nix-update-script for passthru.updateScript
- Add azuwis as maintainer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants