Custom rust toolchains #667

cdmistman · 2023-06-16T07:34:45Z

Right now using custom fenix toolchains (ie, for use with rust-toolchain.toml) requires duplicating a let-bound tools list, which is inconvenient. Further, the rustup components that are installed is fixed, so eg miri can't be installed without adding it to top-level packages.

This change makes it easier to support both use-cases:

components is now an option and is used in place of the let-bound tools
languages.rust.packages has been renamed to languages.rust.toolchain to be more consistent with rustup's terminology, and now has a freeformType to allow usage of components that aren't in nixpkgs
version has been renamed to channel and is a bit more consistent with rustup's concept of a channel
the rust-src component is now properly overridden when defined in components

Here's a few examples (using fenix as an overlay):

# use a custom toolchain
languages.rust.toolchain = pkgs.fenix.latest;

# use a standard fenix toolchain
languages.rust.channel = "nightly";

# override a single component
languages.rust.toolchain.rustfmt = pkgs.fenix.latest.rustfmt;

# only use `rustc` and `cargo`
languages.rust.components = [ "rustc" "cargo" ];

…chains

cdmistman · 2023-06-16T07:40:56Z

i also considered defining toolchain instead, which would be assigned to fenix's toolchain type or a channel. This would allow for a very similar api (although would require nix-community/fenix#108):

languages.rust.toolchain = pkgs.fenix.stable;

languages.rust.toolchain = pkgs.fenix.fromManifestFile inputs.rustup-manifest;

# depends on https://github.com/nix-community/fenix/pull/108
languages.rust.toolchain = pkgs.fenix.fromToolchainName (importTOML ./rust-toolchain.toml).toolchain.channel;

let me know if a different API is desired and I can implement that instead :) Either way, making it easier to use custom toolchains would be awesome

src/modules/languages/rust.nix

domenkozar · 2023-06-18T16:42:19Z

Have you seen #640

cdmistman · 2023-06-18T19:54:37Z

i haven't! i'll note that the current implementation of that PR recreates a rust-toolchain.toml file from the rust options, which means

repos already using a rust-toolchain.toml file must recreate the toolchain spec in the devenv config
the recreated toolchain then results in an Import From Derivation

My PR addresses both; you may use a pre-existing rust-toolchain.toml file directly or avoid an IFD by adding the manifest file to the flake inputs.

If @nothingnesses would accept contributions to their PR I'd be glad to merge our work :)

cdmistman · 2023-06-18T19:59:28Z

additionally, that PR cuts out rust components from nixpkgs - if I'd known this was fine I would've done a couple things differently 😅 namely, rust.toolchain could be a fenix toolchain type

nothingnesses · 2023-06-18T21:07:15Z

Hi. I'd be happy to work together on this issue, but am not sure if there's anything much I can contribute further. I already like your solution better than mine and agree that recreating a "rust-toolchain.toml" file is undesirable and would prefer a way to just use an existing one (I couldn't figure out a way to do it without having to make a TOML parser). I think your solution would even allow using a different overlay, right?

One thing I think I'd change is the default components. I think it'd be better just to stick with the ones included in the default rustup profile, which I've done in my PR. To handle the pre-commit hooks, I just set the components to be the ones from nixpkgs, if they weren't already included in the toolchain options/part of the selected profile; I think you should be able to do something similar to that with you solution.

cdmistman · 2023-06-20T03:40:10Z

I couldn't figure out a way to do it without having to make a TOML parser

nixpkgs.lib.importTOML works and there's also builtins.fromTOML :)

I think your solution would even allow using a different overlay, right?

ah, i guess that's true! didn't think about that - yeah, https://github.com/oxalica/rust-overlay should work with this design as well

I think it'd be better just to stick with the ones included in the default rustup profile, which I've done in my PR

Hm, yeah that should work. I guess I'd like to know if a goal of devenv is to handle language server installation? @domenkozar if you can weigh in here that'd be appreciated. Additionally, I'd like to know if you have any input on the trade-off between only accepting fenix's toolchain type (which would result in a much-friendlier API here) or if I should keep the API flexible with the single options.package as defined in my PR

To handle the pre-commit hooks, I just set the components to be the ones from nixpkgs, if they weren't already included in the toolchain options/part of the selected profile; I think you should be able to do something similar to that with you solution.

yes I could, but I think it's better to use the binary from the package - this has 2 benefits:

the components aren't built 2x (in the event of no substituters)
if the user is using nightly features and is using the nightly toolchain, those will work OOTB with pre-commit-hooks.

I guess I forgot to change that part in this PR, I'll use the bin directory of the built package derivation if the bin exists there - if it doesn't, it'll be null

cdmistman · 2023-06-24T01:13:15Z

Additionally, I'd like to know if you have any input on the trade-off between only accepting fenix's toolchain type (which would result in a much-friendlier API here) or if I should keep the API flexible with the single options.package as defined in my PR

it seems gleam and ruby both have hard-dependencies on external inputs, but nonetheless I think I made a flexible API, defining both options.package and options.toolchain.

This reverts commit 8fe7d07.

domenkozar · 2023-06-29T14:58:45Z

Is it ready for review?

cdmistman · 2023-06-29T18:06:27Z

not quite yet - i tried to use this in a project but didn't have time to dig into failures. i'll have time to take care of this in a few days - i'll ping back here when it works and it's ready for review.

cdmistman · 2023-06-29T18:09:15Z

but i'll note this is one of the more complex nix modules i've written, any input on its current state/design would be appreciated :)

domenkozar · 2023-07-02T20:18:20Z

Maybe we should add that as a comment?

…

On Sun, Jul 2, 2023, 21:11 Simon Žlender ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/modules/languages/rust.nix <#667 (comment)>: > - env.RUST_SRC_PATH = cfg.packages.rust-src; - RUST_SRC_PATH is necessary when rust-src is not at the same location as as rustc. This is the case with the rust toolchain from nixpkgs. You'll run into this issue when using rust-analyzer in vscode. — Reply to this email directly, view it on GitHub <#667 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA63A5EF5Y7O3VRZL2WMT3XOHIYDANCNFSM6AAAAAAZI3XGJ4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

cdmistman · 2023-07-02T20:27:56Z

This is the case with the rust toolchain from nixpkgs. You'll run into this issue when using rust-analyzer in vscode.

I'm really tempted to just implement using fenix. Thoughts? Basically:

a few other languages in devenv don't support using their nixpkgs equivalent unless explicitly used
nixpkgs doesn't have all of rust's components
it's easier to implement using just fenix

szlend · 2023-07-06T21:26:39Z

Try with rust-std, not rust-src.

Either way though I feel like supporting cross-compilation is a bit much for this PR. I don't have experience with WASM, so maybe that's simpler. But getting other targets to work is far move involved than just installing the target rust-std. For example if you want to target x86_64-linux from aarch64-darwin, you'll need to build a cross-compiling stdenv from source, as nixpkgs doesn't have it in its binary cache.

So if someone with more experience can chip in to make a limited set of targets working, that would be great. But I don't think it should be a blocker.

src/modules/languages/rust.nix

szlend · 2023-07-06T21:33:07Z

src/modules/languages/rust.nix

+      env.RUST_SRC_PATH =
+        if cfg.toolchain ? rust-src
+        then "${cfg.toolchain.rust-src}/lib/rustlib/src/rust/library"
+        else pkgs.rustPlatform.rustLibSrc;


If it makes things easier, we only really need this for "nixpkgs" (as long as rust-src is in components when fenix is used).

we only really need this for "nixpkgs"

i don't think that's true:

enable = true; toolchain.rust-std = my-fenix-toolchain.rust-std;

Actually I just checked the docs so I may be wrong on this. Mixed it up with rust-overlay. I would leave it as is.

for clarity, when you say "leave it as is" you mean my current if-expression is fine?

It's rust-src in this instance. But yeah, I may be wrong here, sorry.

src/modules/languages/rust.nix

cdmistman · 2023-07-06T21:53:54Z

yeah, i'll leave extra targets as a non-blocker for this. if the user wants to accomplish this, they might be able to do so by either combineing the rust-std components in the toolchain configuration or adding it to packages, and then setting the appropriate linker env var

adding a targets option would be cool, but would perhaps be more involved since you'd have to map rust triples to nix systems and packages, as well as (possibly) figuring out how to get the cross-system toolchain available without re-evaluating all of nixpkgs.

i think this use-case is perhaps better solved by using devenv in a nix flake instead of as a standalone tool to only have to evaluate nixpkgs once

martinjlowm · 2023-07-06T21:58:28Z

Try with rust-std, not rust-src.

Either way though I feel like supporting cross-compilation is a bit much for this PR. I don't have experience with WASM, so maybe that's simpler. But getting other targets to work is far move involved than just installing the target rust-std. For example if you want to target x86_64-linux from aarch64-darwin, you'll need to build a cross-compiling stdenv from source, as nixpkgs doesn't have it in its binary cache.

So if someone with more experience can chip in to make a limited set of targets working, that would be great. But I don't think it should be a blocker.

I was actually hoping for this. :'( We're cross compiling against aarch64-unknown-linux-gnu on x86_64-unknown-linux-gnu in CI but for the system platform on the developer's machine. I'm not too familiar with Nix, but perhaps there's a better way?

cdmistman · 2023-07-06T22:00:19Z

We're cross compiling against aarch64-unknown-linux-gnu on x86_64-unknown-linux-gnu in CI but for the system platform on the developer's machine

sounds like you want to make it an output in a flake. look at fenix docs for an example of how to accomplish this.

alternatively, you can try my suggestions above

szlend · 2023-07-06T22:20:59Z

Like @cdmistman said, something like this might work, but I can't test cause I'd need to bootstrap gcc from source:

packages = [ <fenix>.targets.aarch64-unknown-linux-gnu.stable.rust-std ];
env.CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER =
  let
    inherit (pkgs.pkgsCross.aarch64-multiplatform.stdenv) cc;
  in
  "${cc}/bin/${cc.targetPrefix}cc";

Though this will break as soon as you have to link with any C dependencies.

szlend · 2023-07-06T22:43:54Z

LGTM, great job on this!

A bit suprised something like components = lib.mkAfter [ "rls" ]; doesn't work. I assumed it would merge with mkOptionDefault, but I guess not. Though I guess the non-default components are kind of niche anyway so it's not really a big issue.

cdmistman · 2023-07-06T22:48:32Z

I assumed it would merge with mkOptionDefault, but I guess not

yeah, I had thought the same as well - quite disappointing that components isn't easily extensible :( if someone else has an idea of how to do this it'd be greatly appreciated, but that's not a rabbit hole I'm wanting to go down 😅

domenkozar · 2023-07-07T11:12:36Z

Could you also change examples/rust to instruct how to use it now?

cdmistman · 2023-07-07T19:57:36Z

i didn't update the documentation - afaict, the docs get auto-updated but i couldn't rebuild them on my machine (aarch64-darwin, can't find libcairo). it seems GHA might rebuild them anyways

cdmistman · 2023-07-08T23:10:43Z

i have to update the PR description and want to do a little more testing. I'm about to get on a flight but I'll be able to do all of that late tonight/tomorrow morning PST (depending on how i'm feeling tonight)

cdmistman · 2023-07-10T00:02:53Z

updated the PR description and hopefully fixed failing CI (for rust - not going to fix python-poetry as it's out of scope) :)

if CI passes/no more feedback should be good to merge!

domenkozar · 2023-07-10T13:32:37Z

Could you also add rename hints for all the changed options?

lib.mkRenamedOptionModule [ "languages" "rust" "version" ] [ "languages" "rust" "channel" ])

domenkozar · 2023-07-10T21:08:44Z

Great work, thank you for the patience with reviews :)

domenkozar · 2023-07-10T21:15:09Z

I wonder if it also fixes #662 by any chance?

jhartma · 2023-07-29T18:20:11Z

Just leaving this here for anybody who wants to get devenv working with rust wasm.

in devenv.yaml

inputs:
  nixpkgs:
    url: github:NixOS/nixpkgs/nixpkgs-unstable
  fenix:
    url: github:nix-community/fenix
    inputs:
      nixpkgs:
        follows: nixpkgs
  rust-overlay:
    url: github:oxalica/rust-overlay
    overlays:
      - default

and in devenv.nix add:

  ...
  let 
    rustVersion = pkgs.rust-bin.selectLatestNightlyWith (toolchain: toolchain.default);
  in
  ...
  languages = {
    rust = {
      enable = true;
      toolchain.rustc = (rustVersion.override {
        extensions = [ "rust-src" ];
        targets = [ "wasm32-unknown-unknown" ];
      });
    };
  };
  ...

martinjlowm · 2023-07-29T22:31:32Z

Or for anyone using Flakes, Fenix and a toolchain file:

{
  inputs = {
    nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
    systems.url = "github:nix-systems/default";
    fenix.url = "github:nix-community/fenix";
  };

  outputs = { self, nixpkgs, devenv, systems, fenix, ... } @ inputs:

    let
      system = builtins.currentSystem;
      pkgs = nixpkgs.legacyPackages.${system};
      fenixpkgs = fenix.packages.${system};
      aarch64-binutils = pkgs.pkgsCross.aarch64-multiplatform.stdenv.cc;
      x86_64-binutils = pkgs.pkgsCross.gnu64.stdenv.cc;
    in
      {
        devShell.${system} = devenv.lib.mkShell {
          inherit inputs pkgs;
          modules = [
            {
              packages = [
                pkgs.libunwind
                aarch64-binutils
              ] ++ pkgs.lib.optionals pkgs.stdenv.isDarwin (with pkgs.darwin.apple_sdk; [
                frameworks.Security
                frameworks.CoreFoundation
                x86_64-binutils
              ]);

              languages.rust = {
                enable = true;
                toolchain.rustc = fenixpkgs.fromToolchainFile { dir = ./.; };
              };
            }
          ];
        };
      };
}

rust-toolchain.toml:

[toolchain]
channel = "1.70.0"
targets = ["x86_64-unknown-linux-gnu", "aarch64-unknown-linux-gnu"]

domenkozar · 2023-07-30T13:44:26Z

Maybe we could allow setting languages.rust.targets and wire everything up?

szlend · 2023-07-30T15:04:22Z

Maybe we could allow setting languages.rust.targets and wire everything up?

We could, but most targets wont work without a cross stdenv so that's a lot to wire up correctly and most of these toolchains need to be bootstrapped from source which can be hours of compiling, depending on your system. Though you can get pretty far with just cargo zigbuild: https://github.com/rust-cross/cargo-zigbuild

domenkozar · 2023-07-30T19:15:28Z

That's exactly why we should turn it into a flag so that it's not needed to go though that pain.

cdmistman added 5 commits June 14, 2023 08:51

define options

35162cf

work

1584e0d

expose components option, use package instead of packages

bc53d8d

remove work file

c413b44

Merge branch 'main' of github.com:cachix/devenv into custom-rust-tool…

7ddebe6

…chains

remove RUST_SRC_PATH...?

da9c5a8

cdmistman commented Jun 16, 2023

View reviewed changes

src/modules/languages/rust.nix Outdated Show resolved Hide resolved

cdmistman added 2 commits June 16, 2023 14:29

fix package.defaultText

8c7cf8e

fix diff

3177156

add toolchain option

4c387ad

cdmistman added 5 commits June 23, 2023 21:16

mutual exclusion

8fe7d07

optionally set pre-commit tools

96830f2

Revert "mutual exclusion"

78d45bd

This reverts commit 8fe7d07.

further fix pre-commit tools

499e602

lazyAttrsOf fenix toolchain type

b541696

nothingnesses mentioned this pull request Jun 24, 2023

Make rust.nix accept toolchain options. #640

Closed

cdmistman added 2 commits July 4, 2023 18:20

temporary - define using only fenix

ef4e235

profile doesnt exist on toolchains :(

36c4386

szlend reviewed Jul 6, 2023

View reviewed changes

src/modules/languages/rust.nix Outdated Show resolved Hide resolved

szlend reviewed Jul 6, 2023

View reviewed changes

src/modules/languages/rust.nix Show resolved Hide resolved

fix mkIf channel

e2ac9a7

szlend approved these changes Jul 6, 2023

View reviewed changes

update rust example

4d6573c

version -> channel

d9e5c07

mkDefault for pre-commit tools

3477dee

add renamed hints

85b5835

domenkozar merged commit fab51e8 into cachix:main Jul 10, 2023
106 of 142 checks passed

cdmistman mentioned this pull request Jul 10, 2023

Rust: error: 'cargo-clippy' is not installed for the toolchain 'nightly-x86_64-apple-darwin' but is available on the path #662

Open

Custom rust toolchains #667

Custom rust toolchains #667

Conversation

cdmistman commented Jun 16, 2023 • edited

cdmistman commented Jun 16, 2023 • edited

domenkozar commented Jun 18, 2023

cdmistman commented Jun 18, 2023

cdmistman commented Jun 18, 2023 • edited

nothingnesses commented Jun 18, 2023 • edited

cdmistman commented Jun 20, 2023 • edited

cdmistman commented Jun 24, 2023

domenkozar commented Jun 29, 2023

cdmistman commented Jun 29, 2023

cdmistman commented Jun 29, 2023

domenkozar commented Jul 2, 2023 via email

cdmistman commented Jul 2, 2023

szlend commented Jul 6, 2023 • edited

szlend Jul 6, 2023 • edited

Choose a reason for hiding this comment

cdmistman Jul 6, 2023

Choose a reason for hiding this comment

szlend Jul 6, 2023

Choose a reason for hiding this comment

cdmistman Jul 6, 2023

Choose a reason for hiding this comment

szlend Jul 6, 2023

Choose a reason for hiding this comment

cdmistman commented Jul 6, 2023 • edited

martinjlowm commented Jul 6, 2023 • edited

cdmistman commented Jul 6, 2023 • edited

szlend commented Jul 6, 2023

szlend commented Jul 6, 2023 • edited

cdmistman commented Jul 6, 2023

domenkozar commented Jul 7, 2023

cdmistman commented Jul 7, 2023

cdmistman commented Jul 8, 2023

cdmistman commented Jul 10, 2023 • edited

domenkozar commented Jul 10, 2023

domenkozar commented Jul 10, 2023

domenkozar commented Jul 10, 2023

jhartma commented Jul 29, 2023 • edited

martinjlowm commented Jul 29, 2023

domenkozar commented Jul 30, 2023

szlend commented Jul 30, 2023

domenkozar commented Jul 30, 2023

cdmistman commented Jun 16, 2023 •

edited

cdmistman commented Jun 16, 2023 •

edited

cdmistman commented Jun 18, 2023 •

edited

nothingnesses commented Jun 18, 2023 •

edited

cdmistman commented Jun 20, 2023 •

edited

szlend commented Jul 6, 2023 •

edited

szlend Jul 6, 2023 •

edited

cdmistman commented Jul 6, 2023 •

edited

martinjlowm commented Jul 6, 2023 •

edited

cdmistman commented Jul 6, 2023 •

edited

szlend commented Jul 6, 2023 •

edited

cdmistman commented Jul 10, 2023 •

edited

jhartma commented Jul 29, 2023 •

edited