From a5919f4754be6f4a9fe091e0ee5538ad85af0050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 13 Apr 2022 10:26:50 +0200 Subject: [PATCH 1/5] =?UTF-8?q?Move=20the=20default=20profiles=20to=20the?= =?UTF-8?q?=20user=E2=80=99s=20home?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than using `/nix/var/nix/{profiles,gcroots}/per-user/`, put the user profiles and gcroots under `$XDG_DATA_DIR/nix/{profiles,gcroots}`. This means that the daemon no longer needs to manage these paths itself (they are fully handled client-side). In particular, it doesn’t have to `chown` them anymore (removing one need for root). This does change the layout of the gc-roots created by nix-env, and is likely to break some stuff, so I’m not sure how to properly handle that. --- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/local-store.cc | 16 ------------- src/libstore/local-store.hh | 2 -- src/libstore/profiles.cc | 26 ++++++++++++++++----- src/libstore/profiles.hh | 4 ++++ src/libstore/store-api.hh | 3 --- src/libutil/util.cc | 18 ++++++++------ src/libutil/util.hh | 3 +++ src/nix-channel/nix-channel.cc | 4 +++- src/nix/daemon.cc | 1 - tests/remote-store.sh | 4 ---- 11 files changed, 42 insertions(+), 41 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 9ab9b17fe..4fec8fb88 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1517,7 +1517,7 @@ void LocalDerivationGoal::startDaemon() try { daemon::processConnection(store, from, to, daemon::NotTrusted, daemon::Recursive, - [&](Store & store) { store.createUser("nobody", 65535); }); + [&](Store & store) {}); debug("terminated daemon connection"); } catch (SysError &) { ignoreException(); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index be21e3ca0..82edaa9bf 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -201,8 +201,6 @@ LocalStore::LocalStore(const Params & params) throw SysError("could not set permissions on '%s' to 755", perUserDir); } - createUser(getUserName(), getuid()); - /* Optionally, create directories and set permissions for a multi-user install. */ if (getuid() == 0 && settings.buildUsersGroup != "") { @@ -1824,20 +1822,6 @@ void LocalStore::signPathInfo(ValidPathInfo & info) } -void LocalStore::createUser(const std::string & userName, uid_t userId) -{ - for (auto & dir : { - fmt("%s/profiles/per-user/%s", stateDir, userName), - fmt("%s/gcroots/per-user/%s", stateDir, userName) - }) { - createDirs(dir); - if (chmod(dir.c_str(), 0755) == -1) - throw SysError("changing permissions of directory '%s'", dir); - if (chown(dir.c_str(), userId, getgid()) == -1) - throw SysError("changing owner of directory '%s'", dir); - } -} - std::optional> LocalStore::queryRealisationCore_( LocalStore::State & state, const DrvOutput & id) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 06d36a7d5..a84eb7c26 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -281,8 +281,6 @@ private: void signPathInfo(ValidPathInfo & info); void signRealisation(Realisation &); - void createUser(const std::string & userName, uid_t userId) override; - // XXX: Make a generic `Store` method FixedOutputHash hashCAPath( const FileIngestionMethod & method, diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index 3e4188188..29ce13b8d 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -280,16 +280,30 @@ std::string optimisticLockProfile(const Path & profile) } +Path profilesDir() +{ + auto profileRoot = getDataDir() + "/nix/profiles"; + createDirs(profileRoot); + return profileRoot; +} + + Path getDefaultProfile() { Path profileLink = getHome() + "/.nix-profile"; try { - if (!pathExists(profileLink)) { - replaceSymlink( - getuid() == 0 - ? settings.nixStateDir + "/profiles/default" - : fmt("%s/profiles/per-user/%s/profile", settings.nixStateDir, getUserName()), - profileLink); + // Migrate from the “old-style” profiles stored under `/nix/var`: + // If the link exists and points to the old location, rewrite it to the + // new one (otherwise keep-it as-it-is as it might have been + // intentionnally changed, in which case we shouldn’t touch it) + auto legacyProfile = getuid() == 0 + ? settings.nixStateDir + "/profiles/default" + : fmt("%s/profiles/per-user/%s/profile", settings.nixStateDir, getUserName()); + if (!pathExists(profileLink) || + (isLink(profileLink) && + readLink(profileLink) == legacyProfile) + ) { + replaceSymlink(profilesDir() + "/profile", profileLink); } return absPath(readLink(profileLink), dirOf(profileLink)); } catch (Error &) { diff --git a/src/libstore/profiles.hh b/src/libstore/profiles.hh index 408ca039c..73667a798 100644 --- a/src/libstore/profiles.hh +++ b/src/libstore/profiles.hh @@ -68,6 +68,10 @@ void lockProfile(PathLocks & lock, const Path & profile); rebuilt. */ std::string optimisticLockProfile(const Path & profile); +/* Creates and returns the path to a directory suitable for storing the user’s + profiles. */ +Path profilesDir(); + /* Resolve ~/.nix-profile. If ~/.nix-profile doesn't exist yet, create it. */ Path getDefaultProfile(); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 9eab4b4e5..5807392a7 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -657,9 +657,6 @@ public: return toRealPath(printStorePath(storePath)); } - virtual void createUser(const std::string & userName, uid_t userId) - { } - /* * Synchronises the options of the client with those of the daemon * (a no-op when there’s no daemon) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 993dc1cb6..40faa4bf2 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -537,6 +537,16 @@ std::string getUserName() return name; } +Path getHomeOf(uid_t userId) +{ + std::vector buf(16384); + struct passwd pwbuf; + struct passwd * pw; + if (getpwuid_r(userId, &pwbuf, buf.data(), buf.size(), &pw) != 0 + || !pw || !pw->pw_dir || !pw->pw_dir[0]) + throw Error("cannot determine user's home directory"); + return pw->pw_dir; +} Path getHome() { @@ -558,13 +568,7 @@ Path getHome() } } if (!homeDir) { - std::vector buf(16384); - struct passwd pwbuf; - struct passwd * pw; - if (getpwuid_r(geteuid(), &pwbuf, buf.data(), buf.size(), &pw) != 0 - || !pw || !pw->pw_dir || !pw->pw_dir[0]) - throw Error("cannot determine user's home directory"); - homeDir = pw->pw_dir; + homeDir = getHomeOf(geteuid()); if (unownedUserHomeDir.has_value() && unownedUserHomeDir != homeDir) { warn("$HOME ('%s') is not owned by you, falling back to the one defined in the 'passwd' file ('%s')", *unownedUserHomeDir, *homeDir); } diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 9b149de80..266da0ae3 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -137,6 +137,9 @@ void deletePath(const Path & path, uint64_t & bytesFreed); std::string getUserName(); +/* Return the given user's home directory from /etc/passwd. */ +Path getHomeOf(uid_t userId); + /* Return $HOME or the user's home directory from /etc/passwd. */ Path getHome(); diff --git a/src/nix-channel/nix-channel.cc b/src/nix-channel/nix-channel.cc index cf52b03b4..263d85eea 100755 --- a/src/nix-channel/nix-channel.cc +++ b/src/nix-channel/nix-channel.cc @@ -1,9 +1,11 @@ +#include "profiles.hh" #include "shared.hh" #include "globals.hh" #include "filetransfer.hh" #include "store-api.hh" #include "legacy.hh" #include "fetchers.hh" +#include "util.hh" #include #include @@ -166,7 +168,7 @@ static int main_nix_channel(int argc, char ** argv) nixDefExpr = home + "/.nix-defexpr"; // Figure out the name of the channels profile. - profile = fmt("%s/profiles/per-user/%s/channels", settings.nixStateDir, getUserName()); + profile = profilesDir() + "/channels"; enum { cNone, diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index c527fdb0a..19fbbf155 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -248,7 +248,6 @@ static void daemonLoop() querySetting("build-users-group", "") == "") throw Error("if you run 'nix-daemon' as root, then you MUST set 'build-users-group'!"); #endif - store.createUser(user, peer.uid); }); exit(0); diff --git a/tests/remote-store.sh b/tests/remote-store.sh index 31210ab47..1ae126794 100644 --- a/tests/remote-store.sh +++ b/tests/remote-store.sh @@ -30,7 +30,3 @@ NIX_REMOTE= nix-store --dump-db > $TEST_ROOT/d2 cmp $TEST_ROOT/d1 $TEST_ROOT/d2 killDaemon - -user=$(whoami) -[ -e $NIX_STATE_DIR/gcroots/per-user/$user ] -[ -e $NIX_STATE_DIR/profiles/per-user/$user ] From 0601050755ba39f52fdd2a0fc0478baaf9b1c7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 13 Apr 2022 10:26:50 +0200 Subject: [PATCH 2/5] Migrate the old profiles to the new location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure that we don’t just create the new profiles directory, but that we also migrate every existing profile to it. --- src/libstore/profiles.cc | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index 29ce13b8d..d185a898c 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -293,17 +293,29 @@ Path getDefaultProfile() Path profileLink = getHome() + "/.nix-profile"; try { // Migrate from the “old-style” profiles stored under `/nix/var`: - // If the link exists and points to the old location, rewrite it to the - // new one (otherwise keep-it as-it-is as it might have been - // intentionnally changed, in which case we shouldn’t touch it) + // If the link exists and points to the old location, then: + // - Rewrite it to point to the new location + // - For every generation of the old default profile, create a symlink + // from the new directory to it (so that all the previous profiles + // and generations are still available). auto legacyProfile = getuid() == 0 ? settings.nixStateDir + "/profiles/default" : fmt("%s/profiles/per-user/%s/profile", settings.nixStateDir, getUserName()); - if (!pathExists(profileLink) || - (isLink(profileLink) && - readLink(profileLink) == legacyProfile) - ) { - replaceSymlink(profilesDir() + "/profile", profileLink); + auto newProfile = profilesDir() + "/profile"; + if (!pathExists(profileLink) + || (isLink(profileLink) + && readLink(profileLink) == legacyProfile)) { + warn("Migrating the default profile"); + replaceSymlink(newProfile, profileLink); + replaceSymlink(legacyProfile, newProfile); + if (pathExists(legacyProfile)) { + for (auto & oldGen : findGenerations(legacyProfile).first) { + replaceSymlink( + oldGen.path, + dirOf(newProfile) + "/" + + std::string(baseNameOf(oldGen.path))); + } + } } return absPath(readLink(profileLink), dirOf(profileLink)); } catch (Error &) { From 1f02aa4098a61e60062e3ecdb713810e24efa25a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 13 Apr 2022 10:26:50 +0200 Subject: [PATCH 3/5] Test the migration of the user profiles --- tests/common.sh.in | 2 +- tests/local.mk | 1 + tests/user-envs-migration.sh | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/user-envs-migration.sh diff --git a/tests/common.sh.in b/tests/common.sh.in index 73c2d2309..74bbbc8ca 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -62,7 +62,7 @@ readLink() { } clearProfiles() { - profiles="$NIX_STATE_DIR"/profiles + profiles="$HOME"/.local/share/nix/profiles rm -rf $profiles } diff --git a/tests/local.mk b/tests/local.mk index 5ac1ede32..b900e1534 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -17,6 +17,7 @@ nix_tests = \ fetchMercurial.sh \ gc-auto.sh \ user-envs.sh \ + user-envs-migration.sh \ binary-cache.sh \ multiple-outputs.sh \ ca/build.sh \ diff --git a/tests/user-envs-migration.sh b/tests/user-envs-migration.sh new file mode 100644 index 000000000..467c28fbb --- /dev/null +++ b/tests/user-envs-migration.sh @@ -0,0 +1,35 @@ +# Test that the migration of user environments +# (https://github.com/NixOS/nix/pull/5226) does preserve everything + +source common.sh + +if isDaemonNewer "2.4pre20211005"; then + exit 99 +fi + + +killDaemon +unset NIX_REMOTE + +clearStore +clearProfiles +rm -rf ~/.nix-profile + +# Fill the environment using the older Nix +PATH_WITH_NEW_NIX="$PATH" +export PATH="$NIX_DAEMON_PACKAGE/bin:$PATH" + +nix-env -f user-envs.nix -i foo-1.0 +nix-env -f user-envs.nix -i bar-0.1 + +# Migrate to the new profile dir, and ensure that everything’s there +export PATH="$PATH_WITH_NEW_NIX" +nix-env -q # Trigger the migration +( [[ -L ~/.nix-profile ]] && \ + [[ $(readlink ~/.nix-profile) == ~/.local/share/nix/profiles/profile ]] ) || \ + fail "The nix profile should point to the new location" + +(nix-env -q | grep foo && nix-env -q | grep bar && \ + [[ -e ~/.nix-profile/bin/foo ]] && \ + [[ $(nix-env --list-generations | wc -l) == 2 ]]) || + fail "The nix profile should have the same content as before the migration" From c80621dbacc501f8bf43863438a3df087436d13e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 17 Jan 2023 14:15:00 +0100 Subject: [PATCH 4/5] Don't try to migrate existing profiles Doing so would be more dangerous than useful, better leave them as-is if they already exist --- src/libstore/profiles.cc | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index d185a898c..ff62d0972 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -292,30 +292,9 @@ Path getDefaultProfile() { Path profileLink = getHome() + "/.nix-profile"; try { - // Migrate from the “old-style” profiles stored under `/nix/var`: - // If the link exists and points to the old location, then: - // - Rewrite it to point to the new location - // - For every generation of the old default profile, create a symlink - // from the new directory to it (so that all the previous profiles - // and generations are still available). - auto legacyProfile = getuid() == 0 - ? settings.nixStateDir + "/profiles/default" - : fmt("%s/profiles/per-user/%s/profile", settings.nixStateDir, getUserName()); - auto newProfile = profilesDir() + "/profile"; - if (!pathExists(profileLink) - || (isLink(profileLink) - && readLink(profileLink) == legacyProfile)) { - warn("Migrating the default profile"); - replaceSymlink(newProfile, profileLink); - replaceSymlink(legacyProfile, newProfile); - if (pathExists(legacyProfile)) { - for (auto & oldGen : findGenerations(legacyProfile).first) { - replaceSymlink( - oldGen.path, - dirOf(newProfile) + "/" - + std::string(baseNameOf(oldGen.path))); - } - } + auto profile = profilesDir() + "/profile"; + if (!pathExists(profileLink)) { + replaceSymlink(profile, profileLink); } return absPath(readLink(profileLink), dirOf(profileLink)); } catch (Error &) { From 6bdf4edb77bea3fdc006e9b8131a43f4dcd0ee42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 17 Jan 2023 14:15:53 +0100 Subject: [PATCH 5/5] Keep the default profile the same MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's used as the “system” profile in a bunch of places, so better not touch it. Besides, it doesn't hurt to keep it since it's owned by root any way, so it doesn't have the `chown` problem that the user profiles had and that led to wanting to move them on the client-side. --- src/libstore/profiles.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index ff62d0972..b202351ce 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -292,7 +292,10 @@ Path getDefaultProfile() { Path profileLink = getHome() + "/.nix-profile"; try { - auto profile = profilesDir() + "/profile"; + auto profile = + getuid() == 0 + ? settings.nixStateDir + "/profiles/default" + : profilesDir() + "/profile"; if (!pathExists(profileLink)) { replaceSymlink(profile, profileLink); }