Skip to content

Comments

tests.nixpkgs-check-by-name: Fix for aliases to packages in pkgs/by-name and better testing#281390

Merged
infinisil merged 3 commits intoNixOS:masterfrom
tweag:by-name-alias-fix
Jan 17, 2024
Merged

tests.nixpkgs-check-by-name: Fix for aliases to packages in pkgs/by-name and better testing#281390
infinisil merged 3 commits intoNixOS:masterfrom
tweag:by-name-alias-fix

Conversation

@infinisil
Copy link
Member

@infinisil infinisil commented Jan 16, 2024

Context

In #275539 I made an oversight which shows itself when you run the tool on the current Nixpkgs:

❯ nix-build -A tests.nixpkgs-check-by-name
/nix/store/k7wwnxbrb2r05g5pw7hxw71gg1gls1dr-nixpkgs-check-by-name
❯ result/bin/nixpkgs-check-by-name --base . .
pkgs.NSPlist: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.
[...]
pkgs.uclibc: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.
pkgs.win-virtio: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.
Validation failed, see above errors

See #281374 to prevent the broken tooling from being used on master, that should be merged fairly soon before the nixos-unstable channel updates in like a day.

The problem

The problem is that there's some detection logic that makes sure the internal _internalCallByNamePackageFile attribute isn't used by packages.

If such a top-level attribute that's not in pkgs/by-name was detected to use this, an error is thrown. However, the detection logic can't distinguish between aliases and non-aliases.

So it gives errors even for aliases to packages defined in pkgs/by-name, e.g.

win-virtio = virtio-win; # Added 2023-10-17

https://github.com/NixOS/nixpkgs/blob/5df0c94c5ed9c4b9664d63047f2f3a7f93c0bb43/pkgs/top-level/all-packages.nix#L28854

The solution

It would be non-trivial to improve the detection logic, so instead this PR just changes the tool to not give an error in these cases.

Furthermore, a passthru.tests is added, addressing #256789 and preventing future such problems. It should be triggered automatically after ofborg finishes evaluation of a PR with a tests.nixpkgs-check-by-name: ... commit.


Add a 👍 reaction to pull requests you find important.

@infinisil
Copy link
Member Author

infinisil commented Jan 16, 2024

@ofborg build tests.nixpkgs-check-by-name tests.nixpkgs-check-by-name.tests

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/ci-will-soon-enforce-pkgs-by-name-for-new-packages/38098/2

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 16, 2024
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Sad but makes sense.

@delroth delroth added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jan 16, 2024
Copy link
Contributor

@quantenzitrone quantenzitrone left a comment

Choose a reason for hiding this comment

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

all changes are trivial enough for me to understand
so LGTM

@delroth delroth added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 2 This PR was reviewed and approved by two persons. labels Jan 17, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jan 17, 2024
@infinisil infinisil merged commit a48d8ea into NixOS:master Jan 17, 2024
@infinisil infinisil deleted the by-name-alias-fix branch January 17, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When updating nixpkgs-check-by-name, it should be run on Nixpkgs

6 participants