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

Improve error on conflict for nix profile install #7788

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/libstore/builtins/buildenv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 26 additions & 0 deletions src/libstore/builtins/buildenv.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
bobvanderlinden marked this conversation as resolved.
Show resolved Hide resolved
: 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<Package> Packages;

void buildProfile(const Path & out, Packages && pkgs);
Expand Down
58 changes: 57 additions & 1 deletion src/nix/profile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,63 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile
manifest.elements.push_back(std::move(element));
}

updateProfile(manifest.build(store));
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 = [&]<typename Iterator>(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"
Copy link
Member

Choose a reason for hiding this comment

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

Probably nicer to write this using multiline string literals, i.e. R"(...)".

Copy link
Member Author

Choose a reason for hiding this comment

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

I have looked into various ways of doing multiline strings in C++. I think R"(...)" was one of them and it seemed it didn't support removing indentation, so it required me to put all text on the first column. The current solution seemed like one that did properly indent, which looked a bit better.

" %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
);
}
}
};

Expand Down
30 changes: 30 additions & 0 deletions tests/nix-profile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,36 @@ 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 --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

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
Expand Down