Skip to content

Conversation

@booxter
Copy link
Contributor

@booxter booxter commented Apr 16, 2025

This patch covers all packages that use stub paths in some way in their
build instructions.

(Except those listed in this comment.)

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 16, 2025
@nix-owners nix-owners bot requested review from Profpatsch and natsukium April 16, 2025 02:03
@github-actions github-actions bot added 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must 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: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels Apr 16, 2025
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from cb54e4f to 5803db0 Compare April 16, 2025 02:10
@booxter booxter changed the base branch from master to staging April 16, 2025 02:10
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 5803db0 to 28e3371 Compare April 16, 2025 04:25
@booxter
Copy link
Contributor Author

booxter commented Apr 16, 2025

I've manually checked these packages on aarch64-darwin. Results are as follows:

vengi-tools: broken since 24.05: https://hydra.nixos.org/build/260818375
cardpeek: broken since 24.11: https://hydra.nixos.org/build/278953995
gildas: builds
nextpnr: broken in staging-next: https://hydra.nixos.org/build/294373417
odin: builds
opensc: builds
bullet: builds
bullet-roboschool: broken in 24.05: https://hydra.nixos.org/build/260816205
libsamplerate: builds
netcdffortran: builds
jaxlib: not utilized since the switch to -bin
pyscard: builds
bazel-test-cpp: passes
xorg.xorgserver: broken in staging-next: https://hydra.nixos.org/build/294644953 (dependency issue)

I've also checked for those already broken packages that failures are similar to what can be seen on hydra. So I hope this doesn't break anything that wasn't broken before.

OpenSC can be cleaned up because it detects the right path for PSCS if configure option is not passed:

opensc> PC/SC default provider:  /System/Library/Frameworks/PCSC.framework/PCSC
opensc> PKCS11 default provider: /nix/store/2mph7dm80lksszb10gf4mank6yp73zfp-opensc-0.26.1/lib/opensc-pkcs11.so
opensc> PKCS11 onepin provider:  /nix/store/2mph7dm80lksszb10gf4mank6yp73zfp-opensc-0.26.1/lib/onepin-opensc-pkcs11.so

bullet can be cleaned up more, need to remove patchPhase sections that tried to work around old SDK.

@booxter
Copy link
Contributor Author

booxter commented Apr 16, 2025

Not sure if this commit / PR should have treewide as its title tag. Please advise.

@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 28e3371 to 22bdec6 Compare April 16, 2025 13:04
@booxter booxter changed the title darwin: Remove use of apple_sdk framework stub paths treewide: Remove use of apple_sdk framework stub paths Apr 16, 2025
@NickCao
Copy link
Member

NickCao commented Apr 16, 2025

The PR title should be treewide, but the changes should be in individual commits for individual packages.

@booxter booxter marked this pull request as draft April 17, 2025 21:25
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch 4 times, most recently from 712c5a4 to 05b7ccf Compare April 18, 2025 01:51
@booxter booxter marked this pull request as ready for review April 18, 2025 01:51
@booxter
Copy link
Contributor Author

booxter commented Apr 18, 2025

I separated each package into a separate commit. I also split out more elaborate pieces out into individual PRs:

With these merged, we should have no packages using old apple_sdk stub paths in-tree.

@nix-owners nix-owners bot requested a review from Astavie April 18, 2025 01:59
@Astavie
Copy link
Contributor

Astavie commented Apr 18, 2025

How come this is being removed? I remember the odin package requiring these inputs to compile on macos, has there been a recent change so that is not required anymore?

@yzx9
Copy link
Contributor

yzx9 commented Apr 18, 2025

How come this is being removed? I remember the odin package requiring these inputs to compile on macos, has there been a recent change so that is not required anymore?

Yes, the Darwin SDK has been fully reworked in #346043. The old-style SDKs were just no-op placeholders now.

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Looks great on the whole, thanks! Some nits.

@smaret
Copy link
Member

smaret commented Apr 18, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 399040 --package gildas


aarch64-darwin

✅ 1 package built:
  • gildas

Copy link
Member

@smaret smaret left a comment

Choose a reason for hiding this comment

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

The change in gi/gildas/package.nix looks good to me. Thanks!

@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 18, 2025
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 05b7ccf to 4fe11b6 Compare April 18, 2025 23:42
@booxter booxter requested a review from emilazy April 18, 2025 23:43
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! I plan to do a nixpkgs-review and then merge within a day or two. Some nits if you feel like addressing them before then, but I won’t block on them.

booxter added 4 commits April 18, 2025 21:01
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 4fe11b6 to 10a73ad Compare April 19, 2025 01:02
booxter added 6 commits April 18, 2025 21:11
Also, patched out all impure paths in the linker.cpp file for darwin.

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
Note: jaxlib currently points to jaxlib-bin so this code is not utilized
for regular jaxlib installations.

Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
@booxter booxter force-pushed the remove-injection-of-old-darwin-framework-stubs branch from 10a73ad to d1ee8a7 Compare April 19, 2025 01:11
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Okay, when I said I’d run nixpkgs-review I hadn’t looked at the rebuilds total. But I built every directly touched package on x86_64-darwin except for xorg.xorgserver (didn’t want to build on staging and too lazy to test it on top of the libAppleWM change, plus it was broken before anyway), bullet-roboschool (broken on x86_64-darwin), and gildas (just takes too long). Thank you!

@emilazy emilazy merged commit bd79aca into NixOS:staging Apr 19, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 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.

7 participants