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.

Sign up for GitHub

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

Jump to bottom

fish: fix build with Darwin sandbox enabled #234119

Merged
merged 1 commit into from
Jun 9, 2023
Merged

fish: fix build with Darwin sandbox enabled #234119

merged 1 commit into from
Jun 9, 2023

Conversation

reckenrode
Copy link
Contributor

Description of changes

fish’s test suite has numerous failures when run on a Darwin machine with the sandbox enabled. The sandbox profile allows the test suite to run successfully.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label May 26, 2023
@ofborg ofborg bot requested review from cole-h, rapenne-s and winterqt May 26, 2023 02:59
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 26, 2023
winterqt
winterqt approved these changes May 26, 2023
View reviewed changes
Copy link
Member

@winterqt winterqt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I didn't know sandboxProfile worked with sandbox = true (as opposed to relaxed)!

@reckenrode
Copy link
Contributor Author

I didn't know sandboxProfile worked with sandbox = true (as opposed to relaxed)!

I don’ think it does. I’m using sandbox = relaxed. I guess I’ve gotten lazy about updating the message to reflect that I’m using relaxed instead of true. 😅

@winterqt
Copy link
Member

I said that because it worked fine with my configuration, where it is true... odd.

@reckenrode
Copy link
Contributor Author

I said that because it worked fine with my configuration, where it is true... odd.

Huh. That is odd.

This was referenced May 26, 2023
Closed
Closed
@mweinelt mweinelt added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Jun 2, 2023
cole-h
cole-h approved these changes Jun 9, 2023
View reviewed changes
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I also tested with sandbox = true (before and after this commit) and it does indeed fix it. (Built natively before and after on aarch64-darwin, but only after through Rosetta for x86_64-darwin.)

Maybe sandboxProfile is always intended to be allowed 🤷

@cole-h cole-h merged commit dad6320 into NixOS:master Jun 9, 2023
@reckenrode reckenrode deleted the fish-darwin-sandbox branch June 11, 2024 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winterqt winterqt winterqt approved these changes

@cole-h cole-h cole-h approved these changes

@rapenne-s rapenne-s Awaiting requested review from rapenne-s

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 11-100 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 reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@reckenrode @winterqt @cole-h @mweinelt