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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

profile install: fail on conflicting name #10065

Closed

Conversation

bobvanderlinden
Copy link
Member

@bobvanderlinden bobvanderlinden commented Feb 22, 2024

Motivation

Currently it is possible to install the same package twice. This installation will only fail on conflicting files and not on conflicting names (profile entry names).

When installing an installable that has the same name as an existing profile entry, it'll be renamed to {name}-1. This can be confusing, especially when installing the exact same package, which doesn't have conflicting files. Two entries will be in the profile {name} and {name}-1.

This change will make package installation conflict based on name. In addition, --force is introduced to overwrite existing package entries (with the same name).

I previously also added a --force option, so that a user can overwrite conflicting entries. I have removed this to keep the PR small and avoid discussion about adding such an option. Instead, upon conflict Nix now suggests to use nix profile remove {name}. An example of the --force implementation can be found here. It can be added later if it is considered a good addition.

There is one use-case where this PR can be annoying. I'm hoping it is small enough it doesn't matter:
When installing 2 packages that have the same name (but are actually different packages), it is not possible, at this time, to install them both into the same profile. This is merely because it is not possible to manually define (or force) the name of the entry. (See --name idea)
That said, having two entries with seemingly the same name is already confusing, so I'm thinking hoping this won't be a real issue in practice.

Context

Fixes #5587
As suggested in #7967

Priorities and Process

Add 馃憤 to pull requests you find important.

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

@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Feb 22, 2024
@bobvanderlinden bobvanderlinden force-pushed the pr-profile-name-conflicts branch 2 times, most recently from fdf8487 to ef71147 Compare February 22, 2024 21:34
Comment on lines 206 to 207
nix build $flake1Dir --out-link result
nix profile install $(readlink result)
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a bit of a nasty one. nix build $flake1Dir --no-link --print-out-paths has 2 outputs: flake1-man and flake1. To make sure we only refer to a single output (.out) I've opted to output links and read the correct link.

Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use nix build $flake1Dir^out for that

Copy link
Member Author

Choose a reason for hiding this comment

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

Aah, right, forgot about the syntax. Thanks!

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

I think that makes sense. Just one nit wrt the tests, but it's good otherwise

Comment on lines 206 to 207
nix build $flake1Dir --out-link result
nix profile install $(readlink result)
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use nix build $flake1Dir^out for that

@edolstra
Copy link
Member

edolstra commented Feb 23, 2024

I don't think this should be a fatal error, at most it should be a warning. As you say, the derived names are not unique apart from the numerical suffix. E.g. I have names in my profile like x86_64-linux (from defaultPackage.x86_64-linux) and full (from legacyPackages.x86_64-linux.wineWowPackages.full). Nix should not disallow installing another package with the same derived name.

It's probably better to warn/error if you try to install the same flakeref twice. But given that the user will already get an error from the environment builder if there is an actual collision, we shouldn't try to be too clever here.

@bobvanderlinden
Copy link
Member Author

I don't think this should be a fatal error, at most it should be a warning.

By then the 'damage' has been done. The package with -2 suffix was already added to the profile. The confusing file-conflict error would've already popped up.

E.g. I have names in my profile like x86_64-linux (from defaultPackage.x86_64-linux)

I'll look into resolving this naming issue. I think it's a separate problem.

and full (from legacyPackages.x86_64-linux.wineWowPackages.full). Nix should not disallow installing another package with the same derived name.

Indeed, there are cases where the naming heuristics will 'fail' or there are other edge-cases where a user wants to install 2 packages that happen to have the same name. I do think this isn't true for newcomers and only advanced users will run into this. One idea that was floating around was to allow naming the entry explicitly using --name, as described here: #7967. If it is actually needed and the user indeed wants to install packages with the same implicit name, the user will need to name the entries differently explicitly.

The use-case I'm trying to address is one that newcomers will run into. Those who are installing the same package or the package from a different source. Once using nixpkgs#ripgrep, another time github:nixos/nixpkgs#ripgrep and yet another time try a stable version github:nixos/nixpkgs/nixos-23.11#ripgrep. Conflicts with these are valid and suggesting to remove the old package is too. Implicitly renaming the entry to ripgrep-2 and relying on the environment builder errors due to file-conflicts can be confusing, but this is how it is working right now.

It's probably better to warn/error if you try to install the same flakeref twice

Indeed, a separate error for this specific case would be even more friendly. I'll look into this as well 馃憤

But given that the user will already get an error from the environment builder if there is an actual collision, we shouldn't try to be too clever here.

The error from the environment builder is due to conflicting files. An elaborate error explaining these conflicting files is what I'd expect in this case for conflicting files. However, most of the users will run into this error when installing (a variant of) the same package. Shielding the user from installing them twice implicitly and the confusing error that follows would be nice I think.

@bobvanderlinden
Copy link
Member Author

bobvanderlinden commented Feb 26, 2024

I created #10090 and #10091 to resolve the defaultPackage issue and make sure installing a package twice will show an specific error.

That still leaves the problem of:

$ nix profile install nixpkgs#ripgrep
$ nix profile install github:nixos/nixpkgs/nixpkgs-unstable#ripgrep
error: An existing package already provides the following file:

         /nix/store/pgkbvml3qyzgq5bjf1q2vbm7wr0s0kv4-ripgrep-14.1.0/bin/rg

       This is the conflicting file from the new package:

         /nix/store/b3xxy0s8q6yziaav3jwkfykjirf1151i-ripgrep-13.0.0/bin/rg

       To remove the existing package:

         nix profile remove ripgrep

       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 github:nixos/nixpkgs/nixos-23.11#legacyPackages.x86_64-linux.ripgrep --priority 4

       To prioritise the existing package:

         nix profile install github:nixos/nixpkgs/nixos-23.11#legacyPackages.x86_64-linux.ripgrep --priority 6

With the changes in this PR it would show:

$ nix profile install nixpkgs#ripgrep
$ nix profile install github:nixos/nixpkgs/nixpkgs-unstable#ripgrep
error: Package 'ripgrep' is already in the profile.
       Remove the package first using 'nix profile remove ripgrep'.

The downside is that it isn't possible to do:

$ nix profile install nixpkgs#wineWowPackages.full
$ nix profile install nixpkgs#qt6.full
error: Package 'full' is already in the profile.
       Remove the package first using 'nix profile remove full'.

The idea in #7967 suggests to allow explicitly specifying the entry names:

$ nix profile install --name wineWowFull nixpkgs#wineWowPackages.full
$ nix profile install --name qt6Full nixpkgs#qt6.full

Question is, which of these cases do we want?

@thufschmitt
Copy link
Member

Discussed during the Nix maintainers meeting on 2024-02-26.
Wontfix.

  • Installing two packages with the same name shouldn't be an error.
  • Getting a unique, human-friendly and predictible name for a package isn't really possible, we'll have to choose between:
    • A friendly name, but not necessarily unique
    • A unique name (resolved flakeref), but not friendly
  • Same tradeof as with the predictible device names in the kernel

@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-02-28-nix-team-meeting-129/40499/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"nix profile" allows installing duplicate packages
4 participants