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

Make $HOME=/proc/homeless-shelter instead of /homeless-shelter #11300

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

noamraph
Copy link
Contributor

Motivation

When building, $HOME is currently set to /homeless-shelter. In some scenarios (specifically when using the linux sandbox with a single-user installation), it is possible to create this directory, and some tools will make it, resulting in a build error.

Instead, if $HOME is set to /proc/homeless-shelter, it is never possible to create it, since /proc is a virtual filesystem which doesn't allow creating files or directories. This resolves the issue mentioned.

Context

#11295 - nix build fails because /homeless-shelter exists.
#8313 - a suggestion to do what this PR does.

I prefer to put /homeless-shelter in /proc rather than /sys, since /proc is standard in linux, and /sys seems less standard. See https://tldp.org/LDP/Linux-Filesystem-Hierarchy/html/c23.html, of the Linux Documentation Project, which describes /proc and doesn't describe /sys.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This makes it so even root can't create $HOME, for example by running `mkdir -p $HOME/.cache/foo`.
@@ -102,7 +102,7 @@ void handleDiffHook(
}
}

const Path LocalDerivationGoal::homeDir = "/homeless-shelter";
const Path LocalDerivationGoal::homeDir = "/proc/homeless-shelter";
Copy link
Member

Choose a reason for hiding this comment

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

/proc doesn't exist on macOS, maybe doesn't exist on any of the *BSDs we have (fledgling?) support for, and probably won't exist on Windows (which has seen some development recently).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that changing the path will cause new trouble - it would just mean that a build script which does mkdir -p $HOME/.local/cache (for example) will create /proc/homeless-shelter instead of /homeless-shelter. But would it help if I add a condition that will set it to /proc/homeless-shelter only on Linux?

@roberth roberth added the derivation-build The process of building an individual derivation (see also sandbox label) label Aug 15, 2024
@noamraph
Copy link
Contributor Author

@Mic92 I got an email with a review from you, saying that MacOS has /dev. I don't see it in the PR page. Then, how about using /proc/homeless-shelter for linux and /dev/homeless-shelter for Darwin? From the code it seems that those are the 2 options.

@Mic92
Copy link
Member

Mic92 commented Aug 15, 2024

I deleted it again because we can actually just keep /homeless-shelter on Mac because it's not writable at any point. However my other thought was, what if /homeless-shelter exits but is bind mount on itself read-only? Or why in single-user mode are not doing this for /?

@tomberek
Copy link
Contributor

From Nix Team: Approve the idea. Still need to clarify the OSX case (or keep it where it is for now).

@tomberek tomberek added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Aug 19, 2024
@tomberek tomberek self-assigned this Aug 19, 2024
@noamraph
Copy link
Contributor Author

Thanks @tomberek ! I added a commit that sets $HOME to /proc/homeless-shelter on Linux and keeps it /homeless-shelter on OSX. WDYT?

@tomberek
Copy link
Contributor

Probably good to test and look around in Nixpkgs for anything that might depend on this. Please keep an eye out for issues, upgrade any systems you have to use it, and so forth.

@tomberek tomberek merged commit 43e82c9 into NixOS:master Aug 20, 2024
11 checks passed
@grahamc
Copy link
Member

grahamc commented Aug 20, 2024 via email

@Mic92
Copy link
Member

Mic92 commented Aug 21, 2024

@noamraph if you got time, could you also make a changelog entry for this change?

@Mic92
Copy link
Member

Mic92 commented Aug 21, 2024

If we’re changing the path, maybe it’d be a good time to not call it a homeless shelter? And instead give it a name that helps the user identify the problem.

Do you have a suggestions in mind? /no-such-path?

@noamraph
Copy link
Contributor Author

@noamraph if you got time, could you also make a changelog entry for this change?

Yes, I hope to do it today.

If we’re changing the path, maybe it’d be a good time to not call it a homeless shelter? And instead give it a name that helps the user identify the problem.

Do you have a suggestions in mind? /no-such-path?

Maybe /homedir-that-should-not-exist ?

I usually don't like long names like that, but I guess that it would usually only appear when there's a problem related to this.

@noamraph
Copy link
Contributor Author

@Mic92 I created a PR with a changelog file: #11350

@grahamc
Copy link
Member

grahamc commented Aug 21, 2024

How about home-is-immutable?

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-08-19-nix-team-meeting-minutes-170/50942/1

noamraph pushed a commit to noamraph/nix that referenced this pull request Aug 22, 2024
Make $HOME=/proc/homeless-shelter instead of /homeless-shelter

(cherry picked from commit 43e82c9)
@winterqt
Copy link
Member

This is already causing regressions in Nixpkgs (see above). I suggest reverting before this gets to a release, please.

@grahamc
Copy link
Member

grahamc commented Aug 24, 2024 via email

@puckipedia
Copy link
Contributor

Note that there are already loads of examples of overwriting HOME in packages that need a writable home, search for ‘export HOME=‘ and you’ll find plenty of examples.

Correct, and, more interestingly, /homeless-shelter already cannot be mkdir'd on sandboxed builds, or unsandboxed builds where the build user isn't root (as they won't have permission to write to /), making me wonder why #11295 sees the behavior it does. thankfullySadly, I do not have a Windows machine to test ubuntu-on-WSL2 on.

Since nix can be used with and without sandboxing, I feel it is typically expected to make efforts to function correctly in both. You can see examples of this very problem here: #11295

I’m not comfortable calling package building problems originating from this change a regression, personally, since it was already causing more sinister and harder to identify / correct issues.

I think that error: home directory '/homeless-shelter' exists; please remove it to assure purity of builds without sandboxing is a pretty clear to identify issue, with a very clear method of correcting it, actually, and I think most people would agree. I'm way more interested in why a Nix build, of all things would fail a test with that. That looks, to me, like a completely different regression than the one described in this PR, which, I suspect, papers over that bug instead; and how everyone else missed this fact, instead opting to break an unknown amount of in-the-wild derivations.

Anyhow, I debugged the issue that rustic hit for y'all, and it shows, to me, that this PR is flawed, and it's not the only derivation to have this issue.

When cargo tries to lock the package cache, it tries to create a file in $HOME. If this fails with EPERM or EROFS, it assumes this is because the home directory is read only, which is what it should be. However, mkdir /proc/meow returns ENOENT, which is considered a failing error code, and makes cargo exit. This means that, in effect, any cargo build done in nixpkgs' history is now unbuildable with modern Nix. It's pretty clear that this is an unacceptable regression. We have people building software from 2008 with modern versions of Nix, and minimal changes. Why would building rust code from like a week back require a "pre-2.25" version of Nix (or Lix, of course)?

@noamraph
Copy link
Contributor Author

FWIW, given @puckipedia 's analysis that mkdir $HOME produces a different error code, I am for reverting my PR and looking for a different solution.

Mic92 added a commit to Mic92/nix-1 that referenced this pull request Aug 25, 2024
…-to-proc"

This reverts commit 43e82c9, reversing
changes made to d79b9bd.

Since /proc/homeless-shelter returns a different ERNO than /homeless-shelter (ENOENT vs EACCES), we need to revert this change.
Software depends on this error code i.e. cargo and therefore breaks.
Mic92 added a commit to Mic92/nix-1 that referenced this pull request Aug 25, 2024
…-to-proc"

This reverts commit 43e82c9, reversing
changes made to d79b9bd.

Since /proc/homeless-shelter returns a different errno than /homeless-shelter (ENOENT vs EACCES), we need to revert this change.
Software depends on this error code i.e. cargo and therefore breaks.
@Mic92
Copy link
Member

Mic92 commented Aug 25, 2024

Revert in #11366

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
derivation-build The process of building an individual derivation (see also sandbox label) documentation idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants