swiftPackages.*: build with the default LLVM version#439408
swiftPackages.*: build with the default LLVM version#439408emilazy merged 13 commits intoNixOS:masterfrom
Conversation
I used this to verify the libc++ changes.
Honestly I did this in the middle of trying things and then forgot about it, and I just don’t feel like bootstrapping the whole thing again to drop this commit.
The versions of `llvmPackages` that require this patch are going away soon.
Swift needs Clang to link with. The linker patch breaks linking when using Swift with a GCC‐based standard environment. Similarly, the internal Clang headers in the resource directory are coupled to the corresponding version of Clang. The resource root patch caused Swift’s Clang to use the resource directory from the version of Clang used in the build environment, which is only compatible if the versions match. Instead, hard‐code the Clang path in the patches and wrappers.
libc++ only supports a limited range of Clang versions, so this was relying on the LLVM version corresponding to the one used in Swift’s fork. Since Swift builds its own LLVM 15 anyway, we can simply install the required libc++ headers along with it.
nixpkgs-ci
bot
added
10.rebuild-linux: 11-100
nix-owners
bot
requested review from
Trundle,
cmm,
dduan,
stephank and
trepetti
GCC doesn’t know what an `-fblocks` or `-fmodules` is. Probably SwiftPM should hard‐code Clang but I’m tired.
Swift now works fine with standard environments using GCC or other versions of LLVM. Some packages, like components of Swift itself or others that combine Swift with features like C++ modules or C blocks, may still need to use an LLVM‐based standard environment, but we do not need to enforce a uniform `swiftPackages.stdenv` any more.
emilazy
force-pushed
the
push-tlqnrmlupwyy
branch
from
e607b26 to
997d97b
Compare
|
| # NOTE: We don't symlink directly here, because that'd add a run-time dep | ||
| # on the full Clang compiler to every Swift executable. The copy here is |
There was a problem hiding this comment.
might be worth keeping this comment around?
There was a problem hiding this comment.
It’d need some tweaking to the wording now that include isn’t also a symlink, and the whole derivation is getting rewritten by Randy anyway. Not sure it’s worth it. This stuff shouldn’t be in $lib and getting pulled in as a runtime dependency of Swift applications at all, anyway…
| inherit (swiftLlvmPackages) clang; | ||
|
|
||
| # Overrides that create a useful environment for swift packages, allowing | ||
| # packaging with `swiftPackages.callPackage`. | ||
| inherit (clang) bintools; |
There was a problem hiding this comment.
i suppose removing these are also breaking changes, though i see no in-tree usage of them
There was a problem hiding this comment.
Frankly the default stdenv change is breaking too, but I just didn’t feel like replacing all the occurrences throughout the tree. These other overrides are for swiftPackages.callPackage, and the default clang and bintools will be used instead in that case.
There was a problem hiding this comment.
Mostly I say this because (assuming I am understanding this correctly) stdenv will change but swiftPackages.stdenv will still eval, while swiftPackages.clang and swiftPackages.bintools are totally gone. But it sounds like nothing should have been using them, so I'm happy.
| nix-update-script, | ||
| }: | ||
| let | ||
| inherit (swiftPackages) stdenv; |
There was a problem hiding this comment.
Do you know why this error is triggered? My best guess at the moment is something in swift-crypto, which suggests that it isn't an isolated issue and will probably come up again.
There was a problem hiding this comment.
Yeah. As mentioned in the commit message, SwiftPM should potentially hard‐code Clang for compiling C code or something. But needing a separate swiftPackages.stdenv sucks and I consider this more a problem of the way we’re handling SwiftPM currently (e.g. a separate swift-crypto package could override the compiler itself, if we had one).
Since all of this is being rewritten – hopefully for 25.11 – I don’t want to try and do more surgery to clean things up, when a lot of the fundamentals have big issues (like the entanglement between the Swift wrapper and cc-wrapper). This was just a blocker for removing old LLVMs that already took way more time than I’d like. The fallback option if we don’t like this is to set swiftPackages.stdenv to llvmPackages.stdenv, but it’s not required for e.g. swiftformat or protoc-gen-swift, at least.
|
am running some builds (on aarch64-darwin) but assuming they pass this looks good to me |
|
(FWIW I’m going to run |
nixpkgs-ci
bot
added
the
12.approvals: 1
|
|
Failures seem clearly like existing issues or sandbox problems. I think this is good to merge. |
|
Whats the EDIT: Yeah, nevermind... looks like it was ran from pre-fix |
nixpkgs-ci
bot
added
12.approvals: 2
nixpkgs-ci
bot
added
12.approvals: 3+
4 participants
This one was a bit of a pain.
Swift 5.8 uses its own fork of LLVM 15, and depends on its custom version of Clang to build and at runtime. It builds these as part of the Swift compiler derivation. However, it also uses
llvmPackages_15.stdenvfor that build and for the build of downstream Swift packages, and has some entangled dependencies on it, like depending on its Clang resource directory and libc++ headers. This PR disentangles those dependencies from the bootstrap, patches, and wrappers, and backports a few upstream patches to let the bootstrap go through with our default LLVM 19.The derivation has fallen behind upstream Swift releases and its various issues should be resolved with @reckenrode’s upcoming rewrite. In the meantime, this drops the dependency on
llvmPackages_15to unblock its removal, while keeping the build of Swift’s custom LLVM 15 fork inside the compiler derivation.I have tested a bunch of builds already, but will now run
nixpkgs-review.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.