Skip to content

nix: check fallback paths#266347

Merged
roberth merged 2 commits into
NixOS:masterfrom
hercules-ci:nix-check-fallback-paths
Nov 13, 2023
Merged

nix: check fallback paths#266347
roberth merged 2 commits into
NixOS:masterfrom
hercules-ci:nix-check-fallback-paths

Conversation

@roberth

@roberth roberth commented Nov 8, 2023

Copy link
Copy Markdown
Member

Description of changes

Add a test for ofborg to run. Detects out of date fallback paths by comparing versions.

#266264 (comment)

Free @lovesegfault to do cool things instead of nagging about fallback paths.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@lovesegfault lovesegfault left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please redeem these free beverage tickets when we see each other again: 🎟️ 🎟️

Thank you!!

@ofborg ofborg Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 8, 2023
@infinisil

Copy link
Copy Markdown
Member

Related:

@roberth roberth force-pushed the nix-check-fallback-paths branch from 6448881 to c8b8ab8 Compare November 10, 2023 17:18
@roberth roberth marked this pull request as ready for review November 10, 2023 17:19
Comment thread pkgs/tools/package-management/nix/default.nix Outdated
Co-authored-by: Artturi <Artturin@artturin.com>
ghost
ghost previously requested changes Nov 11, 2023
Comment on lines +124 to +145
addFallbackPathsCheck = pkg: addTestsShallowly
{ nix-fallback-paths =
runCommand "test-nix-fallback-paths-version-equals-nix-stable" {
paths = lib.concatStringsSep "\n" (builtins.attrValues (import ../../../../nixos/modules/installer/tools/nix-fallback-paths.nix));
} ''
if [[ "" != $(grep -v 'nix-${pkg.version}$' <<< "$paths") ]]; then
echo "nix-fallback-paths not up to date with nixVersions.stable (nix-${pkg.version})"
echo "The following paths are not up to date:"
grep -v 'nix-${pkg.version}$' <<< "$paths"
echo
echo "Fix it by running in nixpkgs:"
echo
echo "curl https://releases.nixos.org/nix/nix-${pkg.version}/fallback-paths.nix >nixos/modules/installer/tools/nix-fallback-paths.nix"
echo
exit 1
else
echo "nix-fallback-paths versions up to date"
touch $out
fi
'';
}
pkg;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is nixos-specific; it belongs in nixos/ not in nixpkgs/pkgs/.

@Artturin Artturin Nov 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It'll have to be added to the nix attribute in any case, just like we add nixosTests.

In my opinion it's ok to keep the definition here because I can't think of a other place to put it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It'll have to be added to the nix attribute in any case, just like we add nixosTests.

That can be done in the nixos module for nix.

I can't think of a other place to put it.

I can: in nixos/. These "fallback paths" are meaningless on non-NixOS uses of nixpkgs.

@Artturin Artturin Nov 12, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The file is used by nixos-rebuild and nix upgrade-nix https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-upgrade-nix.html#description

The test has to be attached to nixVersions.stable in some way for ofborg to run it

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The file is used by nixos-rebuild

Like I said, it's nixos-specific.

The test has to be attached to nixVersions.stable in some way for ofborg to run it

ofborg runs plenty of tests from other parts of the tree, like lib, etc.

@roberth roberth dismissed ghost ’s stale review November 13, 2023 09:47

It is accepted for the tests attribute to use NixOS. Suggested change is non-trivial and doesn't leverage existing solutions as much. The architectural rules can be changed, but that's beyond the scope of this change.

@roberth roberth merged commit 93cb2c2 into NixOS:master Nov 13, 2023
@ghost

ghost commented Nov 30, 2023

Copy link
Copy Markdown

roberth dismissed amjoseph-nixpkgs' review 2 weeks ago

💩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants