Skip to content

nixos/nixpkgs: fix determination for cross-compiled nixos system#292487

Merged
roberth merged 1 commit intoNixOS:masterfrom
jmbaur:nixos-cross-check
Mar 1, 2024
Merged

nixos/nixpkgs: fix determination for cross-compiled nixos system#292487
roberth merged 1 commit intoNixOS:masterfrom
jmbaur:nixos-cross-check

Conversation

@jmbaur
Copy link
Contributor

@jmbaur jmbaur commented Mar 1, 2024

Description of changes

Since the output of lib.systems.elaborate contains functions, an equality check with == does not suffice, lib.systems.equals should be used instead.

Fixes #278001

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Mar 1, 2024
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Mar 1, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 1, 2024
@jmbaur jmbaur requested a review from roberth March 1, 2024 04:56
@Mindavi
Copy link
Contributor

Mindavi commented Mar 1, 2024

Where did this isCross come from? 😅. It's way too vague like this... What about other cross types, like Canadian cross?

@roberth
Copy link
Member

roberth commented Mar 1, 2024

We have three solutions to this problem

  1. Canonicalize all pairs so that if they match, we use only one of the values in both places. Works well for Nixpkgs and its implementation.
  2. Remove the functions.
  3. Don't use ==.

2 is in progress IIRC, but is a slow deprecation. You're doing 3.

1 is a bit more effective though, because it fixes all comparisons of nixpkgs.*Platform.
Could you try add something like this to line 211?

- apply = lib.systems.elaborate;
+ apply = inputBuildPlatform:
+   let elaborated = lib.systems.elaborate inputBuildPlatform;
+   in if lib.systems.equals elaborated cfg.hostPlatform
+     then cfg.hostPlatform  # make identical, so that `==` equality works; see https://github.com/NixOS/nixpkgs/issues/278001
+     else elaborated;

(Perhaps relevant are also #237512, #254763 and related PRs/issues, if you haven't seen them)

That all said, all other modules shouldn't read from anything other than the pkgs module argument, which is the single most reliable source of truth. Unfortunately we can't enforce that; would be simpler.

@roberth
Copy link
Member

roberth commented Mar 1, 2024

@Mindavi

Where did this isCross come from? 😅. It's way too vague like this...

From me. I don't think it is vague.

What about other cross types, like Canadian cross?

Nixpkgs is not a compiler, so Canadian cross does not apply to it. Its configuration only has two platforms: build and host; no target.
Individual packages may well support a Canadian cross, but this isn't about individual packages.

@jmbaur jmbaur force-pushed the nixos-cross-check branch 2 times, most recently from cf798cd to 7395704 Compare March 1, 2024 16:54
@jmbaur
Copy link
Contributor Author

jmbaur commented Mar 1, 2024

@roberth that makes sense, and is also more forgiving to potential usage of == for comparing these platforms, applied your recommendation

@ofborg ofborg bot added the ofborg-internal-error Ofborg encountered an error label Mar 1, 2024
Since the output of `lib.systems.elaborate` contains functions, an
equality check with `==` does not suffice, `lib.systems.equals` should
be used instead.
@jmbaur jmbaur force-pushed the nixos-cross-check branch from 7395704 to 3794246 Compare March 1, 2024 17:05
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Looks good and thank you for adding a test case 👍

@roberth roberth merged commit 55dcd06 into NixOS:master Mar 1, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Mar 1, 2024
@jmbaur jmbaur deleted the nixos-cross-check branch March 4, 2024 20:55
@cole-h cole-h removed the ofborg-internal-error Ofborg encountered an error label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Comparison of nixpkgs.hostPlatform to nixpkgs.buildPlatform is flawed

4 participants