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

WIP x64 darwin default to sdk 11 #323679

Closed
wants to merge 4 commits into from

Conversation

paparodeo
Copy link
Contributor

@paparodeo paparodeo commented Jul 1, 2024

Description of changes

changes to make x86-64 darwin default to sdk 11.

the flags used to check for the sdk just assumed that x64 darwin was correlated with sdk 10.12. that is no longer the case with this change tho the updated flags can be improved.

testing: built stdenv on x64 darwin

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.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 1, 2024
@github-actions github-actions bot added 6.topic: stdenv Standard environment 6.topic: lib The Nixpkgs function library labels Jul 1, 2024
this is probably also not necessarily correct
@paparodeo
Copy link
Contributor Author

cc @emilazy

causes xnu build to fail because it requires headers generated using
-arch x86_32
@emilazy
Copy link
Member

emilazy commented Jul 1, 2024

I support aligning x86_64-darwin and aarch64-darwin here, but don’t think we should do it until @reckenrode’s SDK rework happens.

@paparodeo
Copy link
Contributor Author

I support aligning x86_64-darwin and aarch64-darwin here, but don’t think we should do it until @reckenrode’s SDK rework happens.

ok. i built stdenv then tried inkscape but that dies on rustc #318674. I don't think that the SDK rework and this change are mutually exclusive -- or at least i do not think the sdk rework has started. keeping x64 darwin at 10.12 should be possible with this change by leaving darwinSdkVersion = final.sdkVer or (if final.isAarch64 then "11.0" else "10.12"); in lib/systems/default.nix.

@reckenrode
Copy link
Contributor

The SDK refactor is going to consolidate the separate SDK hierarchies and provide a common pattern for building SDKs. I’m using GCC as a point of reference, which does this pretty well. The Darwin stdenv will see extensive changes. All the separate code paths will be removed. The more extreme changes will remove frameworks and rely on SDKROOT.

But I need to land #307880 and #256590 first before I even look at making SDK changes.

@paparodeo
Copy link
Contributor Author

i still don't understand how this change conflicts with a sdk refactor that has not started. IIRC, when this change was proposed in late march by annalee the sdk refactor that was going to use gcc as a point of reference was still right around the corner.

most of what this change is doing is changing stdenv.isDarwin && stdenv.isx86_64 into stdenv.isDarwin && lib.versionOlder stdenv.hostPlatform.darwinSdkVersion "11", or at least that is the idea, so a modification to set darwinSdkVersion in lib/systems/default.nix will change the default sdk.

these changes could go into master today with xnu refactor and dropping the cc-wrapper and lib/systems/default.nix change and cause 0 rebuilds. i don't see how that would conflict with a future sdk refactor either.

nonetheless, i don't want to spend time on a change that is going nowhere so closing.

@paparodeo paparodeo closed this Jul 1, 2024
@paparodeo paparodeo deleted the x64-sdk11-staging branch July 2, 2024 00:00
@reckenrode
Copy link
Contributor

The “conflict” would be losing the test case of having multiple SDKs and needing to support overriding the SDK version. If it got bumped in the meantime, it wouldn’t be my preference, but it wouldn’t stop that work.

IIRC, when this change was proposed in late march by annalee the sdk refactor that was going to use gcc as a point of reference was still right around the corner.

I’m sorry. It’s targeting the 24.11 release, so I’ve prioritized getting the ld64 and the Darwin cross-compilation PRs done first. I have only so much bandwidth, and I need to be careful to avoid burnout. (I also made a misstep of starting work on the clang 18 update while I was blocked during the 24.05 release freeze, but I put that on hold to focus on this other stuff.)

@paparodeo
Copy link
Contributor Author

paparodeo commented Jul 2, 2024

I don't see how this change prohibits testing multiple sdks -- i see how changing the default for x64 back to 10.12 will require a stdenv rebuild tho i am assuming that an sdk refactor would do that anyway.

i'm not trying to be critical of the sdk refactor not being far enough along, more just trying to be realistic about timelines. if waiting for sdk refactor i don't think that trying x64 darwin at sdk 11 happens till after 24.11.

[edit] i also don't view this change as high priority -- the sdk refactor / rewrite seems much more beneficial.

@reckenrode
Copy link
Contributor

My primary (albeit unlikely) concern would be whether switching the default back even worked. A more realistic concern would be packages removing their overrides.

i'm not trying to be critical of the sdk refactor not being far enough along, more just trying to be realistic about timelines. if waiting for sdk refactor i don't think that trying x64 darwin at sdk 11 happens till after 24.11.

And I’m not trying to slow progress with vaporware promises. 🙂

Honestly, I don’t know if updating the SDK version would possible today even if the refactor were done. When there was talk of bumping to the minimum supported version to 10.13 (for ld64 before I found a workaround), the release team wanted to put an announcement in the 24.05 release before changing the minimum version. As far as I know, that announcement didn’t make the 24.05 release notes, so I’ve been assuming the refactor would not be accompanied by a minimum version bump.

the sdk refactor / rewrite seems much more beneficial.

I’ve been calling it a refactor because the first stage would be landing the 10.12.4 and 11.3 SDKs in the common pattern and as compatible as possible attribute-wise, but it’s definitely a rewrite.

@paparodeo paparodeo restored the x64-sdk11-staging branch July 2, 2024 04:54
@emilazy
Copy link
Member

emilazy commented Jul 2, 2024

The “conflict” would be losing the test case of having multiple SDKs and needing to support overriding the SDK version. If it got bumped in the meantime, it wouldn’t be my preference, but it wouldn’t stop that work.

There’s some stuff that wants the 12.0 SDK, right? Would it be more or less fuss to add that to test those kinds of things in the rework, as opposed to recreating the current SDK set?

Honestly I’d personally be really happy if we could make do with just one SDK, but I don’t know if that’s remotely viable. Still, if it’d be possible to land this without interfering significantly with your SDK rework I’d be in support of it. x86_64-darwin is in its twilight and the less divergence it has with aarch64-darwin the more practical it’ll be to keep it going as long as we can. I suspect the amount of breakage even on older macOS versions will be manageable, especially if we accept best‐effort patches to fix it.

@paparodeo
Copy link
Contributor Author

paparodeo commented Jul 2, 2024

i am unable to re-open this PR so i created a new one: #324155
i was able to drop the change to cc-wrapper by using an unwrapped clang in the xnu header build. also, i left out the change to lib/systems/default.nix which changes the default sdk to "11.0" for x64 darwin to hopefully ease this process a little and just get the code ready for the change without actually making the change.

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.

3 participants