Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Set environment variables in rust-toolchain.toml #2883

Open
jonhoo opened this issue Oct 28, 2021 · 3 comments · May be fixed by #2884
Open

Set environment variables in rust-toolchain.toml #2883

jonhoo opened this issue Oct 28, 2021 · 3 comments · May be fixed by #2884

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Oct 28, 2021

Describe the problem you are trying to solve

I'm managing a sort of meta-build system that manages packages built in many different languages. It aims to delegate to the idiomatic build tools for each language as much as possible rather than re-implement them, which means it relies heavily on being able to express all necessary configuration through idiomatic configuration files. Unfortunately, there are some configuration options that cannot (currently) be expressed easily in configuration options, and instead have to take the form of environment variables. These are unfortunate because while the meta-build system knows to set those environment variables before it delegates to Cargo, it breaks the ability for human users to "just run cargo", which also tends to break IDE integration provided by rust-analyzer and the like, as they also just execute cargo directly and expect that to work.

Some of these can be dealt with by using the new [env] section in .cargo/config.toml, but since those are only set after Cargo is invoked, it doesn't allow setting environment variables that might affect Cargo itself or Rustup toolchains. While it would be possible to set these using something like direnv, that is an additional tool that users would have to have set up and working, and if they don't things would fail in surprising and hard-to-diagnose ways.

Concretely, I want to change PATH and CARGO_HOME:

  • I want to change PATH so that tools provided by the meta-build system (and idiomatic dependencies declared therein) are made available to custom toolchains. The meta-build system injects custom toolchains that ensure that the local build uses the rustc/cargo vended through the meta-build system rather than whatever the user happens to have installed locally, and those toolchains in turn rely on plenty of commands being available on PATH to do their job. They coincidentally also rely on other custom environment variables, which I would also set in a hypothetical rust-toolchain.toml [env] section.
  • I want to change CARGO_HOME both to control where Cargo places its various cache files so that they end up in a place that the meta-build system can track (and clean) and where they don't clutter the users' normal Cargo setup, and to avoid accidentally picking up user-specific configuration from ~/.cargo/config.toml which might make local build artifacts differ in unpredictable ways leading to hard-to-diagnose issues. CARGO_HOME needs to be set before cargo is invoked, as it affects where Cargo looks for configuration files in the first place, and thus can't be set in Cargo's [env].

Describe the solution you'd like

I propose we add an [env] section to rust-toolchain.toml similar to the one supported by Cargo which allows setting environment variables inside Rustup itself before dispatching to the binary in the toolchain.

Open questions

  • The ability to set environment variables is fairly powerful, and could be abused by setting HOME or PATH in such a way that program execution does "bad things". In all likelihood, this means [env] should be subject to the same "trust decision" as is being proposed in Environment variable to disable rust-toolchain.toml #2793.
  • Cargo's [env] syntax allows setting config-relative paths rather than absolute ones with a relative = true option. Should we support the same? I don't know of such paths existing in Rustup currently.
  • Cargo's [env] does not (currently) provide a mechanism for augmenting an environment variable, such as by allowing PATH to be set to some value that includes $PATH. Should it? My instinct is to follow the design of Cargo's [env] section closely, and only add this if Cargo adds support for the same.
jonhoo pushed a commit to jonhoo/rustup.rs that referenced this issue Oct 28, 2021
@jonhoo jonhoo linked a pull request Oct 28, 2021 that will close this issue
@rbtcollins
Copy link
Contributor

A chunk of this seems to overlap with rust-lang/rfcs#1615 - in particular fine grained control.

I wonder, in particular with the kernels desire for a .rustup-toolchain.toml file, if we should take a step back and rethink rustup's interaction with a source tree.

I find many folk misunderstand how toolchain files work - e.g. that a toolchain file will have no effect when the crate is consumed, so if one is trying to manage corporate settings, global machine config is often better than chasing every separate crates needs separately, or that preferring rust-version over a toolchain file is better except when a custom toolchain is being used.

One possibility is to promote the toolchain file into a full blown config file. Perhaps rename things: .rustup/rustup.toml; permit anything we support in there: proxies, default targets, etc. That would (I think) make clearer that the point is to configure rustup, /not/ to influence how rust builds things.

Which then leads to my thoughts on this proposal. I think where the knobs influence rustup itself, thats fine. Where its about changing how rust/cargo etc operate, I think its a poor fit: not everyone uses rustup, it will mean that rustup (even if only in some users configs) is more tightly coupled to the internals of cargo. I'd rather see these things tackled in cargo itself. For instance, if cargo supported a cache-dir setting, it could read .cargo/config.toml, and then honour it.

@jonhoo
Copy link
Contributor Author

jonhoo commented Mar 5, 2023

I agree with what you've said in general, but unfortunately the two things I outlined above cannot be done through Cargo alone.

I need to set $PATH not for Cargo itself, but for the custom Rustup toolchain that's being used. That is, I have a path toolchain defined in rust-toolchain.toml, and the bin/cargo in that toolchain is a shell script that in turn needs to be able to execute commands from the $PATH I'm trying to set. Thus, Cargo isn't part of the call graph.

I need to set $CARGO_HOME, which cannot (in the general case) be configured within Cargo because $CARGO_HOME dictates where Cargo searches for configuration files. In other words, the place where I would configure Cargo to tell it about the updated $CARGO_HOME would be inside $CARGO_HOME.

Another environment variable that may fit into this broader category of environment variables that are very Rust-specific and cannot solely be handled by Cargo is LD_LIBRARY_PATH. That may need to be set for the cargo executable to even be possible to execute, and thus cannot be configured inside Cargo.

@rbtcollins
Copy link
Contributor

rbtcollins commented Aug 20, 2023

I know we discussed this on discord as welll; I've lost track of where we got to with it all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants