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

Warn when building without nix daemon #7374

Closed
wants to merge 1 commit into from
Closed

Conversation

yshui
Copy link
Contributor

@yshui yshui commented Nov 30, 2022

We need root permission to drop supplementary groups, and if we don't do that, some builds can fail in user namespace, most notably go.

Related: #3245

We need root permission to drop supplementary groups, and if we don't do
that, some builds can fail in user namespace, most notably go.

Related: NixOS#3245
@fricklerhandwerk fricklerhandwerk added bug UX The way in which users interact with Nix. Higher level than UI. store Issues and pull requests concerning the Nix store labels Mar 3, 2023
@Ericson2314
Copy link
Member

Ericson2314 commented Mar 10, 2023

@yshui We discussed your PR in the Nix team meeting and did not come to a conclusion how to proceed. We probably don't want to flood users with warnings they can't do anything about, but we still have to review the full scope of the design considerations your PR reveals.

Complete discussion

Triaged in the Nix team meeting:

  • @Ericson2314: PR description perhaps sells it short. If the user enabled sandboxing but is only getting "semi-sandboxing", that deservers a warning.
  • Would be good to good to perhaps improve the error message or at least add more comments.
  • To discuss more thoroughly later

Discussed in the Nix team meeting:

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-03-10-nix-team-meeting-minutes-39/26279/1

@yshui
Copy link
Contributor Author

yshui commented Mar 18, 2023

don't want to spam non-daemon users with a warning

what about only print this warning if the build fails?

@yshui
Copy link
Contributor Author

yshui commented Mar 18, 2023

Or, add a way to detect non-daemon builds from nix scripts (is there already a way?), and print a warning from go nix script?

@thufschmitt
Copy link
Member

Or, add a way to detect non-daemon builds from nix scripts (is there already a way?), and print a warning from go nix script?

Yes, that sounds sensible since as far as I can tell, this essentially concerns go (also, maybe we could fix all the standard go builds like I did for one of them in NixOS/nixpkgs#144398). I assume we can just reuse the same logic that caused the test to fail to detect whether the sandbox has the right groups or not.

@fricklerhandwerk
Copy link
Contributor

Triaged in Nix team meeting:

@yshui feel free to open a new PR if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug store Issues and pull requests concerning the Nix store UX The way in which users interact with Nix. Higher level than UI.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants