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

[infinisil]

Context

This is the final follow-up to #275539, which has much more context. At the time of that PR, the changes were supposed to be automatically used in the next channel update, but after discovering a problem (#281390), I quickly changed how this works in #281374: Now one needs to manually update the pinned version of the tooling before it ends up getting used in CI.

Description of changes

This is the first PR that updates the pinned tooling. This is done by running update-pinned-tool.sh.

Immediately following the merge of this PR, all CI runs against master will start to enforce pkgs/by-name for new packages.

Note that CI will test the new tooling right inside this PR itself as well, so if CI is green, this should be good to merge.

Things done

• Confirmed that CI for this very PR uses the new tooling and succeeds
  • https://github.com/NixOS/nixpkgs/actions/runs/7574031596/job/20627512813?pr=281835
• Checked that PRs adding new packages not to pkgs/by-name fail:
  • Test that a new package fails CI if not added to pkgs/by-name tweag/nixpkgs#82

---

Add a 👍 reaction to pull requests you find important.

[SuperSandro2000]

I cherry-picked the necessary commits from master and this PR and run it on top of #282015 on my nixos-unstable branch and it looks like it does what it should.

 ➜ ./run-local.sh nixos-unstable
Warning: Dirty tree, uncommitted changes won't be taken into account
Using HEAD commit 9c80e56ee93bc0448c2b3f2ac64937da80fe1eb4
Creating Git worktree for the HEAD commit in /run/user/1000/tmp.EAX8mafz39/merged.. Done
Fetching base branch nixos-unstable to compare against.. 842d9d80cfd4560648c785f8a4e6f3b096790e19
Creating Git worktree for the base branch in /run/user/1000/tmp.EAX8mafz39/base.. Done
Merging base branch into the HEAD commit in /run/user/1000/tmp.EAX8mafz39/merged.. 9c80e56ee93bc0448c2b3f2ac64937da80fe1eb4
Reading pinned nixpkgs-check-by-name revision from pinned-tool.json.. 842d9d80cfd4560648c785f8a4e6f3b096790e19
Creating Git worktree for the nixpkgs-check-by-name revision in /run/user/1000/tmp.EAX8mafz39/tool-nixpkgs.. Done
Building/fetching nixpkgs-check-by-name..
/nix/store/8habk3j25bs2a34zn5q5p17b9dl3fywg-nixpkgs-check-by-name
Running nixpkgs-check-by-name..
pkgs.nodejs-16_x: This top-level package was previously defined in pkgs/by-name/no/nodejs-16_x/package.nix, but is now manually defined as `callPackage ./pkgs/development/web/nodejs/v16.nix { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
pkgs.nodejs-slim-16_x: This top-level package was previously defined in pkgs/by-name/no/nodejs-slim-16_x/package.nix, but is now manually defined as `callPackage ./pkgs/development/web/nodejs/v16.nix { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
pkgs.nodejs-slim_16: This top-level package was previously defined in pkgs/by-name/no/nodejs-slim_16/package.nix, but is now manually defined as `callPackage ./pkgs/development/web/nodejs/v16.nix { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
pkgs.nodejs_16: This top-level package was previously defined in pkgs/by-name/no/nodejs_16/package.nix, but is now manually defined as `callPackage ./pkgs/development/web/nodejs/v16.nix { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
Validation failed, see above errors
Cleaning up.. Done

I reverted the nodejs_16 removal in that branch because I still need it.
This just raises on concern: Do we now need to update the rev everytime a new compiler/interpreter version is released?

[infinisil]

@SuperSandro2000

I reverted the nodejs_16 removal in that branch because I still need it.

Not sure what you mean, those validation errors shouldn't occur if you removed nodejs. And I can't see anything relating to nodejs in that PR.

This just raises on concern: Do we now need to update the rev everytime a new compiler/interpreter version is released?

Also not entirely sure what you mean. The rev that gets updated in this PR is only used for the version of the nixpkgs-check-by-name tool. It only needs to be updated if the tooling changed in any significant way and we want to update CI to use those changes.

[SuperSandro2000]

Not sure what you mean, those validation errors shouldn't occur if you removed nodejs. And I can't see anything relating to nodejs in that PR.

I am reverting a removal in my branch on top of that PR , so there is a new package added and the script fails like expected.

The rev that gets updated in this PR is only used for the version of the nixpkgs-check-by-name tool. It only needs to be updated if the tooling changed in any significant way and we want to update CI to use those changes.

I confused that with the reference point the tool is comparing to was.

---

I just have 1 3 tiny questions left: What would be the workflow if e.g. a new version of nodejs would be added? Do would now need to go to pkgs-by-name? Also would the CI then fail for all PRs until nixos-unstable moved?

[infinisil]

I am reverting a removal in my branch on top of that PR , so there is a new package added and the script fails like expected.

I see, makes sense!

What would be the workflow if e.g. a new version of nodejs would be added? Do would now need to go to pkgs-by-name?

Indeed that would be the new way to do it. It could be refactored to a callPackages and live outside pkgs/by-name too though.

Also would the CI then fail for all PRs until nixos-unstable moved?

CI compares the PR head against the base branch of the PR, so nixos-unstable isn't relevant for it. If a top-level callPackage package didn't exist in the base branch, but does exist in the PR, and it's not defined via pkgs/by-name, only then CI fails.

And here's a hack if you really need to: PRs that only fail this new "should be moved to pkgs/by-name" check can still be merged, they won't break master. This can still be used in case we discover that pkgs/by-name doesn't work well for some edge cases. Generally refactoring to adhere to pkgs/by-name is preferable though. Ping me if that doesn't work.

[infinisil]

Let's go! I'll stay online for the next couple hours to tend to any potential fallout

[SuperSandro2000]

Thanks for the explanations!

[nixos-discourse]

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

https://discourse.nixos.org/t/tweag-nix-dev-update-53/39180/1