[RFC 140 2a] `pkgs/by-name`: Enforce for new packages #275539

infinisil · 2023-12-20T01:15:03Z

Note

An open call for the final review and merge was held on 2024-01-15, see #275539 (comment)

Context

This is part of the effort to implement RFC 140, aka pkgs/by-name. While previous PRs already enabled users to define packages using pkgs/by-name, there's still thousands of packages still using all-packages.nix, and new ones added every day.

This PR is part of the RFC 140 migration plan, building on top of #272395 and #274591.

This work is sponsored by Antithesis and Tweag ✨

Changes

This PR prevents new packages using pkgs.callPackage from being defined not in pkgs/by-name. Instead the pkgs/by-name directory has to be used instead. This the A-side of Part 2 of the RFC.

This doesn't affect packages defined using alternate callPackage's like libsForQt5.callPackage or callPackages, which pkgs/by-name cannot handle (yet?).

Note

The effects of this PR will only be seen once the nixos-unstable channel updates, since its pre-built version of the tooling is used for CI.

Things done

Tests updated
- Test to ensure that new packages using callPackage but not using pkgs/by-name throw an error
- Test to ensure that a package moved out of pkgs/by-name throws an error
- Test to ensure that new packages using an alternate callPackage doesn't give an error.
Tested on the entirety of Nixpkgs
- Ensured okay performance (about 25 single-core seconds on my machine)
Tidy code
Clean history

Add a 👍 reaction to pull requests you find important.

infinisil · 2024-01-05T00:53:02Z

This PR is now theoretically ready, but since the changes got a bit big, I split off everything not directly related to this PR into #278805, which should be merged first.

Ericson2314 · 2024-01-08T15:26:39Z

pkgs/by-name/README.md

-Packages found in the named-based structure do not need to be explicitly added to the
+Packages found in the name-based structure must not be added to the
 `top-level/all-packages.nix` file unless they require overriding the default value
 of an implicit attribute (see below).


I think it is still better to lead with the positive part. Something like

Packages found in the name-based structure are automatically included, without needing to be added in a file like all-packages.nix. They must only occur in file if they require overriding the default value of an implicit attribute (see below).

I changed the wording a little bit, but this is essentially applied now :)

nixos-discourse · 2024-01-09T18:49:29Z

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

https://discourse.nixos.org/t/2024-01-09-nixpkgs-architecture-team-meeting-47/38037/1

pkgs/by-name/README.md

philiptaron

I took a thorough look. The tests are quite clear. Nice work. I don't have any code related comments, just copy-editing. Might also run a RFC101 formatter over these Nix files just for giggles.

pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs

pkgs/test/nixpkgs-check-by-name/src/ratchet.rs

philiptaron · 2024-01-09T22:18:03Z

pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs

+                    if let Some(path) = &call_package_path {
+                        format!("./{}", path.display())
+                    } else {
+                        "...".into()


For my notes: pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected is the test of this.

pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs

pkgs/test/nixpkgs-check-by-name/src/eval.rs

philiptaron · 2024-01-09T22:25:53Z

pkgs/test/nixpkgs-check-by-name/src/eval.rs

+                    // This is a bit of an odd case: We don't even _know_ whether this attribute
+                    // would qualify for using pkgs/by-name. We can either:
+                    // - Assume it's not using pkgs/by-name, which has the problem that if a
+                    //   package evaluation gets broken temporarily, the fix can remove it from
+                    //   pkgs/by-name again
+                    // - Assume it's using pkgs/by-name already, which has the problem that if a
+                    //   package evaluation gets broken temporarily, fixing it requires a move to
+                    //   pkgs/by-name


Future ratchets might try to drive this number down, I might imagine.

Oh yeah, I thought about implementing an eval-successful ratchet, but I think this should be a future PR. Then also it's also not pkgs/by-name-specific anymore and the tool could use a rename.

One only needs to look at the list of things detected by NixpkgsProblems to realize we've backed ourselves into a damn fine nixpkgs linter framework. 🚀

pkgs/test/nixpkgs-check-by-name/src/eval.nix

In a future commit this will be extended

Not that important, but nice. Also adds a nice test case show-casing the two current ratchet checks at once.

infinisil · 2024-01-16T18:50:40Z

Unfortunately this has a bug that needs to be addressed. Without doing anything, all PRs to master would start failing CI in like 1 day once nixos-unstable updates. I opened this PR to prevent this: #281374

Here's a follow up to fix the bug and improve CI to prevent this in the future: #281390

This was an oversight in NixOS#275539 not accounted for, which would've failed in CI

infinisil · 2024-01-18T17:55:55Z

Now that nixos-unstable contains the latest fixes, here's the final follow-up PR: #281835 which starts enabling the enforcement!

Includes #275539

Includes NixOS#275539

This was an oversight in NixOS/nixpkgs#275539 not accounted for, which would've failed in CI

Includes NixOS/nixpkgs#275539

infinisil added this to the RFC 140 milestone Dec 20, 2023

infinisil mentioned this pull request Dec 20, 2023

RFC 140 migration plan #258650

Open

infinisil force-pushed the by-name-enforce branch from c53ec7b to 9d746df Compare December 20, 2023 01:19

infinisil added the significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Dec 20, 2023

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Dec 20, 2023

infinisil mentioned this pull request Dec 20, 2023

[RFC 0140] Simple Package Paths NixOS/rfcs#140

Merged

donovanglover mentioned this pull request Dec 21, 2023

flatpak-pip-generator: init at unstable-2023-11-27 #274751

Closed

13 tasks

infinisil mentioned this pull request Dec 21, 2023

pkgs/by-name: Enable gradual migration checks and add run-local.sh #274591

Merged

4 tasks

infinisil force-pushed the by-name-enforce branch 5 times, most recently from 1c31376 to 41b1926 Compare January 5, 2024 00:50

infinisil mentioned this pull request Jan 5, 2024

check-by-name: Refactor to prepare for enforcing pkgs/by-name, make --base required #278805

Merged

1 task

infinisil force-pushed the by-name-enforce branch from 41b1926 to 6ff113c Compare January 5, 2024 01:02

Ericson2314 reviewed Jan 8, 2024

View reviewed changes

infinisil requested a review from philiptaron January 9, 2024 22:02

infinisil force-pushed the by-name-enforce branch from 6ff113c to 6e192d8 Compare January 9, 2024 22:03

infinisil marked this pull request as ready for review January 9, 2024 22:03

philiptaron reviewed Jan 9, 2024

View reviewed changes

pkgs/by-name/README.md Outdated Show resolved Hide resolved

philiptaron approved these changes Jan 9, 2024

View reviewed changes

delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 9, 2024

infinisil added 3 commits January 10, 2024 19:28

tests.nixpkgs-check-by-name: Basic info for non-by-name attributes

69fc71a

In a future commit this will be extended

tests.nixpkgs-check-by-name: Enforce pkgs/by-name for new packages

57afdc5

tests.nixpkgs-check-by-name: Sort the eval validation results

cbff028

Not that important, but nice. Also adds a nice test case show-casing the two current ratchet checks at once.

infinisil mentioned this pull request Jan 16, 2024

workflows/check-by-name: Pin nixpkgs-check-by-name tool #281374

Merged

infinisil added a commit to tweag/nixpkgs that referenced this pull request Jan 16, 2024

tests.nixpkgs-check-by-name: Don't error for pkgs/by-name aliases

0f27917

This was an oversight in NixOS#275539 not accounted for, which would've failed in CI

infinisil mentioned this pull request Jan 16, 2024

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

Merged

infinisil mentioned this pull request Jan 18, 2024

Enable enforcement of pkgs/by-name for new packages (by updating the pinned version) #281835

Merged

infinisil mentioned this pull request Jan 18, 2024

Test that a new package fails CI if not added to pkgs/by-name tweag/nixpkgs#82

Closed

infinisil added a commit that referenced this pull request Jan 19, 2024

check-by-name: Update pinned tooling

bfe48c9

Includes #275539

infinisil mentioned this pull request Jan 19, 2024

wireguard-vanity-keygen: init at 0.0.7 #282164

Merged

13 tasks

h7x4 mentioned this pull request Jan 20, 2024

fennel-ls: init at 0.1.0 #281653

Merged

13 tasks

mkg20001 pushed a commit to mkg20001/nixpkgs that referenced this pull request Jan 21, 2024

check-by-name: Update pinned tooling

abb6cde

Includes NixOS#275539

This was referenced Jan 21, 2024

workflows/check-by-name: Trigger on base branch changes #282707

Merged

sdrpp: 1.0.4 -> 1.1.0-unstable-2024-01-22 #282448

Merged

peterhoeg pushed a commit to peterhoeg/nixpkgs that referenced this pull request Jan 22, 2024

check-by-name: Update pinned tooling

20e9881

Includes NixOS#275539

This was referenced Feb 1, 2024

segger-jlink: init at 794a, nrfconnect: init at 4.3.0, nrf-command-line-tools: init at 10.23.2 #255185

Merged

xppen: init package with nixos module #285660

Open

lelgenio mentioned this pull request Feb 7, 2024

xcbeautify: init at 1.4.0 #286284

Merged

13 tasks

infinisil mentioned this pull request Feb 7, 2024

Improve pkgs/by-name code, minor fix #287083

Merged

1 task

arikgrahl mentioned this pull request Feb 12, 2024

css-checker: init at 0.4.1 #191191

Merged

13 tasks

infinisil added a commit to NixOS/nixpkgs-vet that referenced this pull request Mar 18, 2024

tests.nixpkgs-check-by-name: Don't error for pkgs/by-name aliases

08f591a

This was an oversight in NixOS/nixpkgs#275539 not accounted for, which would've failed in CI

infinisil added a commit to NixOS/nixpkgs-vet that referenced this pull request Mar 18, 2024

check-by-name: Update pinned tooling

e34551c

Includes NixOS/nixpkgs#275539

AndersonTorres mentioned this pull request Aug 10, 2024

smc-fuzzer: init at 0-unstable-2020-12-23 #332586

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC 140 2a] `pkgs/by-name`: Enforce for new packages #275539

[RFC 140 2a] `pkgs/by-name`: Enforce for new packages #275539

infinisil commented Dec 20, 2023 •

edited

Loading

infinisil commented Jan 5, 2024

Ericson2314 Jan 8, 2024

infinisil Jan 10, 2024

nixos-discourse commented Jan 9, 2024

philiptaron left a comment

philiptaron Jan 9, 2024

philiptaron Jan 9, 2024

infinisil Jan 9, 2024

philiptaron Jan 10, 2024

infinisil commented Jan 16, 2024 •

edited

Loading

infinisil commented Jan 18, 2024

[RFC 140 2a] pkgs/by-name: Enforce for new packages #275539

[RFC 140 2a] pkgs/by-name: Enforce for new packages #275539

Conversation

infinisil commented Dec 20, 2023 • edited Loading

Context

Changes

Things done

infinisil commented Jan 5, 2024

Ericson2314 Jan 8, 2024

Choose a reason for hiding this comment

infinisil Jan 10, 2024

Choose a reason for hiding this comment

nixos-discourse commented Jan 9, 2024

philiptaron left a comment

Choose a reason for hiding this comment

philiptaron Jan 9, 2024

Choose a reason for hiding this comment

philiptaron Jan 9, 2024

Choose a reason for hiding this comment

infinisil Jan 9, 2024

Choose a reason for hiding this comment

philiptaron Jan 10, 2024

Choose a reason for hiding this comment

infinisil commented Jan 16, 2024 • edited Loading

infinisil commented Jan 18, 2024

[RFC 140 2a] `pkgs/by-name`: Enforce for new packages #275539

[RFC 140 2a] `pkgs/by-name`: Enforce for new packages #275539

infinisil commented Dec 20, 2023 •

edited

Loading

infinisil commented Jan 16, 2024 •

edited

Loading