swift: split build & Darwin support. #189977

stephank · 2022-09-06T08:32:21Z

Description of changes

This PR splits up the Swift build into smaller derivations, and brings up Darwin support for the toolchain. Fixes #37473.

I've kept the same maintainers listed as before, also for the new derivations that were already part of the toolchain. Hopefully that's okay, otherwise, let me know.

To perform the split, I've added a swiftPackages. It's not very large, but avoids cluttering the global namespace, and helps set up a scope with the required clang, macOS SDK, etc.

Tests are a pain point, and I've disabled them in many places. I believe we can fix many of the tests, but it'll take more effort.

Usage

The swift attribute now gets you just the Swift compiler (swift and swiftc). These tools are now wrapped, and a new setup-hook adds lib/swift of inputs to the Swift include path automatically. This is especially important on Darwin, where we separately package system frameworks.

The attribute swiftpm installs swift-build, swift-test, swift-run, though these are usually invoked without the hyphen by users (swift build). Additionally, a new setup-hook provides default build and check phases for convenience.

Build process

Previously, our Swift build used the Python-based 'build-script' to build Swift, which I believe matches official Swift CI. This new build bypasses the Python scripts and invokes CMake directly. I personally find this approach better for us, because:

We can split up the build. Smaller derivations = shorter iteration times and more granular caching. Especially for bringing up Darwin support, I've found quicker builds to be a great help.
I reached a point where the build-presets.ini patches were becoming hard to stack and maintain.
CMake scripts already have sane defaults for many options.
We can leverage existing Nix CMake smarts.

The basic compiler derivation now builds primarily CMark, Clang, and Swift. We also build LLDB, but only because the REPL uses it. On Darwin, there is also some extra work for back-deployment, though I've not actually tested if/how back-deployment works.

On top of this, I implemented a wrapper similar to cc-wrapper.

I've separately packaged Dispatch, Foundation and XCTest. Dispatch is based on an existing derivation, but Swift support was added.

The second complex build is for SwiftPM. We match the official build process here in first doing a CMake build, then using that to build SwiftPM itself. I wrote a small swiftpm2nix tool based on shell + jq, which actually replaces the bulk of the fetchgit source derivations from the old Swift build.

Beyond that, I also packaged the other parts of the SDK, but believe only sourcekit-lsp is currently useful.

Darwin stdenv

The biggest change to stdenv is that Swift modules are installed. These are os and Darwin for libsystem, and per-framework Swift overlays where available. I do have a small list of modules from the SDK that I couldn't place yet:

AppleArchive
Compression
DataDetection
ExtensionFoundation
ExtensionKit
QuickLookUI
ShazamKit
simd

I skipped these to limit time spent polishing details, and hope we can fix these as they become an issue.

Maybe it's possible to avoid these changes to existing SDK derivations by having separate derivations for all the Swift-specific stuff, but I haven't tried this, and wonder if it's worth it. Apple seems to treat Swift more and more as a core part of the system, and it'd make packaging a bit more tedious.

Things done

Atemu · 2022-09-06T10:13:27Z

pkgs/development/compilers/llvm/11/compiler-rt/default.nix

-  ] ++ lib.optional stdenv.hostPlatform.isAarch32 ./armv7l.patch;
-
+    ../../common/compiler-rt/darwin-plistbuddy-workaround.patch
+    ./armv7l.patch


Why is this patch now here unconditionally?

See: #186575 (review)

Atemu · 2022-09-06T10:13:46Z

pkgs/development/compilers/llvm/11/compiler-rt/default.nix

@@ -59,8 +60,9 @@ stdenv.mkDerivation {
    # extra `/`.
    ./normalize-var.patch
    ../../common/compiler-rt/libsanitizer-no-cyclades-11.patch
-  ] ++ lib.optional stdenv.hostPlatform.isAarch32 ./armv7l.patch;
-
+    ../../common/compiler-rt/darwin-plistbuddy-workaround.patch


Darwin-only?

Atemu · 2022-09-06T10:14:42Z

pkgs/development/compilers/llvm/13/compiler-rt/default.nix

-    ++ lib.optional stdenv.hostPlatform.isDarwin ./darwin-targetconditionals.patch
-    ++ lib.optional stdenv.hostPlatform.isAarch32 ./armv7l.patch;
+    # Prevent a compilation error on darwin
+    ./darwin-targetconditionals.patch


Also darwin-only.

pkgs/development/compilers/swift/compiler/default.nix

Atemu · 2022-09-06T10:29:19Z

pkgs/development/compilers/swift/sourcekit-lsp/default.nix

+  # On Darwin, we only want ncurses in the linker search path, because headers
+  # are part of libsystem. Adding its headers to the search path causes strange
+  # mixing and errors.
+  # TODO: Find a better way to prevent this conflict.
+  ncursesInput = if stdenv.isDarwin then ncurses.out else ncurses;


Is this a general problem on Darwin or only for swift packages?

I think this may be specific to the LLVM modules logic, because if it was broader, I feel like we would've encountered it elsewhere. If so, it's possible ObjC can also cause this (maybe even C? I believe the flag is -fmodules.)

Maybe we should eventually get rid of some of these headers in libsystem. It currently ships headers for ncurses, sqlite, krb5, maybe more, but we package those separately (and libsystem doesn't even provide implementations of those). A similar discussion came up in: #186251 (comment)

stephank · 2022-09-06T11:39:20Z

cc swift maintainers: @dtzWill @trepetti @dduan @Trundle
cc swift-corelibs-libdispatch maintainer: @cmm
cc for darwin stdenv: @toonn? Should we ping more?

stephank · 2022-09-08T18:41:52Z

While trying to build mpv, I noticed it invokes the compiler as swift -frontend, which was not wrapped. I modified the wrapper to also handle that specific case.

The build for mpv then fails with the same CoreVideo error as my little SwiftUI test app. Still debugging that issue.

stephank · 2022-09-10T12:17:04Z

Some more tweaks:

The CoreVideo issue appears to be an actual SDK issue. Should've checked this earlier, but the haeder file on my 12.3 SDK does include stdint.h, while the 11.1 SDK we use doesn't. I've added a patch for it in ca85764.
SwiftPM now propagates Foundation as a native input. It uses Foundation on the host platform while building package manifests. (Even though the target platform might use a different Foundation. Haven't tested this scenario, really, it might pop up when targeting iOS.)
I moved around the swift -frontend wrapper fix, because it was in a part of the wrapper with a modified PATH. This caused the swift frontend to fail translating e.g. swift build to swift-build.
I replaced an external patch for LLVM I had included as a file with a fetchpatch invocation.
Added dependencies for the SwiftUI system framework.

I can now build a SwiftUI app! Will try mpv next, but I also rebased on current staging, and need to do a large rebuild.

stephank · 2022-09-10T18:08:29Z

mpv builds, with only small changes: stephank@e4566d2. Not part of this PR, think it's better to do it separately later? (EDIT: Also not sure what functionality to actually check at run-time. Never used mpv before myself.)
SwiftPM now propagates vtool as a native input. This was the last remaining tool that was causing prompts to install Command-Line Tools when using SwiftPM in a nix shell.
I've been testing the recent changes on aarch64-darwin, and have kicked off a build on x86_64-darwin to verify.

toonn · 2022-09-12T18:39:03Z

Re testing mpv, if it is not built with Swift it should spit out a warning:

[vo/gpu] opengl cocoa backend is deprecated, use vo=libmpv instead

It will also not be able to go into fullscreen mode when you press f (default shortcut).

This is probably not a thorough test of the functionality Swift adds to mpv but may be sufficient?

Atemu · 2022-09-12T18:45:24Z

You're not actually using the new swift UI in that case. You need to add --vo=libmpv and then it works beautifully.

Building with "--disable-macos-cocoa-cb" makes it use libmpv by default but I'm not sure that's the best way of achieving that.

toonn

The Darwin stdenv changes look OK to me. Though I'm not so familiar with aarch64-darwin, maybe @thefloweringash can take a look as well?

toonn · 2022-09-12T18:52:31Z

pkgs/os-specific/darwin/apple-sdk-11.0/apple_sdk.nix

+    CoreVideo = lib.overrideDerivation super.CoreVideo (drv: {
+      installPhase = drv.installPhase + ''
+        # When used as a module, complains about a missing import for
+        # Darwin.C.stdint. Apparently fix in later SDKs.


Suggested change

# Darwin.C.stdint. Apparently fix in later SDKs.

# Darwin.C.stdint. Apparently fixed in later SDKs.

toonn · 2022-09-12T18:58:56Z

@Atemu, this is a way to test mpv is in fact using the Swift built libmpv. I'm saying if it was not then you'd get that error and fullscreen wouldn't work. This was just advice in response to stephank's edit:

(EDIT: Also not sure what functionality to actually check at run-time. Never used mpv before myself.)

stephank · 2022-09-13T14:16:22Z

Thanks for taking a look. Mpv fullscreen does work in the Swift enabled build (and not with my regular nix-darwin install, as expected), so that's a good sign.

For the REPL, it appears the logic is in swift-driver. The swift and swiftc executables forward calls to swift-driver if it exists. Current idea is to add swift-driver to the installation via the wrapper, and this works, but the new driver is not forwarding linker flags correctly. Once that's fixed, I also need to add the LLDB build somewhere

tjni · 2022-09-14T17:16:53Z

Would you mind if I extracted this commit and this commit into a separate PR to merge before this is ready? On my aarch64-darwin machine, they resolve building cargo-watch and xcbuild.

stephank · 2022-09-14T17:44:59Z

Would you mind if I extracted this commit and this commit into a separate PR to merge before this is ready? On my aarch64-darwin machine, they resolve building cargo-watch and xcbuild.

Please do! I had no idea this was broader than Swift. 🙂

stephank · 2022-10-11T05:57:02Z

I rebased and fixed a conflict with #181764. I don't believe that PR affects Darwin. On Linux, the fix from that PR doesn't appear to be necessary anymore.

Binaries for d63aef6 on x86_64-linux:

https://app.cachix.org/cache/stephank
Swift: /nix/store/7yk9wjj2zjbdm33qn2j2qpjvm7p1y0zd-swift-wrapper-5.7
SwiftPM: /nix/store/s976zxa2y0gh04hkgcdjkvs4h51927qa-swiftpm-5.7
SourceKit-LSP: /nix/store/1q8534vz6mr1p3xh4dhd6fk5zc2335n2-sourcekit-lsp-5.7

toonn

Wow, this work is considerable to say the least. Makes reviewing quite a task.
I'd like to ask to avoid further rebasing while reviews are on-going. Fixup commits are a good option for an autosquash rebase afterwards.

Thank you for the effort that must've gone into this! ❤️

pkgs/development/compilers/swift/compiler/patches/swift-nix-resource-root.patch

pkgs/development/compilers/swift/foundation/default.nix

pkgs/development/compilers/swift/libdispatch/default.nix

pkgs/development/compilers/swift/libdispatch/disable-swift-overlay.patch

toonn · 2022-11-02T13:44:30Z

pkgs/development/compilers/swift/sourcekit-lsp/default.nix

+    binPath="$(swiftpmBinPath)"
+    mkdir -p $out/bin
+    cp $binPath/sourcekit-lsp $out/bin/


Suggested change

binPath="$(swiftpmBinPath)"

mkdir -p $out/bin

cp $binPath/sourcekit-lsp $out/bin/

mkdir -p $out/bin

cp "$(swiftpmBinPath)"/sourcekit-lsp $out/bin/

The intermediate variable doesn't really add anything IMO.

Where does swiftpmBinPath come from though, a hook? And do we want to use lib.getBin with it?

toonn · 2022-11-04T19:09:42Z

pkgs/development/compilers/swift/swiftpm/default.nix

+    postInstall = (attrs.postInstall or "")
+      + lib.optionalString stdenv.isDarwin ''
+        # The install name of libraries is incorrectly set to lib/ (via our
+        # CMake setup hook) instead of lib/swift/. This'd by easily fixed by


Suggested change

# CMake setup hook) instead of lib/swift/. This'd by easily fixed by

# CMake setup hook) instead of lib/swift/. This'd be easily fixed by

toonn · 2022-11-04T19:13:42Z

pkgs/development/compilers/swift/swiftpm/default.nix

+    ''
+      + lib.optionalString (!stdenv.isDarwin) ''
+        # The cmake rules apparently only use the Darwin install convention.
+        # Fix up the installation so to module can be found on non-Darwin.


Suggested change

# Fix up the installation so to module can be found on non-Darwin.

# Fix up the installation so the module can be found on non-Darwin.

toonn · 2022-11-04T19:14:44Z

pkgs/development/compilers/swift/swiftpm/default.nix

+    ''
+      + lib.optionalString (!stdenv.isDarwin) ''
+        # The cmake rules apparently only use the Darwin install convention.
+        # Fix up the installation so to module can be found on non-Darwin.


Suggested change

# Fix up the installation so to module can be found on non-Darwin.

# Fix up the installation so the module can be found on non-Darwin.

toonn · 2022-11-04T19:15:40Z

pkgs/development/compilers/swift/swiftpm/default.nix

+      sed -i -e '/BUILD_SHARED_LIBS/d' CMakeLists.txt
+    '';
+
+    postInstall = ''


Maybe refactor this out, since it's repeated?

toonn · 2022-11-04T19:52:54Z

pkgs/development/compilers/swift/compiler/default.nix

+
+  preFixup = lib.optionalString stdenv.isLinux ''
+    # This is cheesy, but helps the patchelf hook remove /build from RPATH.
+    cd $NIX_BUILD_TOP


NIX_BUILD_TOP should usually be avoided. I think it doesn't play nice with nix-shell for example. Is there an alternative way we can do this?

I combined the glue files in a cmake-glue.nix, and it also combines these shell snippets needed for installation.

EDIT: This was supposed to be a comment on #189977 (comment)

nixos-discourse · 2022-11-06T22:05:07Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-macos-monthly/12330/34

stephank · 2022-11-21T10:45:31Z

I did a merge with a recent(ish) staging (when I started working on it again), and was able to verify a build on aarch64-darwin with these changes.

Swift has since released 5.7.1, but we can do that separately.

ylh · 2022-12-07T11:40:38Z

Reporting that this branch is of high enough quality that mpv, with some additional buildInputs tweaking, can now build with the Swift frontend instead of whatever --disable-macos-cocoa-cb gives us. This means nice things like better fullscreen support and the overlaid window decoration that vanishes along with the playback controls. Looking forward to this getting merged.

stephank · 2023-01-02T18:38:33Z

Fwiw, I believe all feedback is addressed? Not sure what else I should do here. Would be great if we could move this forward!

toonn · 2023-01-02T21:09:55Z

I've been waiting for another pair of eyes on this. If that's not forthcoming we can merge though IMO.

Atemu · 2023-01-03T08:36:53Z

Even if there is some issue you overlooked, I think this is a major improvement over the status quo with no obvious flaws. Any further refinements can always be done afterwards.

If you looked over it and everything you found has been addressed, I think it's good enough for staging/unstable.

stephank · 2023-01-16T11:20:28Z

Ping! Can we merge this? 🙂

toonn

Couple remaining questions.

pkgs/development/compilers/swift/swiftpm/default.nix

pkgs/development/compilers/swift/wrapper/default.nix

toonn

LGTM

pkgs/development/compilers/swift/swiftpm/default.nix

stephank requested a review from matthewbauer as a code owner September 6, 2022 08:32

ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Sep 6, 2022

github-actions bot added the 6.topic: stdenv Standard environment label Sep 6, 2022

stephank force-pushed the feat/swift-darwin branch from a6c8932 to ec292a8 Compare September 6, 2022 09:06

stephank mentioned this pull request Sep 6, 2022

Add Darwin support for Swift #37473

Closed

ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Sep 6, 2022

Atemu reviewed Sep 6, 2022

View reviewed changes

stephank force-pushed the feat/swift-darwin branch 2 times, most recently from e7f08ca to 6a3c17e Compare September 8, 2022 17:57

stephank force-pushed the feat/swift-darwin branch from 6a3c17e to 814fdd1 Compare September 10, 2022 12:08

stephank force-pushed the feat/swift-darwin branch from 814fdd1 to 316a741 Compare September 10, 2022 18:02

toonn reviewed Sep 12, 2022

View reviewed changes

toonn mentioned this pull request Sep 12, 2022

Two CoreFoundations lead to redefinition errors #189687

Closed

tjni mentioned this pull request Sep 14, 2022

cargo-watch, stdenv: resolve aarch64-darwin objc redefinition errors #191235

Closed

13 tasks

stephank force-pushed the feat/swift-darwin branch from 316a741 to 311a1bb Compare September 17, 2022 07:34

stephank mentioned this pull request Oct 15, 2022

apple_sdk_11_0: generate frameworks.nix #196197

Merged

13 tasks

toonn reviewed Nov 4, 2022

View reviewed changes

toonn mentioned this pull request Nov 13, 2022

mpv-unwrapped: 0.34.1 -> 0.35.0 #200914

Closed

13 tasks

stephank and others added 11 commits November 15, 2022 14:22

Merge remote-tracking branch 'origin/staging' into HEAD

ae174e2

swiftpm: improve setup hook shell conditions

0e08b08

swift: track version in a central sources.nix

449e2f1

swiftpm2nix: reduce duplication in generated files

9cb7163

swiftPackages: update swiftpm2nix generated files

7d559a3

swiftpm: typo fixes

a8f9977

swiftpm: factor out cmake glue installation

aae3556

swift: remove NIX_BUILD_TOP

a4c8c87

swiftPackages.Foundation: add note about CF

da15d44

swift: remove internal wrapperParams arg

72ba293

swift: add canary to verify no compiler dependency

a567024

swiftPackages.Dispatch: fix linux build

813c58d

ylh mentioned this pull request Dec 7, 2022

mpv: prepare for swift on darwin #204979

Closed

13 tasks

toonn reviewed Jan 16, 2023

View reviewed changes

pkgs/development/compilers/swift/swiftpm/default.nix Show resolved Hide resolved

pkgs/development/compilers/swift/wrapper/default.nix Show resolved Hide resolved

toonn approved these changes Jan 17, 2023

View reviewed changes

pkgs/development/compilers/swift/swiftpm/default.nix Show resolved Hide resolved

toonn merged commit b7590f3 into NixOS:staging Jan 17, 2023

This was referenced Jan 17, 2023

darwin.apple_sdk_11_0: add Security dependency on xpc #207123

Merged

[staging-next] swift: bootstrap using system stdlib #211908

Merged

stephank deleted the feat/swift-darwin branch February 1, 2023 20:10

	# Darwin.C.stdint. Apparently fix in later SDKs.
	# Darwin.C.stdint. Apparently fixed in later SDKs.

	# CMake setup hook) instead of lib/swift/. This'd by easily fixed by
	# CMake setup hook) instead of lib/swift/. This'd be easily fixed by

	# Fix up the installation so to module can be found on non-Darwin.
	# Fix up the installation so the module can be found on non-Darwin.

swift: split build & Darwin support. #189977

swift: split build & Darwin support. #189977

Conversation

stephank commented Sep 6, 2022 • edited Loading

Description of changes

Usage

Build process

Darwin stdenv

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephank Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

stephank commented Sep 6, 2022

stephank commented Sep 8, 2022

stephank commented Sep 10, 2022

stephank commented Sep 10, 2022 • edited Loading

toonn commented Sep 12, 2022

Atemu commented Sep 12, 2022

toonn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toonn commented Sep 12, 2022

stephank commented Sep 13, 2022 • edited Loading

tjni commented Sep 14, 2022

stephank commented Sep 14, 2022

stephank commented Oct 11, 2022

toonn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephank Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

nixos-discourse commented Nov 6, 2022

stephank commented Nov 21, 2022

ylh commented Dec 7, 2022

stephank commented Jan 2, 2023

toonn commented Jan 2, 2023

Atemu commented Jan 3, 2023

stephank commented Jan 16, 2023

toonn left a comment

Choose a reason for hiding this comment

toonn left a comment

Choose a reason for hiding this comment

stephank commented Sep 6, 2022 •

edited

Loading

stephank Sep 6, 2022 •

edited

Loading

stephank commented Sep 10, 2022 •

edited

Loading

stephank commented Sep 13, 2022 •

edited

Loading

stephank Nov 21, 2022 •

edited

Loading