Skip to content

Conversation

@emilazy
Copy link
Member

@emilazy emilazy commented Apr 21, 2025

cc @reckenrode who had some reservations about the choice of apple-sdk over null here.

I have commits to make channel eval pass with null, but they’re a little ugly (e.g. changing packages that already don’t eval on Darwin, fixing stuff in KDE and Perl that doesn’t expect null build inputs) and since it broke a good handful of things in Nixpkgs, even ones that aren’t doing incorrect string interpolations, I figure it’d probably break external users too, and apple-sdk is strictly more correct in a sense (although sadly it becomes less strictly more correct in the presence of multiple SDK versions).

I’ve left in the truly benign/net‐positive clean‐ups I did on the road to making null work, though. I’m happy to change this to use null and restore the additional fix‐up commits if that’s preferred.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Apr 21, 2025
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Apr 21, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 21, 2025
@emilazy emilazy force-pushed the push-xuuomvvtkotq branch 2 times, most recently from e41cfd0 to 3dbddb5 Compare April 21, 2025 20:49
@emilazy emilazy force-pushed the push-xuuomvvtkotq branch from 3dbddb5 to 21356eb Compare April 21, 2025 20:58
@emilazy emilazy force-pushed the push-xuuomvvtkotq branch 4 times, most recently from 07ca4e8 to 310086a Compare April 21, 2025 21:20
@emilazy
Copy link
Member Author

emilazy commented Apr 21, 2025

Switched this to null instead, and added back all the changes I had to make for eval to pass when I was trying that out originally one by one, as if it’d be any different this time just because the things that break it are so silly. Oh well. I’m not sure if null is actually the ideal thing here, but apple-sdk is sort of wrong/weird in the multi‐version case, and libintl is precedent for doing it this way, so… here it is.

emilazy added 5 commits April 21, 2025 22:43
This was removed by upstream in
<renpy/renpy@f2d3e0d>.
This isn’t used by the X11 backend anyway, which is the only one
we build for macOS. Even though it has a native macOS frontend. But
I don’t want to spend more time on this.
Patch needs adjusting, build process is super messed up, package
is out of date and not used in Nixpkgs other than by another unused
library… not worth it.
@emilazy emilazy force-pushed the push-xuuomvvtkotq branch from 310086a to f0c9536 Compare April 21, 2025 21:43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? When the cc-wrapper hook processes the flags, it resolves environment variables?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that they get expanded inside env.*? Maybe with structured attributes you have to do it in a Bash phase instead? I could just recommend that instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it has to be done in Bash in one of the phases. Otherwise, I think you’ll get something like:

clang: warning: argument unused during compilation: '-L$SDKROOT/System/Library/Frameworks/OpenGL.framework/Versions/Current/Libraries' [-Wunused-command-line-argument]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amended appropriately. Though since only one package in the tree even does this, probably it wasn’t even worth documenting.

Copy link
Contributor

@reckenrode reckenrode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM.

Copy link
Contributor

@K900 K900 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Apr 23, 2025
@emilazy
Copy link
Member Author

emilazy commented Apr 23, 2025

Merging to unblock Darwin deprecations for the 25.11 deadline, but keeping the door open to changing null to something more compatible if necessary for #400739 concerns.

@emilazy emilazy merged commit 882f8af into NixOS:staging Apr 23, 2025
26 of 29 checks passed
@emilazy emilazy deleted the push-xuuomvvtkotq branch April 23, 2025 16:06
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: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants