From 3113b13df9afaab918792c725742c6bc3fcec88b Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 8 Feb 2023 20:03:57 +0100 Subject: [PATCH 1/5] buildenv: throw BuildEnvFileConflictError with more context At the moment an Error is thrown that only holds an error message regarding `nix-env` and `nix profile`. These tools make use of builtins.buildEnv, but buildEnv is also used in other places. These places are unrelated to Nix profiles, so the error shouldn't mention these tools. This generic error is now BuildEnvFileConflictError, which holds more contextual information about the files that were conflicting while building the environment. --- src/libstore/builtins/buildenv.cc | 12 +++++------- src/libstore/builtins/buildenv.hh | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index b1fbda13d..7bba33fb9 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -92,13 +92,11 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, if (S_ISLNK(dstSt.st_mode)) { auto prevPriority = state.priorities[dstFile]; if (prevPriority == priority) - throw Error( - "files '%1%' and '%2%' have the same priority %3%; " - "use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' " - "or type 'nix profile install --help' if using 'nix profile' to find out how " - "to change the priority of one of the conflicting packages" - " (0 being the highest priority)", - srcFile, readLink(dstFile), priority); + throw BuildEnvFileConflictError( + readLink(dstFile), + srcFile, + priority + ); if (prevPriority < priority) continue; if (unlink(dstFile.c_str()) == -1) diff --git a/src/libstore/builtins/buildenv.hh b/src/libstore/builtins/buildenv.hh index 73c0f5f7f..a018de3af 100644 --- a/src/libstore/builtins/buildenv.hh +++ b/src/libstore/builtins/buildenv.hh @@ -12,6 +12,32 @@ struct Package { Package(const Path & path, bool active, int priority) : path{path}, active{active}, priority{priority} {} }; +class BuildEnvFileConflictError : public Error +{ +public: + const Path fileA; + const Path fileB; + int priority; + + BuildEnvFileConflictError( + const Path fileA, + const Path fileB, + int priority + ) + : Error( + "Unable to build profile. There is a conflict for the following files:\n" + "\n" + " %1%\n" + " %2%", + fileA, + fileB + ) + , fileA(fileA) + , fileB(fileB) + , priority(priority) + {} +}; + typedef std::vector Packages; void buildProfile(const Path & out, Packages && pkgs); From 872cdb4346f72186ae683f90ac97b8a7d9bddfd4 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Fri, 10 Feb 2023 14:18:27 +0100 Subject: [PATCH 2/5] nix-profile-install: show helpful error upon package conflict Whenever a file conflict happens during "nix profile install" an error is shown that was previously thrown inside builtins.buildEnv. We catch BuildProfileConflictError here so that we can provide the user with more useful instructions on what to do next. Most notably, we give the user concrete commands to use with all parameters already filled in. This avoids the need for the user to look up these commands in manual pages. --- src/nix/profile.cc | 56 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 32364e720..3464a977d 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -329,7 +329,61 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile manifest.elements.push_back(std::move(element)); } - updateProfile(manifest.build(store)); + try { + updateProfile(manifest.build(store)); + } catch (BuildEnvFileConflictError & conflictError) { + auto findRefByFilePath = [&](Iterator begin, Iterator end) { + for (auto it = begin; it != end; it++) { + auto profileElement = *it; + for (auto & storePath : profileElement.storePaths) { + if (conflictError.fileA.starts_with(store->printStorePath(storePath))) { + return std::pair(conflictError.fileA, profileElement.source->originalRef); + } + if (conflictError.fileB.starts_with(store->printStorePath(storePath))) { + return std::pair(conflictError.fileB, profileElement.source->originalRef); + } + } + } + throw conflictError; + }; + // There are 2 conflicting files. We need to find out which one is from the already installed package and + // which one is the package that is the new package that is being installed. + // The first matching package is the one that was already installed (original). + auto [originalConflictingFilePath, originalConflictingRef] = findRefByFilePath(manifest.elements.begin(), manifest.elements.end()); + // The last matching package is the one that was going to be installed (new). + auto [newConflictingFilePath, newConflictingRef] = findRefByFilePath(manifest.elements.rbegin(), manifest.elements.rend()); + + throw Error( + "An existing package already provides the following file:\n" + "\n" + " %1%\n" + "\n" + "This is the conflicting file from the new package:\n" + "\n" + " %2%\n" + "\n" + "To remove the existing package:\n" + "\n" + " nix profile remove %3%\n" + "\n" + "The new package can also be installed next to the existing one by assigning a different priority.\n" + "The conflicting packages have a priority of %5%.\n" + "To prioritise the new package:\n" + "\n" + " nix profile install %4% --priority %6%\n" + "\n" + "To prioritise the existing package:\n" + "\n" + " nix profile install %4% --priority %7%\n", + originalConflictingFilePath, + newConflictingFilePath, + originalConflictingRef.to_string(), + newConflictingRef.to_string(), + conflictError.priority, + conflictError.priority - 1, + conflictError.priority + 1 + ); + } } }; From 3efa476c5439f8f6c1968a6ba20a31d1239c2f04 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Fri, 24 Feb 2023 21:26:36 +0100 Subject: [PATCH 3/5] tests: nix-profile: test install error message upon conflicting files --- tests/nix-profile.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh index 266dc9e49..d4beaa190 100644 --- a/tests/nix-profile.sh +++ b/tests/nix-profile.sh @@ -140,6 +140,32 @@ printf World2 > $flake2Dir/who nix profile install $flake1Dir [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] +expect 1 nix profile install $flake2Dir +diff -u <(nix profile install $flake2Dir 2>&1 1> /dev/null || true) <(cat << EOF +error: An existing package already provides the following file: + + $(nix build --no-link --print-out-paths ${flake1Dir}"#default.out")/bin/hello + + This is the conflicting file from the new package: + + $(nix build --no-link --print-out-paths ${flake2Dir}"#default.out")/bin/hello + + To remove the existing package: + + nix profile remove path:${flake1Dir} + + The new package can also be installed next to the existing one by assigning a different priority. + The conflicting packages have a priority of 5. + To prioritise the new package: + + nix profile install path:${flake2Dir} --priority 4 + + To prioritise the existing package: + + nix profile install path:${flake2Dir} --priority 6 +EOF +) +[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] nix profile install $flake2Dir --priority 100 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] nix profile install $flake2Dir --priority 0 From 0167862e8e3faddeab6fb40a08c05a63865fe96b Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Tue, 28 Feb 2023 10:53:42 +0100 Subject: [PATCH 4/5] fixup! tests: nix-profile: test install error message upon conflicting files --- tests/nix-profile.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh index d4beaa190..652e8a8f2 100644 --- a/tests/nix-profile.sh +++ b/tests/nix-profile.sh @@ -141,7 +141,11 @@ printf World2 > $flake2Dir/who nix profile install $flake1Dir [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] expect 1 nix profile install $flake2Dir -diff -u <(nix profile install $flake2Dir 2>&1 1> /dev/null || true) <(cat << EOF +diff -u <( + nix --offline profile install $flake2Dir 2>&1 1> /dev/null \ + | grep -vE "^warning: " \ + || true +) <(cat << EOF error: An existing package already provides the following file: $(nix build --no-link --print-out-paths ${flake1Dir}"#default.out")/bin/hello From 12538605fdc99be846b736080558567edbb8a393 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Wed, 1 Mar 2023 07:40:44 +0100 Subject: [PATCH 5/5] nix-profile: add FIXME about using C++20 std::ranges --- src/nix/profile.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 3464a977d..3d5c0c8a3 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -332,6 +332,8 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile try { updateProfile(manifest.build(store)); } catch (BuildEnvFileConflictError & conflictError) { + // FIXME use C++20 std::ranges once macOS has it + // See https://github.com/NixOS/nix/compare/3efa476c5439f8f6c1968a6ba20a31d1239c2f04..1fe5d172ece51a619e879c4b86f603d9495cc102 auto findRefByFilePath = [&](Iterator begin, Iterator end) { for (auto it = begin; it != end; it++) { auto profileElement = *it;