lib.systems.elaborate: fix passing `rust` (more) by alyssais · Pull Request #271707 · NixOS/nixpkgs

alyssais · 2023-12-02T16:19:15Z

Description of changes

An important idea around the rust stuff in lib.systems is that it's elaborated — this means that it should idempotently add to the values passed in, if any. But we missed that the names used for the parameter and the elaborated value for "rustcTarget"/"config" didn't line up. The intention was to use "rustcTarget" everywhere in the new interface, as a more descriptive name than "config".

This fixes setting the system in NixOS configuration, which results in an already elaborated system being elaborated again. Before, this wouldn't produce the correct result:

% nix-instantiate --eval -A stdenv.hostPlatform.rust.rustcTarget --system armv7l-linux
"armv7-unknown-linux-gnueabihf"
% NIX_PATH= nix-instantiate --eval -E '(import nixos/lib/eval-config.nix { system = "armv7l-linux"; modules = []; }).pkgs.stdenv.hostPlatform.rust.rustcTarget'
"arm-unknown-linux-gnueabihf"

Fixes: #271000

Very technically, this is a backwards-incompatible change, as for about a couple of weeks it was accidentally possible to pass rust.config (as opposed to the old interface, rustc.config, which has been supported for years and continues to be until it's eventually removed after a deprecation period). This was unintentional, undocumented (the documentation still points people towards the old interface for now), only possible for a couple of weeks, and there was no reason to do it. If we really had to, we could do a proper deprecation period for the accidentally rust.config as well, but I don't think there's any reason to, and it would extend this whole project by six more months. But if the RMs really think we have to, we can.

Things done

Priorities

Add a 👍 reaction to pull requests you find important.

yu-re-ka

This fixes the build of an armv7l cross NixOS for me.
Thanks!

RaitoBezarius · 2023-12-02T16:34:34Z

We should check out of tree Rust builders to see what they do regarding that.

alyssais · 2023-12-02T16:39:04Z

We should check out of tree Rust builders to see what they do regarding that.

Regarding what exactly?

misuzu

LGTM! Fixes both armv7l & riscv64 issue for me

RaitoBezarius · 2023-12-02T16:44:57Z

We should check out of tree Rust builders to see what they do regarding that.

Regarding what exactly?

Regarding whether they pass rust.config or rustc.config if they setup cross compilation.

alyssais · 2023-12-02T16:57:35Z

Regarding whether they pass rust.config or rustc.config if they setup cross compilation.

If they pass rustc.config, everything is fine. If they pass rust.config, they'd have to have started doing that since 2023-11-16, when it made it into master.

Which out-of-tree things do you think we should be checking?

RaitoBezarius · 2023-12-02T18:04:50Z

Regarding whether they pass rust.config or rustc.config if they setup cross compilation.

If they pass rustc.config, everything is fine. If they pass rust.config, they'd have to have started doing that since 2023-11-16, when it made it into master.

Which out-of-tree things do you think we should be checking?

crane, cargo2nix, naersk (?) are the ones on the top of my mind.

alyssais · 2023-12-02T18:31:31Z

cargo2nix and naersk haven't had any changes since rust.config made it into Nixpkgs master, and I don't see anything relevant in the few commits crane has had in that time period.

RaitoBezarius · 2023-12-02T19:18:34Z

In this case, this breaking change is exceptionally allowed because of no "popular" out-of-tree consumers maybe depending upon it.

An important idea around the rust stuff in lib.systems is that it's elaborated — this means that it should idempotently add to the values passed in, if any. But we missed that the names used for the parameter and the elaborated value for "rustcTarget"/"config" didn't line up. The intention was to use "rustcTarget" everywhere in the new interface, as a more descriptive name than "config". This fixes setting the system in NixOS configuration, which results in an already elaborated system being elaborated again. Before, this wouldn't produce the correct result: % nix-instantiate --eval -A stdenv.hostPlatform.rust.rustcTarget --system armv7l-linux "armv7-unknown-linux-gnueabihf" % NIX_PATH= nix-instantiate --eval -E '(import nixos/lib/eval-config.nix { system = "armv7l-linux"; modules = []; }).pkgs.stdenv.hostPlatform.rust.rustcTarget' "arm-unknown-linux-gnueabihf" Fixes: e3e57b8 ("lib.systems: elaborate Rust metadata") Fixes: NixOS#271000

github-actions · 2023-12-03T00:32:33Z

Successfully created backport PR for release-23.11:

[Backport release-23.11] lib.systems.elaborate: fix passing rust (more) #271790

andresilva · 2023-12-04T11:48:09Z

This change broke rustc-wasm32. Here's the error while trying to build rustc:

rustc> thread 'main' panicked at 'Target "wasm32-unknown-wasi" does not have a "wasi-root" key', compile.rs:295:17
rustc> note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
rustc> Build completed unsuccessfully in 0:00:11
rustc> make: *** [Makefile:12: all] Error 1

alyssais · 2023-12-04T11:55:10Z

Ah yes, whoops, we had an in-tree use of rust.config.

rust.config was never intended to exist, and has been removed. Link: NixOS#271707 (comment) Fixes: 9731208 ("lib.systems.elaborate: fix passing `rust` (more) (NixOS#271707)")

rust.config was never intended to exist, and has been removed. Link: #271707 (comment) Fixes: 9731208 ("lib.systems.elaborate: fix passing `rust` (more) (#271707)") (cherry picked from commit a449e06)

rust.config was never intended to exist, and has been removed. Link: NixOS#271707 (comment) Fixes: 9731208 ("lib.systems.elaborate: fix passing `rust` (more) (NixOS#271707)")

context: NixOS/nixpkgs@e3e57b8 NixOS/nixpkgs#271707

`rustc.config` is called `rust.rustcTarget` now, and `{rustc -> rust}.platform`. This is the new way (tm), and is preferred since NixOS#271707 - though the documentation still is outdated, and some expressions in nixpkgs were using the old interface. This updates both.

flokli · 2024-06-13T18:20:05Z

I updated the docs and remaining references to rustc in #319624.

`rustc.config` is called `rust.rustcTarget` now, and `{rustc -> rust}.platform`. This is the new way (tm), and is preferred since NixOS/nixpkgs#271707 - though the documentation still is outdated, and some expressions in nixpkgs were using the old interface. This updates both.

context: NixOS/nixpkgs@e3e57b8 NixOS/nixpkgs#271707

alyssais added the backport release-23.11 label Dec 2, 2023

alyssais requested review from a team, misuzu and yu-re-ka December 2, 2023 16:19

alyssais requested a review from Ericson2314 as a code owner December 2, 2023 16:19

alyssais requested a review from a user December 2, 2023 16:19

github-actions bot added the 6.topic: lib The Nixpkgs function library label Dec 2, 2023

yu-re-ka approved these changes Dec 2, 2023

View reviewed changes

delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 2, 2023

misuzu approved these changes Dec 2, 2023

View reviewed changes

delroth added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Dec 2, 2023

alyssais force-pushed the rustcTarget branch from b90bf66 to 4d23987 Compare December 2, 2023 20:57

delroth removed the 12.approvals: 2 This PR was reviewed and approved by two persons. label Dec 2, 2023

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 Dec 2, 2023

yu-re-ka merged commit 9731208 into NixOS:master Dec 3, 2023

github-actions bot mentioned this pull request Dec 3, 2023

[Backport release-23.11] lib.systems.elaborate: fix passing rust (more) #271790

Merged

1 task

alyssais deleted the rustcTarget branch December 4, 2023 11:55

alyssais mentioned this pull request Dec 4, 2023

rustc-wasm32: fix build #272024

Merged

13 tasks

mikatammi mentioned this pull request Dec 6, 2023

Switch to NixOS 23.11 tiiuae/ghaf#401

Merged

13 tasks

Kranzes added a commit to Kranzes/crate2nix that referenced this pull request Jun 11, 2024

rust.lib -> stdenv.hostPlatform.rust

ea9f0af

context: NixOS/nixpkgs@e3e57b8 NixOS/nixpkgs#271707

flokli mentioned this pull request Jun 13, 2024

config.rust: update references from rustc #319624

Merged

13 tasks

Kranzes added a commit to Kranzes/crate2nix that referenced this pull request Jun 16, 2024

rust.lib -> stdenv.hostPlatform.rust

e6797b9

context: NixOS/nixpkgs@e3e57b8 NixOS/nixpkgs#271707

SFrijters mentioned this pull request Jun 20, 2024

Incorrect linker flags passed to Rust riscv32imc-unknown-none-elf cross-compiler #281527

Open

Uh oh!

Conversation

alyssais commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Priorities

Uh oh!

yu-re-ka left a comment

Choose a reason for hiding this comment

Uh oh!

RaitoBezarius commented Dec 2, 2023

Uh oh!

alyssais commented Dec 2, 2023

Uh oh!

misuzu left a comment

Choose a reason for hiding this comment

Uh oh!

RaitoBezarius commented Dec 2, 2023

Uh oh!

alyssais commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RaitoBezarius commented Dec 2, 2023

Uh oh!

alyssais commented Dec 2, 2023

Uh oh!

RaitoBezarius commented Dec 2, 2023

Uh oh!

github-actions bot commented Dec 3, 2023

Uh oh!

andresilva commented Dec 4, 2023

Uh oh!

alyssais commented Dec 4, 2023

Uh oh!

flokli commented Jun 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

alyssais commented Dec 2, 2023 •

edited

Loading

alyssais commented Dec 2, 2023 •

edited

Loading