-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Set up user namespace in builder, not in parent #8023
base: master
Are you sure you want to change the base?
Conversation
CC @yorickvP Since you are currently refactoring this code, and might have some opinions on this sort of thing. |
511b1d0
to
0e2e212
Compare
0e2e212
to
3f95f76
Compare
3f95f76
to
e64b80e
Compare
Hmm I wonder if we have a different macOS spurious error with the non-blocking GC besides the EOF one. |
e64b80e
to
88265e3
Compare
This is $ nix build .\#hydraJobs.installerTests.ubuntu-22-04.x86_64-linux.install-force-daemon -L
...
installer-test-ubuntu> ~~> Setting up the default profile
installer-test-ubuntu> installing 'nix-2.15.0pre20230320_88265e3'
installer-test-ubuntu> error:
installer-test-ubuntu> … while setting up the build environment
installer-test-ubuntu> … writing file '/proc/self/uid_map'
installer-test-ubuntu> error: writing to file: Operation not permitted |
88265e3
to
8c4b9af
Compare
8c4b9af
to
08785f1
Compare
This is a minor simplification; there's now slightly less handshaking required between the builder and the parent, and we create one less pipe when starting the builder. As a niche benefit, this also replaces our access to `/proc/builder_pid` in the parent with an access to `/proc/self` in the builder. The former access could fail if `/proc` was mounted in a different pid namespace from the one we are running in; now we will work even if there is such a misconfiguration. ---- Rebaser's (@Ericson2314's) notes: A good questions why it was ever done the old way, since this is simpler and easier to write. The original commit doing this was c68e591, adding user namespae support for the first time, and referencing issue NixOS#625. But neither mention this design decision. In NixOS#2717 (comment) Eelco said: > I don't remember why it's done this way. Maybe the kernel required it > at the time? That seems to me like a likely explanation. User namespaces were famously fiddly for many years.
08785f1
to
90a4752
Compare
Motivation
This is a rebase of #2717. The following is @catern's original PR description
This is a minor simplification; there's now slightly less handshaking required between the builder and the parent, and we create one less pipe when starting the builder.
As a niche benefit, this also replaces our access to
/proc/builder_pid
in the parent with an access to/proc/self
in the builder. The former access could fail if/proc
was mounted in a different pid namespace from the one we are running in; now we will work even if there is such a misconfiguration.Context
Rebaser's (@Ericson2314's) notes:
A good questions why it was ever done the old way, since this is simpler and easier to write. The original commit doing this was c68e591, adding user namespace support for the first time, and referencing issue #625. But neither mention this design decision.
In #2717 (comment) @edolstra said:
That seems to me like a likely explanation. User namespaces were famously fiddly for many years.
Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.sh
src/*/tests
tests/nixos/*
Priorities
Add 👍 to pull requests you find important.