Skip to content

Comments

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

Merged
infinisil merged 8 commits intoNixOS:masterfrom
tweag:by-name-reproducible
Dec 21, 2023
Merged

pkgs/by-name: Enable gradual migration checks and add run-local.sh#274591
infinisil merged 8 commits intoNixOS:masterfrom
tweag:by-name-reproducible

Conversation

@infinisil
Copy link
Member

@infinisil infinisil commented Dec 16, 2023 •
edited
Loading

Description of changes

After #272395 extended the nixpkgs-check-by-name tool to have gradual migration checking, aka ratchet checks, this PR now enables them by passing the --base flag to the tool.

Furthermore, this is a really good opportunity to make the CI check runnable locally, solving #266931 as well as possible, replacing #266937.
Fundamentally it's impossible to make it fully reproducible because the base branch fetching/merging can't happen in the same way, but this PR makes everything be shared that can be shared between CI and the local run.

It's not perfect, but better than nothing.

Things done

  • Tested in CI: Introduce empty arg in a PR tweag/nixpkgs#81
  • Tested locally: 
    $ ./pkgs/test/nixpkgs-check-by-name/scripts/run-local.sh by-name-reproducible https://github.com/tweag/nixpkgs.git
Using HEAD commit cf06cb1cd4cf0f0e92fa3bd3ce894c1255fae8a1
Creating Git worktree for the HEAD commit in /tmp/tmp.osLpsrjqvl/merged.. Done
Fetching base branch by-name-reproducible to compare against.. cc0fe3b4b9f268e53fd0c1ce706f91a8d6af1373
Creating Git worktree for the base branch in /tmp/tmp.osLpsrjqvl/base.. Done
Merging base branch into the HEAD commit in /tmp/tmp.osLpsrjqvl/merged.. cf06cb1cd4cf0f0e92fa3bd3ce894c1255fae8a1
Determining the channel to use for PR base branch by-name-reproducible.. nixos-unstable
Fetching latest version of channel nixos-unstable.. /nix/store/k87dn8r5rr9nsa297my0h3z02hx62mnq-source
Git revision of channel nixos-unstable is 91a00709aebb3602f172a0bf47ba1ef013e34835
Fetching the prebuilt version of nixpkgs-check-by-name.. /nix/store/381kv4kh7sx2a43w8vcbqcy67msvyfzq-nixpkgs-check-by-name
Running nixpkgs-check-by-name..
pkgs.vcpkg-tool: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage pkgs/by-name/vc/vcpkg-tool/package.nix { ... }` with a non-empty second argument.
Validation failed, see above errors
Cleaning up.. Done
  • Updated contributor docs and code comments
  • Clean commit history

Add a 👍 reaction to pull requests you find important.

@infinisil infinisil requested a review from a user December 16, 2023 02:21
@infinisil infinisil added this to the RFC 140 milestone Dec 16, 2023
@github-actions github-actions bot added the 6.topic: policy discussion Discuss policies to work in and around Nixpkgs label Dec 16, 2023
@infinisil infinisil mentioned this pull request Dec 16, 2023
Closed
13 tasks
@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 Dec 16, 2023
@infinisil infinisil force-pushed the by-name-reproducible branch from 96acead to 1025f21 Compare December 19, 2023 02:07
@infinisil infinisil changed the title Make pkgs/by-name check as reproducible as possible, and prepare for --base Make pkgs/by-name check as reproducible as possible, and pass --base Dec 19, 2023
@infinisil infinisil mentioned this pull request Dec 19, 2023
Closed
@infinisil infinisil force-pushed the by-name-reproducible branch from 1025f21 to 63a27c1 Compare December 19, 2023 22:21
@infinisil infinisil changed the title Make pkgs/by-name check as reproducible as possible, and pass --base pkgs/by-name: Enable gradual migration checks and add run-local.sh Dec 19, 2023
@infinisil infinisil force-pushed the by-name-reproducible branch 2 times, most recently from 3f3080f to cc0fe3b Compare December 19, 2023 22:31
@infinisil infinisil marked this pull request as ready for review December 19, 2023 22:35
This was referenced Dec 20, 2023
Open
Merged
roberth
roberth reviewed Dec 20, 2023
View reviewed changes
@infinisil
tests.nixpkgs-check-by-name: Create script to run locally
92238ac 
Due to the check soon depending on the base branch (see `--base`),
the CI check can't reasonably share all code with a local check.
We can still make a script to run it locally, just not sharing all code.
@infinisil
tests.nixpkgs-check-by-name: Add documentation for scripts
9e03178
@infinisil
workflows/check-by-name: Slim down and prepare for --base
1ad45e5 
Now that we have a script to run the check locally,
there's no real need to output the information to reproduce anymore,
which allows cleaning up the CI workflow.

Furthermore, this prepares the CI workflow to be passed `--base`, as
introduced recently.
@infinisil
check-by-name: Pass --base in CI and local running script
1968bee 
This enables the ratchet checks for pkgs/by-name, allowing gradual
migrations!
@infinisil infinisil force-pushed the by-name-reproducible branch from cc0fe3b to 1968bee Compare December 20, 2023 16:38
RaitoBezarius
RaitoBezarius reviewed Dec 21, 2023
View reviewed changes
l0b0
l0b0 reviewed Dec 21, 2023
View reviewed changes
l0b0
l0b0 reviewed Dec 21, 2023
View reviewed changes
l0b0
l0b0 reviewed Dec 21, 2023
View reviewed changes
l0b0
l0b0 reviewed Dec 21, 2023
View reviewed changes
@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 21, 2023
infinisil and others added 2 commits December 22, 2023 00:04
@infinisil
maintainers/scripts/check-by-name.sh: Introduce symlink alias
f882df7
@infinisil @l0b0
pkgs/test/nixpkgs-check-by-name/scripts: Various improvements
e130ee3 
- trace function, avoids littering `echo >&2` all throughout
- Avoid `eval`, remove unneeded shellcheck

Co-Authored-By: Victor Engmark <[email protected]>
@infinisil infinisil force-pushed the by-name-reproducible branch from b62b605 to e130ee3 Compare December 21, 2023 23:06
@infinisil
Copy link
Member Author

infinisil commented Dec 21, 2023

Thanks for the detailed review @l0b0! I now squashed all the script improvements into e130ee3, adding you as a co-author :)

@infinisil
Copy link
Member Author

infinisil commented Dec 21, 2023 •
edited
Loading

I'll merge this, because:

This allows me to proceed with finalising the next step for RFC 140:

@infinisil infinisil merged commit 02b3c06 into NixOS:master Dec 21, 2023
@infinisil infinisil mentioned this pull request Dec 21, 2023
Closed
@infinisil infinisil deleted the by-name-reproducible branch January 5, 2024 00:41
@infinisil infinisil mentioned this pull request Jan 5, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RaitoBezarius RaitoBezarius RaitoBezarius left review comments

@roberth roberth roberth left review comments

+1 more reviewer

@l0b0 l0b0 l0b0 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: policy discussion Discuss policies to work in and around Nixpkgs 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: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

RFC 140

Development

Successfully merging this pull request may close these issues.

5 participants

@infinisil @l0b0 @RaitoBezarius @roberth @delroth