pkgs/by-name: recommendations for multi-versioned packages #292214
Conversation
delroth
added
the
12.approvals: 1
|
Not directly related, but a thing that concerns me is that |
|
@AndersonTorres Quick thoughts: I like how This seems like an orthogonal problem though, could you open an issue for this? |
ofborg
bot
added
10.rebuild-darwin: 0
|
There are a lot of options in here, but it's not clear to me on first read which I'd want. Is there no way to loosen the checks somehow for packages that have yet to migrate? |
I do think the first one is the best general recommendation, maybe that should be highlighted more. The others are really just further refactorings that could optionally be done, which at that point don't actually have much to do with
I'm fairly undecided about this, but that is indeed a possibility. It will even be fairly easy once the automated migration is implemented (in code), because that needs to check the very same property: It can only migrate packages that don't share any files with other packages. |
There was a problem hiding this comment.
Thanks for writing this up, @infinisil. Very helpful!
Especially the list of example PRs where this popped up. I'd like to suggest to include #289690 in said list.
See below a minor blank space suggestion for consistency if this is relevant/important for project READMEs.
delroth
added
12.approvals: 2
Over the past weeks, we've seen one oversight with the new enforcement of `pkgs/by-name` for new packages. This documents the problem and the recommendation for resolving it.
infinisil
force-pushed
the
by-name-multi-version-docs
branch
from
ebba600 to
8032bb6
Compare
|
@afh Thanks, done that! |
delroth
removed
the
12.approvals: 2
The `pkgs/by-name` check currently fails on the staging-next merge into master (https://github.com/NixOS/nixpkgs/actions/runs/8110296543/job/22167262748) because it contains the changes of NixOS#278920, which introduced a "new" package using `callPackage` but not using `pkgs/by-name`. This was never noticed in that PR because CI last ran almosts 2 months ago, which is before the check for new packages was introduced. This wouldn't be a problem normally, it's only become a problem because of the staging-next merge into master, which effectively PRs the same change again (and the `pkgs/by-name` check doesn't try to distinguish between branches). The fix for this is a bit special because it's not actually a "new" package, but rather just a new version of an existing package. The `pkgs/by-name` check can't distinguish between such cases though. So instead we make sure that the `pkgs/by-name` check doesn't think of it as a package using `callPackage` by using the recommendation for multi-versioned packages from NixOS#292214.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-deal-with-two-versions-of-a-package-in-by-name/40841/2 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-deal-with-two-versions-of-a-package-in-by-name/40841/5 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Description of changes
Over the past weeks, we've seen one oversight with the new enforcement of
pkgs/by-namefor new packages.This documents the problem and the recommendation for resolving it.
Here's some PRs that could've benefited from this:
In the future, the error message should also link to this documentation
Ping @AndersonTorres @afh
This work is sponsored by Antithesis ✨
Add a 👍 reaction to pull requests you find important.