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

go: propagate the apple sdk #332146

Merged
merged 1 commit into from
Oct 11, 2024
Merged

go: propagate the apple sdk #332146

merged 1 commit into from
Oct 11, 2024

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented Aug 4, 2024

Description of changes

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.

@zowoq zowoq added the 2.status: work-in-progress This PR isn't done label Aug 4, 2024
@zowoq zowoq self-assigned this Aug 4, 2024
@emilazy
Copy link
Member

emilazy commented Aug 4, 2024

It might be best to hold off on this for now unless there's a pressing need, as the upcoming SDK rework is likely to change this interface again (to something much simpler, thankfully).

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Aug 4, 2024
@reckenrode
Copy link
Contributor

To add some detail and background: overrideSDK was added at literally the last minute to 23.11. I had to add it during the final staging-next cycle to fix curl because otherwise it was going to break things propagating frameworks. That version was incomplete. I improved it for 24.05, but I realized during that work it would not be possible to implement it generally. In particular, anything that is a dependency but not an input is invisible to it (e.g., in a hook or a file or a string).

The Darwin refactor will turn the SDK into a normal package. If you need the 11.x SDK, you add apple-sdk_11 as a build input. If your package exposes the SDK as part of its public API, you can propagate it. This will all work as expected for a normal package. If you need to set the deployment target, you will be able to use darwinDeploymentTargetHook to do that.

Another aspect of this is I am reworking how SDKs are built. I have the 10.12 through 14.4 SDKs in my WIP branch. They all build, and they all have source-based components. There’s not going to be a difference in SDKs between Darwin platforms other than what’s available. xcrun will also be available in the stdenv, pointing the nixpkg SDK and binaries.

I have a very WIP branch available at https://github.com/reckenrode/nixpkgs/tree/darwin-sdk-refactor. It’s very messy (because I’m using an adapter right now to avoid stdenv bootstraps), but the core of it works. I have successfully built Wine, MoltenVK, RPM, xcbuild, and btop with appropriate SDKs. It’s very unlikely this will land for the next staging-next cycle (because I still have to sort out bootstrapping the stdenv), but it may the one after that, and it will definitely be in 24.11.

@ofborg ofborg bot requested review from Mic92, katexochen, qbit, kalbasit and mfrw August 4, 2024 04:07
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 501+ 10.rebuild-darwin: 2501-5000 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Aug 4, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 10, 2024
@reckenrode
Copy link
Contributor

The new SDK stuff is at #346043. For Go’s purpose, you’ll want to propagate the minimum SDK and deployment target like I did for Swift in #346947.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 7, 2024
@reckenrode
Copy link
Contributor

If you don’t mind, I added this PR to the follow up tracking list at #346043.

@zowoq zowoq changed the title go: move darwin to overrideSDK go: propagate the apple sdk Oct 8, 2024
@zowoq zowoq removed the 2.status: work-in-progress This PR isn't done label Oct 11, 2024
@zowoq
Copy link
Contributor Author

zowoq commented Oct 11, 2024

@ofborg eval

@zowoq zowoq marked this pull request as ready for review October 11, 2024 00:14
@zowoq zowoq removed their assignment Oct 11, 2024
@emilazy
Copy link
Member

emilazy commented Oct 11, 2024

@ofborg eval

@emilazy
Copy link
Member

emilazy commented Oct 11, 2024

Not sure why ofborg won’t eval. Can you try rebasing this on the latest staging to kick it into gear?

@zowoq
Copy link
Contributor Author

zowoq commented Oct 11, 2024

Not much point waiting for ofborg, the eval queue is ~90.

@zowoq zowoq merged commit 05cf0c4 into NixOS:staging Oct 11, 2024
13 of 14 checks passed
@zowoq zowoq deleted the go-overridesdk branch October 11, 2024 01:42
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/title-the-darwin-sdks-have-been-updated/55295/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: golang 10.rebuild-darwin: 501+ 10.rebuild-darwin: 2501-5000 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants