Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rustc: use targetPlatform to detect pkgsLLVM, unbreak with LLVM and musl #379632

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

getchoo
Copy link
Member

@getchoo getchoo commented Feb 5, 2025

Pseudo-alternative to #330037

Following #376988, it seems pkgsLLVM.pkgsMusl and similar combinations work as expected! This has led me to do a bit of testing based on this comment from a Rust developer on using LLVM/libunwind:

Systems without a GNU userland shouldn't use the *-linux-unknown-gnu toolchains, which explicitly require a GNU userland. Your supposed to use *-linux-unknown-musl.

Sure enough after a little tweaking, pkgsLLVM.pkgsMusl.rustc appears to be fully functional :)

Tested against #375030 and with the following:

> nix build -f . pkgsLLVM.pkgsMusl.rustc.unwrapped.tests.{fd.tests,ripgrep} --print-out-paths
/nix/store/mlbxa62xnyq1sbdvhnxxvsaspcgpbql7-fd-x86_64-unknown-linux-musl-10.2.0-test-version
/nix/store/axbmix7qmp670xllvny8w9s4xiyfw46f-ripgrep-x86_64-unknown-linux-musl-14.1.1

We should of course continue looking for ways to get pkgsLLVM alone to build rustc properly, but hopefully this can hold us over until a solution is found

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

Previously `pkgsLLVM.rustc` was correctly flagged, but not the spliced
wrapper in dependencies like `pkgsLLVM.ripgrep` for example
This is something that is not supported by upstream, no longer used in
other distributions like Gentoo (where it was originally sourced from),
doesn't fix issues with compilation at runtime, and only works around
the root problem of Rust wanting to use `libgcc_s` when it's not
available

Let's remove it
@getchoo getchoo added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label Feb 5, 2025
@pwaller
Copy link
Contributor

pwaller commented Feb 5, 2025

Heads up the requisite has been reverted in #379615. Hopefully it gets fixed and relanded before too long.

@wolfgangwalther
Copy link
Contributor

Heads up the requisite has been reverted in #379615. Hopefully it gets fixed and relanded before too long.

Unfortunately, it's impossible to land this soon, it requires a deprecation phase on the NixOS side which imho can't be avoided and can't be worked around. So.. don't hold you breath - I have put it on hold for me for now :(

@getchoo getchoo marked this pull request as draft February 6, 2025 02:52
@getchoo
Copy link
Member Author

getchoo commented Feb 6, 2025

That is sad to hear. The composition introduced in that PR was honestly a game changer in solving this problem

I'll see if we can get the same logic working OOTB with pkgsLLVM in the meantime ;)

@alyssais
Copy link
Member

alyssais commented Feb 6, 2025

The composition introduced in that PR was honestly a game changer in solving this problem

Is it not just sugar for setting { useLLVM = true; config = "x86_64-unknown-linux-musl"; } or whatever?

@getchoo
Copy link
Member Author

getchoo commented Feb 6, 2025

Basically lol, I was just having some trouble with trying to extend the current stdenv with some of those variables -- it's the same as what happens with packages evaluated from pkgsLLVM.pkgsMusl now AFAICT

This PR will continue to work with

import <nixpkgs> {
  localSystem.system = "x86_64-linux";
  crossSystem = {
    config = "x86_64-unknown-linux-musl";
    useLLVM = true;
    linker = "lld";
  };
  config = { };
  overlays = [ ];
}

But I'm not sure how we can make this buildable again directly 🤷

@wolfgangwalther
Copy link
Contributor

The composition introduced in that PR was honestly a game changer in solving this problem

Is it not just sugar for setting { useLLVM = true; config = "x86_64-unknown-linux-musl"; } or whatever?

Essentially yes. I guess the most value those predefined package sets bring is.. the fact that they encode this knowledge and thus you're more likely to find something that doesn't immediately break everything. Similar to lib/systems/examples.nix, but for non-cross. So yeah, convenience.

@wolfgangwalther
Copy link
Contributor

That is sad to hear. The composition introduced in that PR was honestly a game changer in solving this problem

We probably found a way without touching NixOS and hopefully without lengthy deprecation phases.

Can you try this on top of #380342?

@emilazy
Copy link
Member

emilazy commented Feb 9, 2025

I think this PR is based on somewhat of a misconception. The linked comment reads:

Systems without a GNU userland shouldn't use the *-linux-unknown-gnu toolchains, which explicitly require a GNU userland. Your supposed to use *-linux-unknown-musl.

But this is in the context of someone using a (Musl‐based) Alpine system, and LLVM is only mentioned in the context of Gentoo’s llvm-musl profile. The intended meaning is not “systems without [a GCC‐based compilation toolchain]”, but “systems without [a glibc‐based runtime]”. (Following discussion reveals that the reporter was actually using *-linux-unknown-musl and the whole thing was a miscommunication.)

pkgsLLVM should be largely orthogonal to glibc vs. Musl. (I suspect the reason it’s working with Musl is due to Rust’s conflation of Musl with static‐linking‐by‐default or something?) I don’t think it’s the case that Rust only supports Musl when using an LLVM‐based compilation toolchain, and that wouldn’t make much sense. I think that the correct alternative to this and #330037 would be to pass the appropriate target flags to make Rust use libunwind directly, or maybe use a custom target definition if necessary. At most maybe the Rust standard library needs patching for incorrect assumptions about the unwinder library? However, I do think that this PR is an improvement over the status quo apart from the inaccurate comment, since the symlink is a terrible hack that we shouldn’t need.

I think that the more general solution is probably going to be packaging llvm-libgcc, though that shouldn’t be necessary for Rust. As the linked documentation says, glibc makes assumptions about libgcc, so replacing libgcc with compiler-rt + libunwind while using glibc works best with a libgcc shim. (But it’s possible this is out of date or I’m misunderstanding things.)

cc @RossComputerGuy

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 6.topic: rust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants