Don't replace /etc in fhs-userenv #80457
Conversation
|
/marvin opt-in |
|
Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here. |
|
/status needs_reviewer |
|
Bot mistake. |
|
Setting @abbradar as a reviewer for now |
marvin-mk2
bot
added
awaiting_reviewer
needs_reviewer
and removed
needs_reviewer
awaiting_reviewer
labels
marvin-mk2
bot
added
needs_reviewer
awaiting_reviewer
and removed
needs_reviewer
awaiting_reviewer
labels
| @@ -68,60 +68,10 @@ let | |||
| ${profile} | |||
| ''; | |||
|
|
|||
| # Compose /etc for the chroot environment | |||
| etcPkg = stdenv.mkDerivation { | |||
There was a problem hiding this comment.
The only reason I can think of is to provide /etc/ld.so.conf but this is not provided here either.
There was a problem hiding this comment.
cc @vcunat @matthewbauer Any idea why this was added in the first place? Is there some impurity introduced if we keep the original /etc?
There was a problem hiding this comment.
Is this maybe needed on non-nixos distributions because /etc/ld.so.conf could mess with the build?
There was a problem hiding this comment.
My guess of the intention: link as little as possible, thus reducing impurity brought from the host OS. EDIT: say, why should config of most OS services be visible in there?
marvin-mk2
bot
added
awaiting_reviewer
needs_reviewer
and removed
needs_reviewer
awaiting_reviewer
labels
|
@glittershark please review. |
|
not really familiar enough with the original reasoning here to confidently review. |
|
I guess at least this behaviour should be made optional. I'd be happy to work on this, but perhaps I should wait until a decision has been reached about #55973. Closing for now. |
Motivation for this change
buildFHSUserEnv currently replaces /etc, selectively symlinking over some files from the host /etc into the chroot.
This means some things will be missing - which is inconvenient when using buildFHSUserEnv to spin up an interactive nix-shell.
This PR is not yet for merging at this point, but for testing and discussing whether doing this (either by default or somehow as an option) makes sense at all.
It seems to work for me ;)
/cc @abbradar looks like you have a lot of background about this part of the system ;)