darwin.stdenv: improvements and overrideSDK rewrite #287609

reckenrode · 2024-02-09T23:30:17Z

Description of changes

This PR makes several improvements to the Darwin stdenv situation.

Adds SDK root

It adds the SDK version to the stdenv as a new darwin.apple_sdk.sdkRoot package. This is done via a hook that adds -isysroot with a path to a stub SDK that just defines version information (based on the xcbuild.sdk). This is needed because clang passes -platform_version <deployment target> <sdk version> unconditionally to the linker, and the SDK version is inferred to be the same as the deployment target if -isysroot or SDKROOT do not point to SDK from which it can get the version.

macOS implements different behaviors depending on the SDK version. There are two notable examples: dark mode and version checks.

macOS requires the linked SDK to be at least 10.14 for dark mode to work. Currently, x86_64-darwin binaries in nixpkgs do not work with dark mode. With the SDK root specified, clang will pass the correct SDK version, ensuring that binaries linked against the 11.0 SDK work as expected.

For version checks, macOS returns a compatible version when the linked SDK is older than 11.0. I ran into that issue while working on #236414. MetalD3D checks the macOS version, but because the load commands in Wine’s binary indicate an older SDK, macOS returns a “compatible” version: 10.20. Since the check is for 14, the check fails, and MetalD3D does not work as expected.

Closes #265139.

Fleshes out `overrideSDK`

This PR includes a rewrite of overrideSDK, addressing most of the issues identified in #242666 (comment). This was necessary to support building Rust applications such as lapce, neovide, and wezterm on x86_64-darwin using the 11.0 SDK. It is also necessary to fix building applications that use wrapGAppsHook and the 11.0 SDK.

According to ofborg’s evalulation performance check, the rewritten overrideSDK performs about the same as the previous version even though it is more capable: it supports replacing inputs of all types, supports additional SDK-based overrides (libobjc, libs, private frameworks), and should be easier to maintain. It should also work with cross-compilation, but this is untested due to needing #256590 to fix cross-compilation support.

I plan to address overriding the SDK for Rust in a separate PR. There are some changes to rustPlatform needed to make it work robustly with overriding the SDK. Documentation will also follow in a separate PR.

Things done

Add a 👍 reaction to pull requests you find important.

reckenrode · 2024-02-09T23:32:19Z

@toonn

reckenrode · 2024-02-09T23:47:14Z

The force push added support for multiple outputs in the override (though I don’t believe any of the packages currently overridden require it).

reckenrode · 2024-02-25T14:46:49Z

Force pushed a fix for the evaluation failure and rebased on the current staging branch.

toonn

Everything looks good so far. I'll look at override-sdk.nix in another review. Didn't want to have you wait on the rest.

pkgs/os-specific/darwin/apple-sdk/sdkRoot.nix

reckenrode · 2024-03-17T21:56:46Z

Aside from adding compiler support, I added an optimization to reduce evaluation time by only building the mapping once per unique package set instead of doing it for every one then deduping at the end. I’m curious about the ofborg performance evaluation because the previous one was terrible.

reckenrode · 2024-03-18T03:22:22Z

I managed to improve the terrible performance by ~30%, but the increase in evaluation time is still terrible. Need to look into some stuff on optimizing nix expressions without losing the generality of this solution.

reckenrode · 2024-03-22T04:05:07Z

I fixed the merge conflict. I also optimized the algorithm. I left it as a separate commit, so the non-optimized version can serve as a reference. Running the ofborg check locally, it’s only a ~4.7% regression (from 386s to 404s). That’s a lot better considering it was as high as 600% and was 400% after my last attempt at optimization.

What I ended up doing was using genericClosure to calculate all dependencies for an input, then I sort and fold over them to calculate the replacements. I then update the original inputs using the new mapping I generate from that.

I’m aware of an issue with Qt 6 where it gets the wrong SDK root. The other overrides appear to work correctly though, so that’s strange. I’m planning to look into that next.

reckenrode · 2024-03-22T12:28:12Z

The Qt 6 issue is because it’s using apple_sdk_11_0.stdenv instead of using overrideSDK.

reckenrode · 2024-03-22T14:44:23Z

@ofborg eval

toonn

Mostly noticed some typos.

I honestly can't say I've understood everything in detail and I haven't done any testing yet but I wanted to provide at least a first review of overrideSDK.

I wish it was possible to with a single genericClosure invocation rather than the toposort but I think that would require the alternative solution you had in mind with memoization and that does sound worse than the current implementation.

pkgs/stdenv/darwin/override-sdk.nix

Based on the derivation from xcbuild.sdk. apple_sdk.sdkRoot provides version plists and a hook that passes them automatically to the compiler as `-isysroot`. This is needed to correctly set the SDK version in compiled binaries. Otherwise, clang will infer the SDK version to be the same as the deployment target, which is usually not what you want.

reckenrode · 2024-03-29T13:18:40Z

@AndersonTorres Sorry about the ping. A Meson I need to build locally patch accidentally snuck into the PR.

reckenrode · 2024-03-29T13:18:55Z

The latest force push rebases on current staging to pick up the libiconv changes.

reckenrode · 2024-03-29T13:52:05Z

@toonn I squashed the wip commits and set the status to ready. I also added a commit dropping unused curl assertions from the stdenv bootstrap. The contents of the PR should otherwise be the same as before the squash.

This is effectively a rewrite of `overrideSDK`. It was required because `wrapGAppsHook` propagates `depsTargetTarget` with the expectation that it will effectively be `buildInputs` when the hook is itself used as a `nativeBuildInput`. This propagates Gtk, which itself propagates the default Dariwn SDK, making it effectively impossible to override the SDK when a package depends on Gtk and uses `wrapGAppsHook`. This rewrite implements the following improvements: * Cross-compilation should be supported correctly (untested); * Supports public and private frameworks; * Supports SDK `libs`; * Remaps instead of replacing extra (native) build inputs in the stdenv; * Updates any Darwin framework references in `nix-support`; and * It updates `xcodebuild` regardless of which input its in. The implementation avoids recursion for performance reasons. Instead, it enumerates transitive dependencies and walks the list from the leaf packages backwards to the parent packages.

Setting the SDK root by default allows `overrideSDK` to correctly set the SDK version when using a different SDK. It also allows the correct SDK version to be set when using an older deployment target. Not setting the correct SDK version can result in unexpected behavior at runtime. Examples: * Automatic dark mode switching requires linking against an SDK version of 10.14 or newer. With the current behavior, the only way to do this is by using a 10.14+ deployment target even when the application supports older platforms when build with a newer SDK. * MetalD3D checks that the system version is at least 14.0. The API it uses returns a compatibility version when the the SDK is older than 11.0, which causes it to display an error and terminate the application even when even when its requirements are all met.

toonn

Still LGTM : )

nixos-discourse · 2024-03-29T21:09:11Z

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

https://discourse.nixos.org/t/darwin-updates-news/42249/7

It seems NixOS/nixpkgs#287609 caused the following build error for us on MacOS: ``` error: … while evaluating a branch condition at /nix/store/hxvhrj46m6j9m1x2z4qa4s1r4gw18sqv-source/pkgs/stdenv/booter.nix:99:7: 98| thisStage = 99| if args.__raw or false | ^ 100| then args' … in the right operand of the update (//) operator at /nix/store/hxvhrj46m6j9m1x2z4qa4s1r4gw18sqv-source/pkgs/stdenv/booter.nix:84:7: 83| { allowCustomOverrides = index == 1; } 84| // (stageFun prevStage)) | ^ 85| (lib.lists.reverseList stageFuns); (stack trace truncated; use '--show-trace' to show the full trace) error: value is a function while a set was expected ``` We should look into if this is an issue in nixpkgs or in our MacOS environment (GitHub). This commit changes the following Nix versions: * 2.21.2 -> 2.21.0 * 2.20.6 -> 2.20.5 * 2.19.4 -> 2.19.3 * 2.3.18 -> 2.3.17

github-actions bot added the 6.topic: stdenv Standard environment label Feb 9, 2024

ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Feb 9, 2024

reckenrode force-pushed the darwin-stdenv-improvements branch from cfa8ea6 to 6fb6926 Compare February 9, 2024 23:46

This was referenced Feb 9, 2024

winePackages{unstable,staging}: 9.0 -> 9.2, cleanup #286580

Merged

darwin.d3dmetal: init at 1.1 #287618

Closed

reckenrode force-pushed the darwin-stdenv-improvements branch from 6fb6926 to 549e9e3 Compare February 25, 2024 14:46

reckenrode force-pushed the darwin-stdenv-improvements branch from 549e9e3 to f93cab1 Compare February 25, 2024 14:52

ofborg bot added the 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild label Feb 25, 2024

ofborg bot requested a review from copumpkin February 25, 2024 16:38

ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 25, 2024

toonn reviewed Mar 17, 2024

View reviewed changes

pkgs/os-specific/darwin/apple-sdk/sdkRoot.nix Outdated Show resolved Hide resolved

reckenrode force-pushed the darwin-stdenv-improvements branch from f93cab1 to 4f1b301 Compare March 17, 2024 21:48

reckenrode mentioned this pull request Mar 19, 2024

Build failure: neovide (both nixpkgs-23.11 and nixpkgs-unstable) on x86_64-darwin #290611

Open

reckenrode force-pushed the darwin-stdenv-improvements branch 2 times, most recently from 43f88b1 to b53a80f Compare March 22, 2024 03:46

ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 22, 2024

reckenrode force-pushed the darwin-stdenv-improvements branch from b53a80f to 4f65fb3 Compare March 22, 2024 04:01

ofborg bot added 8.has: clean-up and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 22, 2024

toonn reviewed Mar 25, 2024

View reviewed changes

reckenrode added 2 commits March 29, 2024 08:47

darwin.apple_sdk_10_12: add objc4

79db5b6

ofborg bot requested a review from AndersonTorres March 29, 2024 13:08

ofborg bot added 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Mar 29, 2024

reckenrode force-pushed the darwin-stdenv-improvements branch from 7a3549b to 061f322 Compare March 29, 2024 13:17

reckenrode force-pushed the darwin-stdenv-improvements branch from a91f1d0 to 6ac357e Compare March 29, 2024 13:50

reckenrode marked this pull request as ready for review March 29, 2024 13:50

reckenrode added 3 commits March 29, 2024 10:00

darwin.stdenv: use wrapBintoolsWith instead of a direct import

017eda2

darwin.stdenv: drop curl from assertions

1f23117

reckenrode force-pushed the darwin-stdenv-improvements branch from 6ac357e to e6ea301 Compare March 29, 2024 14:04

reckenrode force-pushed the darwin-stdenv-improvements branch from e6ea301 to 71c6ee9 Compare March 29, 2024 14:08

ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Mar 29, 2024

toonn approved these changes Mar 29, 2024

View reviewed changes

toonn merged commit 81a44b6 into NixOS:staging Mar 29, 2024
22 checks passed

reckenrode deleted the darwin-stdenv-improvements branch March 29, 2024 17:30

reckenrode mentioned this pull request Apr 4, 2024

Build failure: Wezterm (x86-64 darwin) #239384

Closed

reckenrode mentioned this pull request Apr 24, 2024

wine-staging: 9.6 -> 9.7 #305816

Merged

ghost mentioned this pull request May 18, 2024

git-credential-keepassxc: fix build on darwin #312508

Closed

13 tasks

mendelskiv93 mentioned this pull request Oct 22, 2024

feat(ci)_: build on windows and macos status-im/status-go#5840

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

darwin.stdenv: improvements and overrideSDK rewrite #287609

darwin.stdenv: improvements and overrideSDK rewrite #287609

reckenrode commented Feb 9, 2024 •

edited

Loading

reckenrode commented Feb 9, 2024

reckenrode commented Feb 9, 2024

reckenrode commented Feb 25, 2024 •

edited

Loading

toonn left a comment •

edited

Loading

reckenrode commented Mar 17, 2024

reckenrode commented Mar 18, 2024

reckenrode commented Mar 22, 2024 •

edited

Loading

reckenrode commented Mar 22, 2024

reckenrode commented Mar 22, 2024

toonn left a comment

reckenrode commented Mar 29, 2024

reckenrode commented Mar 29, 2024

reckenrode commented Mar 29, 2024

toonn left a comment

nixos-discourse commented Mar 29, 2024

darwin.stdenv: improvements and overrideSDK rewrite #287609

darwin.stdenv: improvements and overrideSDK rewrite #287609

Conversation

reckenrode commented Feb 9, 2024 • edited Loading

Description of changes

Adds SDK root

Fleshes out overrideSDK

Things done

reckenrode commented Feb 9, 2024

reckenrode commented Feb 9, 2024

reckenrode commented Feb 25, 2024 • edited Loading

toonn left a comment • edited Loading

Choose a reason for hiding this comment

reckenrode commented Mar 17, 2024

reckenrode commented Mar 18, 2024

reckenrode commented Mar 22, 2024 • edited Loading

reckenrode commented Mar 22, 2024

reckenrode commented Mar 22, 2024

toonn left a comment

Choose a reason for hiding this comment

reckenrode commented Mar 29, 2024

reckenrode commented Mar 29, 2024

reckenrode commented Mar 29, 2024

toonn left a comment

Choose a reason for hiding this comment

nixos-discourse commented Mar 29, 2024

reckenrode commented Feb 9, 2024 •

edited

Loading

Fleshes out `overrideSDK`

reckenrode commented Feb 25, 2024 •

edited

Loading

toonn left a comment •

edited

Loading

reckenrode commented Mar 22, 2024 •

edited

Loading