Skip to content

feat: nixpkgs-vet --version#158

Merged
philiptaron merged 2 commits intoNixOS:mainfrom
dtomvan:main
May 19, 2025
Merged

feat: nixpkgs-vet --version#158
philiptaron merged 2 commits intoNixOS:mainfrom
dtomvan:main

Conversation

@dtomvan
Copy link
Copy Markdown
Contributor

@dtomvan dtomvan commented May 10, 2025

This PR simply adds the clap-flag version, so that -V or --version can be passed to get the current version of nixpkgs-vet.

Why: to make versionCheckHook work. Some effort is being made right now to package nixpkgs-vet inside of nixpkgs (here) and versionCheckHook is a good practice to allow r-ryantm to test its updates, and a way to make sure that a build only succeeds when the output binary has at least some basic functionality.

@dtomvan dtomvan requested a review from a team as a code owner May 10, 2025 14:28
Copy link
Copy Markdown
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.

Thanks! Could you add a test using versionCheckHook to the package build?

@philiptaron
Copy link
Copy Markdown
Contributor

Huh.

Process completed with exit code 143.

No error logs.

Not great.

@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented May 11, 2025

Thanks! Could you add a test using versionCheckHook to the package build?

Oh, I'm sorry. I forgot the in-tree package.nix, and only thought about nixpkgs...

With your commit, if I add doInstallCheck = true; I get:

Executing versionCheckPhase
Did not find version 0.1.4 in the output of the command /nix/store/cd833kis53ajzk2wjkwdqkimrqx94s07-nixpkgs-vet-0.1.4/bin/nixpkgs-vet --help
Program to check the validity of pkgs/by-name

This CLI interface may be changed over time if the CI workflow making use of it is adjusted to
deal with the change appropriately.

Exit code:
- `0`: If the validation is successful
- `1`: If the validation is not successful
- `2`: If an unexpected I/O error occurs

Standard error:
- Informative messages
- Detected problems if validation is not successful

Usage: nixpkgs-vet --base <BASE> <NIXPKGS>

Arguments:
  <NIXPKGS>
          Path to the main Nixpkgs to check. For PRs, set this to a checkout of the PR branch

Options:
      --base <BASE>
          Path to the base Nixpkgs to run ratchet checks against. For PRs, set this to a checkout of the PRs base branch

  -h, --help
          Print help (see a summary with '-h')

  -V, --version
          Print version
Successfully managed to find version 0.1.4 in the output of the command /nix/store/cd833kis53ajzk2wjkwdqkimrqx94s07-nixpkgs-vet-0.1.4/bin/nixpkgs-vet --version
nixpkgs-vet 0.1.4

So, that seems correct, no? I suspect some other tests are failing then, like /nix/store/8adxcgysy2jprpr6xpxpxv1l0f9qnn55-test-nixpkgs-vet-with-nix-2.25.5.drv, since running those in parallel takes a lot of memory...

Locally, with 16 GB of RAM, and almost filling all of my 20GB swap, everything passed:

result -> /nix/store/ivg6pwfr1b6c589189ir21rzll1zz74r-auto-pr-update
result-2 -> /nix/store/cd833kis53ajzk2wjkwdqkimrqx94s07-nixpkgs-vet-0.1.4
result-3 -> /nix/store/72jrhrxvxma41sz4l3vzn1b86gqkbsns-ci
result-4 -> /nix/store/6ilkcbaxdhg3v43yxm5i302r06fyy3qq-test-nixpkgs-vet-with-nix-2.24.12
result-5 -> /nix/store/wp0plccpzv3nbxr6fm44jf6k2zm07bda-test-nixpkgs-vet-with-nix-2.25.0pre20241101_2e5759e3
result-6 -> /nix/store/k2vhzj0c36q6f5m8a77lacbpvhhjzzw9-test-nixpkgs-vet-with-lix-2.91.1
result-7 -> /nix/store/c4r0r93j5523dpbyggdgzyrjv6wqjmdr-test-nixpkgs-vet-with-nix-2.25.5
result-8 -> /nix/store/lqk0h3cizhkaadm5axwgfwyxkrq2wi6j-test-nixpkgs-vet-with-nix-2.3.18
result-9 -> /nix/store/11ij3v009r8hfpxbh8xbpsj2cq16zzh3-nix-shell
result-10 -> /nix/store/0w9xvplihlc6nlf7i89q0dwv9qa6zc5k-treefmt-check

@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented May 11, 2025

Cachix sets max-jobs to auto. I'm pretty sure this leads to Github killing the test midway due to OOM. So I think the following YAML snippet will allow all tests to be ran one-by-one in CI, if desired.

with:
  extra_nix_config: |
    max-jobs = 1

@philiptaron philiptaron merged commit e722601 into NixOS:main May 19, 2025
3 checks passed
@mdaniels5757 mdaniels5757 mentioned this pull request Mar 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants