Skip to content

linux: remove assert on linux#427219

Merged
K900 merged 1 commit intoNixOS:masterfrom
wolfgangwalther:ci-eval-asserts-linux
Jul 22, 2025
Merged

linux: remove assert on linux#427219
K900 merged 1 commit intoNixOS:masterfrom
wolfgangwalther:ci-eval-asserts-linux

Conversation

@wolfgangwalther
Copy link
Contributor

@wolfgangwalther wolfgangwalther commented Jul 21, 2025

Asserting the host platform for linux is bad, because it can't be caught by CI. For the linux package itself, it doesn't make a difference, because it also has meta.platforms = linux set, so this will fail evaluation - and in a way that can nicely be caught by CI. Also, this error will propagate everywhere else.

Almost everywhere at least, so we add a conditional on running the pkgs.nixos test only on Linux. This was previously guarded by the assert, but would now throw eval errors deep into the NixOS eval - there are certain assumptions about packages there (glibcLocales not being null the first I hit) that only work on Linux. Which is fine.

Hopefully this has zero rebuilds, let's see...

Related: #426629

Things done

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: kernel The Linux kernel labels Jul 21, 2025
@wolfgangwalther
linux: remove assert on linux
0febbb4 
Asserting the hostplatform for `linux` is bad, because it can't be
caught by CI. For the `linux` package itself, it doesn't make a
difference, because it also has `meta.platforms = linux` set, so this
will fail evaluation - and in a way that can nicely be caught by CI.
@wolfgangwalther wolfgangwalther force-pushed the ci-eval-asserts-linux branch from c8e522b to 0febbb4 Compare July 21, 2025 19:16
@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Jul 21, 2025

Hopefully this has zero rebuilds, let's see...

A bunch of rebuilds on the first try. Added one missing meta.platforms = linux for ply. The remaining "new" packages are linuxPackages...stdenv - which seems to be just an alias to stdenv. Looks a bit odd, but shouldn't be a problem:

% nix-build --argstr system aarch64-darwin -A linuxKernel.packages.linux_5_10.stdenv
/nix/store/8ncahfy2083zkv8rgmzzndpdqj3yxq2a-stdenv-darwin

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 21, 2025
@K900 K900 merged commit ff84a25 into NixOS:master Jul 22, 2025
26 of 29 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jul 22, 2025

Successfully created backport PR for release-25.05:

@nixpkgs-ci nixpkgs-ci bot mentioned this pull request Jul 22, 2025
Merged
1 task
@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Jul 22, 2025
@wolfgangwalther wolfgangwalther deleted the ci-eval-asserts-linux branch July 22, 2025 06:46
@vcunat
Copy link
Member

vcunat commented Jul 24, 2025

This thing blocks all darwin channels now via https://hydra.nixos.org/build/303268171/nixlog/1/tail

@vcunat
Copy link
Member

vcunat commented Jul 24, 2025

Failing an assertion is an accepted signal to drop that package variant. Trying to access a nonexistent attribute is not accepted.

Example test case:

nix eval -f. --argstr system aarch64-darwin linux_rpi4.outPath

(always errors out, but assertions are OK)

@wolfgangwalther wolfgangwalther mentioned this pull request Jul 24, 2025
Merged
11 tasks
@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Jul 24, 2025

Failing an assertion is an accepted signal to drop that package variant.

Not anymore with the plan in #426629.

But I see we need to do more to be able to remove the linux assert.

@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Jul 24, 2025 •
edited
Loading

The specific problem we are seeing is that linux-rpi sets extraMeta.platforms, which apparently causes a lot more to be evaluated before being able to trigger the "unsupported system" error that is intended here.

I don't quite understand why this doesn't throw in #426629, though.

@vcunat
Copy link
Member

vcunat commented Jul 24, 2025

The platforms look very wrong:

$ nix eval -f. linux_rpi4.meta.platforms
[ "armv5tel-linux" "armv6l-linux" "armv7a-linux" "armv7l-linux" "armv6l-netbsd" "armv7a-netbsd" "armv7l-netbsd" "arm-none" "armv6l-none" "aarch64-darwin" "aarch64-freebsd" "aarch64-genode" "aarch64-linux" "aarch64-netbsd" "aarch64_be-none" "aarch64-none" "aarch64-windows" ]

@vcunat
Copy link
Member

vcunat commented Jul 24, 2025

Ah, of course. lib.platforms.arm and lib.platforms.aarch64 contain also non-Linuxes.

@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Jul 24, 2025

The platforms look very wrong:

nixpkgs/pkgs/os-specific/linux/kernel/linux-rpi.nix

Lines 43 to 53 in ccaad1e

extraMeta =
if (rpiVersion < 3) then
{
platforms = with lib.platforms; arm;
hydraPlatforms = [ ];
}
else
{
platforms = with lib.platforms; arm ++ aarch64;
hydraPlatforms = [ "aarch64-linux" ];
};

I guess "arm" includes darwin?

@vcunat
Copy link
Member

vcunat commented Jul 24, 2025 •
edited
Loading

After fixing this platforms to linux-only the eval seems OK for this package.

@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Jul 24, 2025 •
edited
Loading

I'll create a PR in a second: #428023

@wolfgangwalther wolfgangwalther mentioned this pull request Jul 24, 2025
Merged
1 task
@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Jul 24, 2025

I don't quite understand why this doesn't throw in #426629, though.

Ah, the eval error only appears deeper into the package. meta.available can be evaluated fine, even on darwin. Thus, attrpath generation created the attrpath, but outpath evaluation silently skipped it (due to using nix-env).

This is what I observed in #426629 (comment).

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

Reviewers

@philiptaron philiptaron philiptaron approved these changes

@K900 K900 Awaiting requested review from K900

@NeQuissimus NeQuissimus Awaiting requested review from NeQuissimus

@alyssais alyssais Awaiting requested review from alyssais

@Ma27 Ma27 Awaiting requested review from Ma27

@TredwellGit TredwellGit Awaiting requested review from TredwellGit

Assignees

No one assigned

Labels

6.topic: kernel The Linux kernel 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wolfgangwalther @vcunat @philiptaron @K900