stdenv Rust fixes by alyssais · Pull Request #268168 · NixOS/nixpkgs

alyssais · 2023-11-17T20:01:25Z

Description of changes

See commit messages. Fixes a couple of regressions from #233628.

wasm32-rustc is still not ideal. As followup, I'd like to investigate whether we could just always include the wasm32-unknown-unknown target in rustc builds, since it doesn't require any libc dependency or anything.

Things done

Ericson2314

Yeah agreed this seems strictly better.

andresilva · 2023-11-17T22:04:53Z

FWIW this works fine with a package that I maintain that depends on rustc-wasm32. The wasi config threw me off at first until I read the commit message, maybe adding a small comment there is warranted.

pbsds · 2023-11-17T23:03:32Z

took a while to build, but pagefind functions as expected

alyssais · 2023-11-18T09:55:49Z

@Ericson2314 can I get your ack for dropping the assertion that at most one of "rust" and "rustc" is given? Caused eval to fail; reason now explained in the first commit message.

ghost · 2023-11-19T04:34:23Z

lib/systems/default.nix

Suggested change

};

} // (args.rust or {});

Otherwise people have no way to override any of this stuff without editing this file.

Previously // args was the last thing in the //-chain, which provided an escape hatch.

I don't think that works. If you pass { rust.platform = { arch = …; os = …; }; }, target-family is supposed to be filled in automatically. If we did a merge like this at the end, the whole platform attrset would be overridden with just the values you passed, so target-family wouldn't end up being there.

I'm also not confident that this needs to be supported, at least not right now. It was never supported when these were in rust.lib, except by using an overlay, which you could also use in this situation. It would be a bit ugly, but making workarounds for things be ugly when they're fixable bugs in the underlying code is not the worst thing, because it encourages fixing the thing rather than working around it.

ghost · 2023-11-19T04:35:43Z

@ofborg eval

Usually, attributes passed explicitly to elaborate take precedence over the elaborated ones, but since we also elaborate the nested "rust" attrset, we need to push that one level down, so the rest of "rust" is still filled in if you just pass { rust = { config = ... } }. I've had to drop the assertion that checked that at most one of "rust" and "rustc" was part of the un-elaborated system, because doing this broke passing an elaborated system in, which should be idempotent. For the same reason, I've also had to make it possible for rust.rustcTargetSpec to be passed in. Otherwise, on the second call, since platform was filled in by the first, the custom target file would be constructed. The only other way to avoid this would be to compare the platform attrs to all built in Rust targets to check it wasn't one of those, and that isn't feasible. Fixes: e3e57b8 ("lib.systems: elaborate Rust metadata")

The previous version stopped working when we started elaborating Rust metadata. Here, I've made it a bit nicer by actually setting targetPlatform to an elaborated system. Setting the config to wasi to get elaborate to understand it is a bit of a hack, but I think it's less of a hack than what we had before. The only actual difference this makes to the rustc-wasm32 derivation compared to the previous working version, is that now crt-static is set. This is probably the right thing anyway. Fixes: e3e57b8 ("lib.systems: elaborate Rust metadata")

alyssais · 2023-11-19T10:08:37Z

@Ericson2314 can I get your ack for dropping the assertion that at most one of "rust" and "rustc" is given? Caused eval to fail; reason now explained in the first commit message.

Also had to make it possible to pass in rustcTargetSpec for similar reasons, also now explained in commit message.

emilylange · 2023-11-21T16:51:32Z

Pardon the question, but what is left to get this merged?
Given this neatly resolves the currently present breakage of pagefind, polkadot and lldap :)

alyssais · 2023-11-21T17:00:13Z

I would still like @Ericson2314's ack for the changes I mentioned.

bendlas

I'm currently running this patchset to fix lldap for my deployments.

andresilva · 2023-11-23T22:15:35Z

IMO we should merge this as-is. It's strictly better than the current situation, since there are broken packages depending on this fix, and if @Ericson2314 has any concerns they can be addressed in a follow-up PR.

github-actions · 2023-11-24T11:22:03Z

Successfully created backport PR for release-23.11:

[Backport release-23.11] stdenv Rust fixes #269615

misuzu · 2023-12-02T08:53:23Z

This change broke rustc on armv7l-linux and riscv64-linux when evaluating with NixOS system:

62f7a6dcc1a2baf50e757a6a0bce1083692e0e56 is the first bad commit
commit 62f7a6dcc1a2baf50e757a6a0bce1083692e0e56
Author: Alyssa Ross <[email protected]>
Date:   Fri Nov 17 20:16:22 2023 +0100

    lib.systems.elaborate: fix passing `rust`

    Usually, attributes passed explicitly to elaborate take precedence
    over the elaborated ones, but since we also elaborate the nested
    "rust" attrset, we need to push that one level down, so the rest of
    "rust" is still filled in if you just pass
    { rust = { config = ... } }.

    I've had to drop the assertion that checked that at most one of "rust"
    and "rustc" was part of the un-elaborated system, because doing this
    broke passing an elaborated system in, which should be idempotent.

    For the same reason, I've also had to make it possible for
    rust.rustcTargetSpec to be passed in.  Otherwise, on the second call,
    since platform was filled in by the first, the custom target file
    would be constructed.  The only other way to avoid this would be to
    compare the platform attrs to all built in Rust targets to check it
    wasn't one of those, and that isn't feasible.

    Fixes: e3e57b8f1885 ("lib.systems: elaborate Rust metadata")

 lib/systems/default.nix | 184 ++++++++++++++++++++++++------------------------
 1 file changed, 92 insertions(+), 92 deletions(-)

Used commands:

git bisect start
git bisect good 19cbff5
git bisect bad e92039b55bcd
git bisect run nix-build nixos -A pkgs.rustc --argstr system riscv64-linux -I nixos-config=test.nix --dry-run

test.nix looks like this:

{
  boot.loader.grub.device = "/dev/sda";
  fileSystems."/".device = "/dev/sda1";
  services.sshd.enable = true;
}

See #271000 for more info

alyssais added 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: stdenv Standard environment labels Nov 17, 2023

alyssais requested review from andresilva and pbsds November 17, 2023 20:01

alyssais requested a review from Ericson2314 as a code owner November 17, 2023 20:01

alyssais requested a review from a user November 17, 2023 20:01

github-actions bot added 6.topic: lib The Nixpkgs function library and removed 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: stdenv Standard environment labels Nov 17, 2023

alyssais force-pushed the rustc-wasm32 branch from d4fb336 to 905f288 Compare November 17, 2023 20:03

alyssais requested a review from RaitoBezarius as a code owner November 17, 2023 20:03

github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Nov 17, 2023

alyssais changed the title ~~[staging-next] stdenv Rust fixes~~ stdenv Rust fixes Nov 17, 2023

alyssais changed the base branch from staging-next to master November 17, 2023 20:03

github-actions bot removed the 6.topic: python Python is a high-level, general-purpose programming language. label Nov 17, 2023

alyssais removed the request for review from RaitoBezarius November 17, 2023 20:03

alyssais mentioned this pull request Nov 17, 2023

mesa: fix cross compilation to x86_64 #233628

Merged

12 tasks

Ericson2314 approved these changes Nov 17, 2023

View reviewed changes

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

alyssais force-pushed the rustc-wasm32 branch from 905f288 to 3a38b49 Compare November 17, 2023 22:10

delroth removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 17, 2023

pbsds approved these changes Nov 17, 2023

View reviewed changes

delroth added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Nov 17, 2023

alyssais force-pushed the rustc-wasm32 branch from 3a38b49 to 3df2182 Compare November 18, 2023 09:54

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

ghost reviewed Nov 19, 2023

View reviewed changes

alyssais force-pushed the rustc-wasm32 branch from 3df2182 to 6846d96 Compare November 19, 2023 10:06

alyssais added 2 commits November 19, 2023 11:06

alyssais force-pushed the rustc-wasm32 branch from 6846d96 to 886c892 Compare November 19, 2023 10:06

emilylange mentioned this pull request Nov 19, 2023

Build failure: lldap #268571

Closed

emilylange linked an issue Nov 19, 2023 that may be closed by this pull request

Build failure: lldap #268571

Closed

andresilva approved these changes Nov 19, 2023

View reviewed changes

delroth added the 12.approvals: 3+ This PR was reviewed and approved by three or more persons. label Nov 19, 2023

emilylange added the backport release-23.11 label Nov 21, 2023

bendlas approved these changes Nov 23, 2023

View reviewed changes

This comment was marked as off-topic.

Sign in to view

alyssais merged commit c9b74e4 into NixOS:master Nov 24, 2023

alyssais deleted the rustc-wasm32 branch November 24, 2023 11:21

github-actions bot mentioned this pull request Nov 24, 2023

[Backport release-23.11] stdenv Rust fixes #269615

Merged

1 task

pbsds mentioned this pull request Nov 24, 2023

pagefind: 1.0.3 -> 1.0.4 #267950

Merged

13 tasks

misuzu mentioned this pull request Nov 29, 2023

Different stdenv.hostPlatform.rust.rustcTarget with and without NixOS system #271000

Closed

Uh oh!

Conversation

alyssais commented Nov 17, 2023

Description of changes

Things done

Uh oh!

Ericson2314 left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva commented Nov 17, 2023

Uh oh!

pbsds commented Nov 17, 2023

Uh oh!

alyssais commented Nov 18, 2023

Uh oh!

ghost Nov 19, 2023

Choose a reason for hiding this comment

Uh oh!

alyssais Nov 19, 2023

Choose a reason for hiding this comment

Uh oh!

alyssais Nov 19, 2023

Choose a reason for hiding this comment

Uh oh!

ghost commented Nov 19, 2023

Uh oh!

alyssais commented Nov 19, 2023

Uh oh!

emilylange commented Nov 21, 2023

Uh oh!

alyssais commented Nov 21, 2023

Uh oh!

bendlas left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

github-actions bot commented Nov 24, 2023

Uh oh!

misuzu commented Dec 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

andresilva commented Nov 23, 2023 •

edited

Loading