Skip to content

Revert "stdenv: set NIX_DONT_SET_RPATH_FOR_TARGET on Darwin"#355667

Merged
emilazy merged 2 commits intoNixOS:stagingfrom
reckenrode:push-qwlsorymprko
Nov 17, 2024
Merged

Revert "stdenv: set NIX_DONT_SET_RPATH_FOR_TARGET on Darwin"#355667
emilazy merged 2 commits intoNixOS:stagingfrom
reckenrode:push-qwlsorymprko

Conversation

@reckenrode
Copy link
Contributor

@reckenrode reckenrode commented Nov 13, 2024 •
edited
Loading

This reverts commit 10c87ee.

Fixes #355622.

@emilazy @sternenseemann

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.

@github-actions github-actions bot added 6.topic: stdenv Standard environment and removed backport staging-24.11 labels Nov 13, 2024
@reckenrode
Copy link
Contributor Author

reckenrode commented Nov 13, 2024

Before the revert:

$ nix-instantiate . -A pkgsCross.gnu64.pkgsBuildBuild.jq
/nix/store/idydjykv86f3wzy87jrx0pf8rzf08hmq-jq-1.7.1.drv!bin
$ nix-instantiate . -A pkgsCross.gnu64.pkgsBuildHost.jq
/nix/store/ay9bsvb6vslgi0xqmvmyfwczmnyzkqv3-jq-1.7.1.drv!bin

After the revert:

$  nix-instantiate . -A pkgsCross.gnu64.pkgsBuildBuild.jq
/nix/store/p3b6czc6y9ba6la6kf8j9bx6rpm3gcw4-jq-1.7.1.drv!bin
$ nix-instantiate . -A pkgsCross.gnu64.pkgsBuildHost.jq
/nix/store/p3b6czc6y9ba6la6kf8j9bx6rpm3gcw4-jq-1.7.1.drv!bin

@reckenrode
Copy link
Contributor Author

reckenrode commented Nov 13, 2024 •
edited
Loading

Technically this is breaking because it changes the semantics of Darwin as a target platform, but not reverting on 24.11 would keep this major regression for cross-compilation on stable until 25.05.

@RossComputerGuy Thoughts?

@reckenrode reckenrode mentioned this pull request Nov 13, 2024
Open
@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 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: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 13, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 14, 2024
emilazy
emilazy reviewed Nov 14, 2024
View reviewed changes
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 good in principle but can we maybe update the comment here to be more useful about our current motivation to not uncomment it, or at least add some detail in the commit message?

@szlend
Copy link
Member

szlend commented Nov 16, 2024 •
edited
Loading

Is it still possible to get this in time for 24.11? It's a pretty big regression for cross-compilation. My understanding is it just reverts to 24.05 rpath behavior.

@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

Not in time for the initial release (unless we delay it, which seems unlikely for cross), but given the release manger has approved it, it will go in the first staging-24.11 backport cycle.

@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

Would still like to see the comment and/or commit message updated, but otherwise will merge within a day or two.

@reckenrode
Copy link
Contributor Author

reckenrode commented Nov 16, 2024

I pushed an additional commit clarifying the nature of the mass rebuilds.

@emilazy emilazy merged commit fe7cd60 into NixOS:staging Nov 17, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2024

Successfully created backport PR for staging-24.11:

@github-actions github-actions bot mentioned this pull request Nov 17, 2024
Merged
1 task
@reckenrode reckenrode deleted the push-qwlsorymprko branch November 19, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emilazy emilazy emilazy approved these changes

@RossComputerGuy RossComputerGuy RossComputerGuy approved these changes

@Artturin Artturin Awaiting requested review from Artturin

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@philiptaron philiptaron Awaiting requested review from philiptaron

+1 more reviewer

@szlend szlend szlend approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment 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-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@reckenrode @szlend @emilazy @RossComputerGuy @wegank