Support .rust-toolchain.toml

[ojeda]

Problem you are trying to solve

In the Linux kernel, we would have liked to take advantage of rustup's rust-toolchain.toml feature to simplify our Quick Start instructions (see Rust-for-Linux/linux#953) back when we needed to pin our compiler version.

However, it appears that there is no support for a hidden version of that configuration file, i.e. .rust-toolchain.toml, which would mean it is very visible when listing the root directory of the kernel sources.

However, most kernel developers did not use Rust, and even for those that did, the file is not really relevant: it would have very rarely changed and if they needed to change the toolchain version, they should have used a directory override in any case, rather than modifying the file.

Thus it is really not needed to show it all the time in listings, and so a hidden .rust-toolchain.toml file would be better. I guess for other projects that may be the case as well.

This would follow other files like .clang-format, .gitignore, .gitattributes, .mailmap, .rustfmt.toml, etc. where configuration is not directly related to the project (say, a Kconfig file) but to tooling.

Solution you'd like

Support for the .rust-toolchain.toml filename.

[rbtcollins]

Its not clear to me that rust-toolchain does what you want in that ticket.

You're not using a custom built toolchain, and you have developers using newer toolchains (and thus the expectation) that newer toolchains should work. Have you considered setting rust-version in your Cargo.toml to require a known minimum version and then lean on Rust's compatability guarantees, as an alternate approach?

rust-toolchain{,.toml} files can cause arbitrary code execution, and IDEs are looking to disable toolchain file support. In that context I don't think a hidden file is a good idea.

[TheAlgorythm]

Have you considered setting rust-version in your Cargo.toml to require a known minimum version and then lean on Rust's compatability guarantees, as an alternate approach?

I don't think, that this will work for the kernel, as it requires a specific version of rustc if I'm not mistaken.
Further rust-toolchain.toml can also specify components which isn't possible in Cargo.toml.

[ojeda]

You're not using a custom built toolchain, and you have developers using newer toolchains (and thus the expectation) that newer toolchains should work

No, sorry. The kernel uses a bunch of unstable Rust features, and thus we are pinned to a particular version so far.

Sometimes kernel developers may use a newer version for a bunch of reasons. That is OK and should still work by e.g. a directory override (if rustup was used) or a custom $RUSTC, etc.

Have you considered setting rust-version in your Cargo.toml to require a known minimum version and then lean on Rust's compatability guarantees, as an alternate approach?

The kernel does not use Cargo. Even if it did, Cargo is orthogonal to what a potential MSRV could be.

rust-toolchain{,.toml} files can cause arbitrary code execution, and IDEs are looking to disable toolchain file support. In that context I don't think a hidden file is a good idea.

Those issues seem to be related to untrusted contents/repos. The kernel tree is trusted -- if someone can arbitrarily modify the files in your kernel tree, then you already have a myriad of ways to execute code in the developer's computer as soon as e.g. make is called.

And, in any case, fixing that is on the side of tooling, by providing a way for users to disable all "smart" features when dealing with untrusted repositories. I don't see how adding or removing a file in any given repository fixes anything if the context is untrusted repositories to begin with.

[nbdd0121]

rust-toolchain{,.toml} files can cause arbitrary code execution, and IDEs are looking to disable toolchain file support. In that context I don't think a hidden file is a good idea.

.cargo/config is hidden, and you can use that to specify a custom linker. I think these issues and a hidden rust-toolchain.toml file are orthogonal.

For Rust-for-Linux use case, apart from the tree being assumed to be trusted, we also currently only specify an official release version.

[rbtcollins]

I've closed the PR, see the comment in there before I did that.

I think this is a broad design choice question, rather than a simple make-the-change and execute. An RFC is the general mechanism for doing that, but the rustup team is quite stretched at the moment - and while I'd be delighted to help more folk contribute and beef up the team's capacity and resilience, I can think of a dozen more pressing things - such as concurrent component add safety, dealing with the docker layer problem gracefully, moving to async for concurrent download-and-compact, signing and generate security improvements - as things that would benefit a large number of users straight away.

[ojeda]

If you mean there is no time to write an RFC, then we can do so on our own -- that should not be a problem.

The issue itself is not critical for us, and I understand it is not a priority for you either, but it would be nice to eventually see it happen, so that we can start using the toolchain file in the kernel.

[rbtcollins]

I meant that bandwidth to review and think through an RFC isn't really present at the moment. I wouldn't like to see you in the position that some of the cargo RFC's (like the xdg dirs one) have ended up in, where there is lots of interest, but not from the folk doing the work. I do appreciate that you're happy to write the code, but a chunk of the work is not related to the code but the overall impact, supporting users of the tool etc.

[TheAlgorythm]

Therefore I asked what the common process (like a RFC) for such a change is.

And as there was no process related criticism and the addition of rust-toolchain.toml was done without a RFC, I thought it would be fine.

If security is the rationale, this could help as the hidden toolchain file doesn't need backwards compatibility. Therefore it could be stricter such as disallowing path. This would also solve the problem of IDEs completely disabling the toolchain file as there is a "secure" version.

But if I understood you correctly there isn't enough review capacity and therefore it wouldn't help if I write a RFC.

[kinnison]

I realise this is fairly late into your planning; but, if you do not invoke cargo/rustc directly but rather as part of the general Kbuild process, then why not simply set RUST_TOOLCHAIN in the environment you export? If you need to have particular components installed then there's currently no ecosystem wide way to ensure that anyway since locally installed toolchains may or may not have them (eg. if someone dnf install's rust they probably won't get clippy or all the OS targets anyway) so you likely will need some tooling around probing for target support.

Currently there are two files which present toolchain information (rust-toolchain and rust-toolchain.toml) if we add a third then not only is there documentation/ongoing-support to consider; but also things like priority of content, etc. Should a .rust-toolchain.toml override rust-toolchain.toml or should it be overridden? etc.

As Robert says, there's limited bandwidth for actually thinking all of these things through with Rustup, so it may be worthwhile looking at working within the Kbuild makefiles if at all possible.

[ojeda]

I realise this is fairly late into your planning; but, if you do not invoke cargo/rustc directly but rather as part of the general Kbuild process [...] it may be worthwhile looking at working within the Kbuild makefiles if at all possible.

There is no problem changing things within Kbuild on our side, so if you think there would be other/better solutions, we are happy to hear them!

In general, having rustc work the same way as within the build system would be nice to avoid surprises.

If you need to have particular components installed [...] so you likely will need some tooling around probing for target support.

Yes, e.g. rust-src. The system already checks and guides the user for that (and other cases), though, so that should not be a problem (and can also be changed to e.g. add more checks, if a solution required it).

Currently there are two files which present toolchain information (rust-toolchain and rust-toolchain.toml) if we add a third then not only is there documentation/ongoing-support to consider; but also things like priority of content, etc. Should a .rust-toolchain.toml override rust-toolchain.toml or should it be overridden? etc.

Yeah, the overall decision is not trivial (and perhaps asking some projects/users on their sentiment would help, but of course that takes time too).

However, I think the question around precedence is easy to answer, since there is precedent: the same decision should be taken as it was done for rust-toolchain vs. rust-toolchain.toml. That is, the new file would be last. From commit 8d6ebff that introduced the second file: "If both rust-toolchain and rust-toolchain.toml are present in a folder, rustup prints a warning and uses rust-toolchain."

I requested @TheAlgorythm to document that behavior in the docs in the same PR: https://github.com/rust-lang/rustup/pull/3195/files#diff-6732b5488956df6f883ef27333b945d647c0ef920013aa9af1e7534f0202f779R81-R86

The PR adds some tests on the diagnostics as the existing ones for the other files, though I think a check on the precedence would be nice too, not sure if there is one for the existing 2 files. If not, adding one could be a prerequisite patch!

And thanks for your time either way!

[TheAlgorythm]

The PR adds some tests on the diagnostics as the existing ones for the other files, though I think a check on the precedence would be nice too, not sure if there is one for the existing 2 files. If not, adding one could be a prerequisite patch!

This is already tested.

[ojeda]

I may be missing something, but which of those tests does so?

[TheAlgorythm]

I hope i understood you correctly. But in https://github.com/TheAlgorythm/rustup/blob/hidden_toolchain_toml/tests/cli-rustup.rs#L2288-L2377 the first marked test was already there and it checks the used file.

But now I see a problem: The precedence is only checked via stderr and not via behaviour.

[ojeda]

The precedence is only checked via stderr and not via behaviour.

Yeah, exactly, that is what I meant -- we check the diagnostics, but we don't check that the right file is actually used (I didn't look too closely if the existing tests do so for the 2 files, but I didn't see it from a quick glance).

I guess it is not too important, because users will notice the warnings and fix it anyway, but it would be nice.

[daira]

@nbdd0121 wrote:

.cargo/config is hidden, and you can use that to specify a custom linker.

I'd consider that a design error. It is certainly a hazard. I wish .cargo had used a non-hidden name.

In any case it isn't a good argument for adding more cases of hidden configuration than already exist, especially when what you want seems easily achievable using RUST_TOOLCHAIN as @kinnison pointed out.