treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready #324155

paparodeo · 2024-07-02T21:26:02Z

Description of changes

continuation of #323679 which i closed and can not re-open.

with this change and plus a modification to lib/systems/default.nix x64 darwin will default to sdk 11.0.

the first commit modifies various conditionals to not assume that x86_64-darwin uses sdk 10.12 and instead checks the darwinSdkVersion. Additionally, darwin.xnu is modified so x64 darwin uses unwrapped clang when generating header files, as the headers require a 32 bit arch which is not allowed when using cc-wrapper as it sets -arch to x86_64. this commit is sufficient for building stdenv when darwinSdkVersion is set to "11.0" in lib/systems/default.nix.

the second commit fixes evaluation errors which assume that x64 darwin can access the apple_sdk.sdk attribute which is only available as part of the 10.12 sdk so rather than checking for x64 darwin, condition on darwinSdkVersion is older than "11.0".

with these two commits and the following diff x64 darwin will default to using sdk 11.0

enable sdk 11 on all platforms

--- a/lib/systems/default.nix
+++ b/lib/systems/default.nix
@@ -251,7 +251,7 @@ let
         else null;
       # The canonical name for this attribute is darwinSdkVersion, but some
       # platforms define the old name "sdkVer".
-      darwinSdkVersion = final.sdkVer or (if final.isAarch64 then "11.0" else "10.12");
+      darwinSdkVersion = final.sdkVer "11.0";
       darwinMinVersion = final.darwinSdkVersion;
       darwinMinVersionVariable =
         if final.isMacOS then "MACOSX_DEPLOYMENT_TARGET"

testing: using the patch to enable sdk 11, built inkscape and verified removing overrideSDK for a few sampled packages (vcpkg-tool, wasmedge) works.

Things done

cc: @emilazy @reckenrode

Add a 👍 reaction to pull requests you find important.

pkgs/os-specific/darwin/pam-reattach/default.nix

pkgs/development/compilers/llvm/13/default.nix

reckenrode · 2024-07-02T22:30:43Z

pkgs/development/libraries/webkitgtk/default.nix

-  ] ++ lib.optional (stdenv.isDarwin && !stdenv.isAarch64) (
+  ] ++ lib.optional (stdenv.isDarwin && apple_sdk ? sdk) (


Is this still necessary? libproc.h is included in the 10.12 SDK Libsystem and should be identical to this one.

hmm, webkitgtk is marked as broken on darwin so not sure. i added a comment stating it might not be necessary and switched the check to sdk is < 11

reckenrode · 2024-07-02T22:36:08Z

pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix

+  # use unwrapped clang to generate headers because wrapper is not compatible with a 32 bit -arch.
+  # aarch64 should likely do this as well and remove the --replace MACHINE_ARCH above
+  MIGCC = if stdenv.isx86_64 && lib.versionAtLeast stdenv.hostPlatform.darwinSdkVersion "11"
+    then "${lib.getBin buildPackages.stdenv.cc.cc}/bin/clang"


mig needs to target the build platform because it’s run at build time to generate headers.

Suggested change

then "${lib.getBin buildPackages.stdenv.cc.cc}/bin/clang"

then "${lib.getBin pkgsBuildBuild.stdenv.cc.cc}/bin/clang"

I’d also add this can be done unconditionally. I do something similar in 37a87d0, but I think this way is probably cleaner. I can’t remember why I didn’t specify a full path to for MIGCC in that commit.

i don't have aarch64 to test on. i suspect that making it unconditional means that MACHINE_ARCH doesn't need to get patched for aarch64 as well, and thus observing the comment saying that the header build needs to use 32 bit arch. it does change the layouts for the x64 darwin structures so i assume that the aarch64 ones have mismatched structure sizes as well.

i'd like this to be part of a followup given the lack of hardware and would require the PR to move staging. created #324440 -- will cause a merge conflict with this PR but should be easy enough to resolve.

pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix

reckenrode · 2024-07-02T22:39:37Z

Also pinging @toonn.

update code to not assume that x64 darwin must use sdk 10.12. After this change it's possible to build a sdk 11 stdenv on darwin x64

update code to not assume that x64 darwin uses sdk 10.12. These changes want to reach into apple_sdk.sdk to grab a header file which is only required for sdk 10.12.

toonn

Most of it looks good to me, just a couple questions.

toonn · 2024-07-07T15:47:55Z

pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix

-  MIGCC = "cc";
+  # use unwrapped clang to generate headers because wrapper is not compatible with a 32 bit -arch.
+  # aarch64 should likely do this as well and remove the --replace MACHINE_ARCH above
+  MIGCC = if stdenv.isx86_64 && lib.versionAtLeast stdenv.hostPlatform.darwinSdkVersion "11"


I think this shouldn't be conditional? Even the 10.12 script relies on the 32-bit arch compiler. Or does the wrapper only force a 64-bit architecture when using the 11 SDK?

toonn · 2024-07-07T17:37:15Z

pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix

@@ -150,7 +154,7 @@ appleDerivation' (if headersOnly then stdenvNoCC else stdenv) (
    mv $out/Library/Frameworks/IOKit.framework $out/Library/PrivateFrameworks
  '';

-  appleHeaders = builtins.readFile (./. + "/headers-${arch}.txt");
+  appleHeaders = builtins.readFile (./. + "/headers-${stdenv.hostPlatform.darwinSdkVersion}-${arch}.txt");


Will we ever want to split on more than the architecture? I'd say the goal is to unsplit these header checks eventually. If they're even still useful.

The way to do these in the future probably requires us to inventory the upstream SDKs and compare our headers to those in the SDK. Otherwise, using headers + stubs with more than a few SDKs is going to get painful quickly.

(Or I wouldn’t worry too much about the current situation because it’s going to need to change.)

RossComputerGuy

LGTM, however I would like to get #325175 in before we merge any other LLVM changes to minimize rebasing and conflicts.

toonn · 2024-07-07T18:52:28Z

@RossComputerGuy, how close is that PR to being marked ready?

RossComputerGuy · 2024-07-07T19:00:49Z

@toonn it's ready, trying to figure out CI failures. Should be ready tonight or within the next couple days.

paparodeo · 2024-07-07T23:59:14Z

@toonn it's ready, trying to figure out CI failures. Should be ready tonight or within the next couple days.

so it is not ready.

RossComputerGuy · 2024-07-08T00:01:29Z

so it is not ready.

The actual work is done, there's just CI roadblocks. I checked using nix-diff and rebuild count should be 0. It sounds like the CI issue it's having should be simpler enough and only affects Darwin.

paparodeo · 2024-07-08T03:19:12Z

so it is not ready.

The actual work is done, there's just CI roadblocks. I checked using nix-diff and rebuild count should be 0. It sounds like the CI issue it's having should be simpler enough and only affects Darwin.

it only effects the builds which have a stdenv based on llvm. i am not sure how you think it is good etiquette to think that you should be able to open up a non-critical PR that creates merge conflicts with this one and then request that this not get merged so you don't have to resolve the merge conflicts with the files you deleted so I can deal with fixing them in this PR. And your PR doesn't even work yet. Fixing the merge conflicts is not really a big deal for me but your request is just rude.

RossComputerGuy · 2024-07-08T14:33:43Z

Fixing the merge conflicts is not really a big deal for me but your request is just rude.

Sorry, it wasn't my intention to be rude. I'll merge this one first if nobody has anything left to address.

paparodeo · 2024-07-08T21:08:03Z

Fixing the merge conflicts is not really a big deal for me but your request is just rude.

Sorry, it wasn't my intention to be rude. I'll merge this one first if nobody has anything left to address.

it doesn't really matter. i overreacted.

wentasah · 2024-09-19T19:48:53Z

Hi,

in the nix-ros-overlay project, we're experiencing the following eval failure:

error: opening file '/nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/xnu/headers-10.12-arm64.txt': No such file or directory

Can somebody here suggests what's wrong?

paparodeo · 2024-09-19T20:01:06Z

Hi,

in the nix-ros-overlay project, we're experiencing the following eval failure:
error: opening file '/nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/xnu/headers-10.12-arm64.txt': No such file or directory
Can somebody here suggests what's wrong?

arm64 doesnt support 10.12

reckenrode · 2024-09-19T20:57:22Z

Hi,

in the nix-ros-overlay project, we're experiencing the following eval failure:
error: opening file '/nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/xnu/headers-10.12-arm64.txt': No such file or directory
Can somebody here suggests what's wrong?

Somehow stdenv.hostPlatform.darwinSdkVersion is being set to 10.12 on aarch64-darwin. The oldest version supported by aarch64-darwin is 11.0, which is the current default in nixpkgs.

nixpkgs/pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix

Line 152 in 8ce34b3

    
           appleHeaders = builtins.readFile (./. + "/headers-${stdenv.hostPlatform.darwinSdkVersion}-${arch}.txt");

wentasah · 2024-09-19T21:07:47Z

Thanks for quick answers. I don't think we pin the SDK versions anywhere. Who and how can do that? Can you see something interesting from the trace? Posting just the end, which seems to contain some package names, not just generic nixpkgs stuff:

      … in the condition of the assert statement
         at /nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/build-support/cc-wrapper/default.nix:293:1:
          292| # Ensure bintools matches
          293| assert libc_bin == bintools.libc_bin;
             | ^
          294| assert libc_dev == bintools.libc_dev;

       … while calling the 'getAttr' builtin
         at <nix/derivation-internal.nix>:44:19:
           43|       value = commonAttrs // {
           44|         outPath = builtins.getAttr outputName strict;
             |                   ^
           45|         drvPath = strict.drvPath;

       … while calling the 'derivationStrict' builtin
         at <nix/derivation-internal.nix>:34:12:
           33|
           34|   strict = derivationStrict drvAttrs;
             |            ^
           35|

       … while evaluating derivation 'Libsystem-1238.60.2'
         whose name attribute is located at /nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/stdenv/generic/make-derivation.nix:336:7

       … while evaluating attribute 'installPhase' of derivation 'Libsystem-1238.60.2'
         at /nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/Libsystem/default.nix:36:3:
           35|
           36|   installPhase = ''
             |   ^
           37|     export NIX_ENFORCE_PURITY=

       … while calling the 'getAttr' builtin
         at <nix/derivation-internal.nix>:44:19:
           43|       value = commonAttrs // {
           44|         outPath = builtins.getAttr outputName strict;
             |                   ^
           45|         drvPath = strict.drvPath;

       … while calling the 'derivationStrict' builtin
         at <nix/derivation-internal.nix>:34:12:
           33|
           34|   strict = derivationStrict drvAttrs;
             |            ^
           35|

       … while evaluating derivation 'xnu-3789.70.16'
         whose name attribute is located at /nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/stdenv/generic/make-derivation.nix:336:7

       … while evaluating attribute 'appleHeaders' of derivation 'xnu-3789.70.16'
         at /nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix:152:3:
          151|
          152|   appleHeaders = builtins.readFile (./. + "/headers-${stdenv.hostPlatform.darwinSdkVersion}-${arch}.txt");
             |   ^
          153| } // lib.optionalAttrs headersOnly {

       … while calling the 'readFile' builtin
         at /nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix:152:18:
          151|
          152|   appleHeaders = builtins.readFile (./. + "/headers-${stdenv.hostPlatform.darwinSdkVersion}-${arch}.txt");
             |                  ^
          153| } // lib.optionalAttrs headersOnly {

       error: opening file '/nix/store/l3amk5lsakpc93him5kry24kax23sn4h-source/pkgs/os-specific/darwin/apple-source-releases/xnu/headers-10.12-arm64.txt': No such file or directory

Full trace here.

emilazy · 2024-09-19T21:24:09Z

Do you have a flake command that reliably reproduces the failure?

wentasah · 2024-09-19T21:27:06Z

Yes: nix flake show github:lopsided98/nix-ros-overlay

reckenrode · 2024-09-19T22:05:02Z

I added tracing to your flake. It’s failing when it tries to evaluate armv7a-darwin.

Edit: This is a minimal reproducer:

$ nix eval --impure -E 'import (builtins.getFlake "github:NixOS/nixpkgs")  { system = "armv7a-darwin"; }'
error:
       … while evaluating a branch condition
         at /nix/store/7mfai28m4yqdnvhbyzd7jg57nzlwgc73-source/pkgs/stdenv/booter.nix:99:7:
           98|     thisStage =
           99|       if args.__raw or false
             |       ^
          100|       then args'

       … while evaluating 'args' to select '__raw' on it
         at /nix/store/7mfai28m4yqdnvhbyzd7jg57nzlwgc73-source/pkgs/stdenv/booter.nix:99:10:
           98|     thisStage =
           99|       if args.__raw or false
             |          ^
          100|       then args'

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: opening file '/nix/store/7mfai28m4yqdnvhbyzd7jg57nzlwgc73-source/pkgs/os-specific/darwin/apple-source-releases/xnu/headers-10.12-arm64.txt': No such file or directory

wentasah · 2024-09-19T22:11:05Z

I guess, this is some niche platform, which we can disable. But probably it should not look for headers-*-arm64.txt, because it's 32bit platform.

emilazy · 2024-09-19T22:17:26Z

armv7a-darwin is not a platform that has ever been supported or that really makes much sense beyond unsupported iPhones. You should probably use something more like Nixpkgs’ lib.systems.flakeExposed list.

wentasah · 2024-09-19T22:22:56Z

Sounds like a good idea. Thanks to all for help.

reckenrode · 2024-09-19T22:26:04Z

armv7a-darwin is included in lib.platforms.darwin for some reason. Maybe it worked in the past, but I can’t imagine it’s worked anytime recently. macOS has never supported 32-bit ARM (and Apple Silicon doesn’t implement it). iOS dropped support in iOS 11, which was released in 2017. Apple stopped accepting 32-bit submissions to the App Store in 2015.

This gets rid of evaluation error for armv7a-darwin. It makes no sense to support this platform in this overlay. According to the discussion around around NixOS/nixpkgs#324155 (comment), it is better to support just the systems exposed by the nixpkgs flake. Currently, these are: - aarch64-darwin - aarch64-linux - armv6l-linux - armv7l-linux - i686-linux - powerpc64le-linux - riscv64-linux - x86_64-darwin - x86_64-freebsd - 86_64-linux

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

paparodeo mentioned this pull request Jul 2, 2024

WIP x64 darwin default to sdk 11 #323679

Closed

13 tasks

paparodeo changed the title ~~treewide: change various flags to allow x64 darwin to default to sdk 11.0~~ treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready Jul 2, 2024

reckenrode reviewed Jul 2, 2024

View reviewed changes

pkgs/os-specific/darwin/pam-reattach/default.nix Outdated Show resolved Hide resolved

reckenrode reviewed Jul 2, 2024

View reviewed changes

pkgs/development/compilers/llvm/13/default.nix Outdated Show resolved Hide resolved

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

reckenrode reviewed Jul 2, 2024

View reviewed changes

pkgs/os-specific/darwin/apple-source-releases/xnu/default.nix Outdated Show resolved Hide resolved

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 2, 2024

paparodeo added 2 commits July 2, 2024 23:30

treewide: fixes to allow x64 darwin to default to sdk 11

3cb23ce

update code to not assume that x64 darwin must use sdk 10.12. After this change it's possible to build a sdk 11 stdenv on darwin x64

treewide: only access apple_sdk.sdk when sdk < 11

07cba93

update code to not assume that x64 darwin uses sdk 10.12. These changes want to reach into apple_sdk.sdk to grab a header file which is only required for sdk 10.12.

paparodeo force-pushed the x64-sdk11-no-rebuilds branch from b88d9a5 to 07cba93 Compare July 3, 2024 00:05

pam-reattach: minor cmakeFlags cleanup

d44ffe5

paparodeo marked this pull request as ready for review July 3, 2024 00:30

paparodeo requested a review from RossComputerGuy as a code owner July 3, 2024 00:30

paparodeo mentioned this pull request Jul 3, 2024

darwin.xnu: use unwrapped clang for MIGCC #324440

Merged

13 tasks

toonn reviewed Jul 7, 2024

View reviewed changes

RossComputerGuy reviewed Jul 7, 2024

View reviewed changes

RossComputerGuy approved these changes Jul 8, 2024

View reviewed changes

wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jul 8, 2024

RossComputerGuy mentioned this pull request Jul 8, 2024

llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #325175

Merged

13 tasks

RossComputerGuy merged commit 7a95a89 into NixOS:master Jul 9, 2024
28 checks passed

paparodeo deleted the x64-sdk11-no-rebuilds branch July 9, 2024 00:24

wentasah mentioned this pull request Sep 19, 2024

Eval error after recent nixpkgs update lopsided98/nix-ros-overlay#484

Closed

wentasah mentioned this pull request Sep 19, 2024

Reduce the number of systems offered by the flake lopsided98/nix-ros-overlay#485

Merged

emilazy mentioned this pull request Oct 13, 2024

maintainers/team-list: establish the Darwin team #348183

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready #324155

treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready #324155

paparodeo commented Jul 2, 2024 •

edited

Loading

reckenrode Jul 2, 2024

paparodeo Jul 2, 2024 •

edited

Loading

reckenrode Jul 2, 2024

reckenrode Jul 2, 2024

paparodeo Jul 2, 2024 •

edited

Loading

reckenrode commented Jul 2, 2024

toonn left a comment

toonn Jul 7, 2024

toonn Jul 7, 2024

reckenrode Jul 7, 2024

RossComputerGuy left a comment

toonn commented Jul 7, 2024 •

edited

Loading

RossComputerGuy commented Jul 7, 2024

paparodeo commented Jul 7, 2024

RossComputerGuy commented Jul 8, 2024

paparodeo commented Jul 8, 2024

RossComputerGuy commented Jul 8, 2024

paparodeo commented Jul 8, 2024

wentasah commented Sep 19, 2024

paparodeo commented Sep 19, 2024

reckenrode commented Sep 19, 2024 •

edited

Loading

wentasah commented Sep 19, 2024 •

edited

Loading

emilazy commented Sep 19, 2024

wentasah commented Sep 19, 2024

reckenrode commented Sep 19, 2024 •

edited

Loading

wentasah commented Sep 19, 2024 •

edited

Loading

emilazy commented Sep 19, 2024 •

edited

Loading

wentasah commented Sep 19, 2024

reckenrode commented Sep 19, 2024

		] ++ lib.optional (stdenv.isDarwin && !stdenv.isAarch64) (
		] ++ lib.optional (stdenv.isDarwin && apple_sdk ? sdk) (

	then "${lib.getBin buildPackages.stdenv.cc.cc}/bin/clang"
	then "${lib.getBin pkgsBuildBuild.stdenv.cc.cc}/bin/clang"

treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready #324155

treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready #324155

Conversation

paparodeo commented Jul 2, 2024 • edited Loading

Description of changes

Things done

reckenrode Jul 2, 2024

Choose a reason for hiding this comment

paparodeo Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

reckenrode Jul 2, 2024

Choose a reason for hiding this comment

reckenrode Jul 2, 2024

Choose a reason for hiding this comment

paparodeo Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

reckenrode commented Jul 2, 2024

toonn left a comment

Choose a reason for hiding this comment

toonn Jul 7, 2024

Choose a reason for hiding this comment

toonn Jul 7, 2024

Choose a reason for hiding this comment

reckenrode Jul 7, 2024

Choose a reason for hiding this comment

RossComputerGuy left a comment

Choose a reason for hiding this comment

toonn commented Jul 7, 2024 • edited Loading

RossComputerGuy commented Jul 7, 2024

paparodeo commented Jul 7, 2024

RossComputerGuy commented Jul 8, 2024

paparodeo commented Jul 8, 2024

RossComputerGuy commented Jul 8, 2024

paparodeo commented Jul 8, 2024

wentasah commented Sep 19, 2024

paparodeo commented Sep 19, 2024

reckenrode commented Sep 19, 2024 • edited Loading

wentasah commented Sep 19, 2024 • edited Loading

emilazy commented Sep 19, 2024

wentasah commented Sep 19, 2024

reckenrode commented Sep 19, 2024 • edited Loading

wentasah commented Sep 19, 2024 • edited Loading

emilazy commented Sep 19, 2024 • edited Loading

wentasah commented Sep 19, 2024

reckenrode commented Sep 19, 2024

paparodeo commented Jul 2, 2024 •

edited

Loading

paparodeo Jul 2, 2024 •

edited

Loading

paparodeo Jul 2, 2024 •

edited

Loading

toonn commented Jul 7, 2024 •

edited

Loading

reckenrode commented Sep 19, 2024 •

edited

Loading

wentasah commented Sep 19, 2024 •

edited

Loading

reckenrode commented Sep 19, 2024 •

edited

Loading

wentasah commented Sep 19, 2024 •

edited

Loading

emilazy commented Sep 19, 2024 •

edited

Loading