From a3979e67f491a3a6a29b119af61ef75d5793c699 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 7 Sep 2021 16:49:59 +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 581d63d0e..42c808979 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1394,7 +1394,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 1ee71b1c0..738945647 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -185,8 +185,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 != "") { @@ -1786,20 +1784,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 70d225be3..f89b64860 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -275,8 +275,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 7bd21519c..155cd3b68 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -630,9 +630,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 b833038a9..4da1379cc 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -529,6 +529,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() { @@ -536,13 +546,7 @@ Path getHome() { auto homeDir = getEnv("HOME"); 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()); } return *homeDir; }(); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 20591952d..9d84d2d45 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -130,6 +130,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 940923d3b..539fe6dd5 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 be28cb9262ce1125752eafa90c0e07f0f602d5f3 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 5 Oct 2021 17:40:03 +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 | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index 29ce13b8d..d695cf795 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -293,17 +293,24 @@ 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)) { + replaceSymlink(newProfile, profileLink); + for (auto & oldGen : findGenerations(legacyProfile).first) { + replaceSymlink( + oldGen.path, + newProfile + "/" + std::string(baseNameOf(oldGen.path))); + } } return absPath(readLink(profileLink), dirOf(profileLink)); } catch (Error &) { From a2bcf35e0dbf96f7bbc73b878f5151d9d74f1288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Mon, 11 Apr 2022 10:20:36 +0200 Subject: [PATCH 3/5] Properly migrate the existing profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure that the default profile (including all its generations) are still available after we move it to the user’s home directory --- src/libstore/profiles.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index d695cf795..d185a898c 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -305,11 +305,16 @@ Path getDefaultProfile() if (!pathExists(profileLink) || (isLink(profileLink) && readLink(profileLink) == legacyProfile)) { + warn("Migrating the default profile"); replaceSymlink(newProfile, profileLink); - for (auto & oldGen : findGenerations(legacyProfile).first) { - replaceSymlink( - oldGen.path, - newProfile + "/" + std::string(baseNameOf(oldGen.path))); + 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)); From 303abee699886bc0438d8cc5ddcc580041bc0fb9 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 6 Oct 2021 10:31:09 +0200 Subject: [PATCH 4/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 8ce28d318..6888b4856 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -60,7 +60,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 8032fc38a..67f67e653 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -7,6 +7,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..ddfd12f58 --- /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 -r ~/.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 a3c17cfc73c8fcd467f6701e40b3658d7bc71cea Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 14 Oct 2021 15:43:22 +0200 Subject: [PATCH 5/5] Harden the user-envs-migration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure that an absent `.nix-profile` at the begining doesn’t crash it --- tests/user-envs-migration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/user-envs-migration.sh b/tests/user-envs-migration.sh index ddfd12f58..467c28fbb 100644 --- a/tests/user-envs-migration.sh +++ b/tests/user-envs-migration.sh @@ -13,7 +13,7 @@ unset NIX_REMOTE clearStore clearProfiles -rm -r ~/.nix-profile +rm -rf ~/.nix-profile # Fill the environment using the older Nix PATH_WITH_NEW_NIX="$PATH"