Skip to content

fix: Set RUSTUP_TOOLCHAIN and invoke the proxies instead of directly invoking sysroot binaries #16563

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

Merged
merged 1 commit into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions crates/flycheck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub use cargo_metadata::diagnostic::{
Applicability, Diagnostic, DiagnosticCode, DiagnosticLevel, DiagnosticSpan,
DiagnosticSpanMacroExpansion,
};
use toolchain::Tool;

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationStrategy {
Expand Down Expand Up @@ -89,10 +90,10 @@ impl FlycheckHandle {
id: usize,
sender: Box<dyn Fn(Message) + Send>,
config: FlycheckConfig,
cargo: PathBuf,
sysroot_root: Option<AbsPathBuf>,
workspace_root: AbsPathBuf,
) -> FlycheckHandle {
let actor = FlycheckActor::new(id, sender, config, cargo, workspace_root);
let actor = FlycheckActor::new(id, sender, config, sysroot_root, workspace_root);
let (sender, receiver) = unbounded::<StateChange>();
let thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker)
.name("Flycheck".to_owned())
Expand Down Expand Up @@ -174,7 +175,7 @@ struct FlycheckActor {
/// Either the workspace root of the workspace we are flychecking,
/// or the project root of the project.
root: AbsPathBuf,
cargo: PathBuf,
sysroot_root: Option<AbsPathBuf>,
/// CargoHandle exists to wrap around the communication needed to be able to
/// run `cargo check` without blocking. Currently the Rust standard library
/// doesn't provide a way to read sub-process output without blocking, so we
Expand All @@ -195,11 +196,18 @@ impl FlycheckActor {
id: usize,
sender: Box<dyn Fn(Message) + Send>,
config: FlycheckConfig,
cargo: PathBuf,
sysroot_root: Option<AbsPathBuf>,
workspace_root: AbsPathBuf,
) -> FlycheckActor {
tracing::info!(%id, ?workspace_root, "Spawning flycheck");
FlycheckActor { id, sender, config, cargo, root: workspace_root, command_handle: None }
FlycheckActor {
id,
sender,
config,
sysroot_root,
root: workspace_root,
command_handle: None,
}
}

fn report_progress(&self, progress: Progress) {
Expand Down Expand Up @@ -334,7 +342,10 @@ impl FlycheckActor {
ansi_color_output,
target_dir,
} => {
let mut cmd = Command::new(&self.cargo);
let mut cmd = Command::new(Tool::Cargo.path());
if let Some(sysroot_root) = &self.sysroot_root {
cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(sysroot_root));
}
cmd.arg(command);
cmd.current_dir(&self.root);

Expand Down
12 changes: 6 additions & 6 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,8 @@ impl WorkspaceBuildScripts {
cmd
}
_ => {
let mut cmd = Command::new(
Sysroot::discover_tool(sysroot, Tool::Cargo)
.map_err(|e| io::Error::new(io::ErrorKind::NotFound, e))?,
);
let mut cmd = Command::new(Tool::Cargo.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);

cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
cmd.args(&config.extra_args);
Expand Down Expand Up @@ -431,7 +429,8 @@ impl WorkspaceBuildScripts {
}
let res = (|| {
let target_libdir = (|| {
let mut cargo_config = Command::new(Sysroot::discover_tool(sysroot, Tool::Cargo)?);
let mut cargo_config = Command::new(Tool::Cargo.path());
Sysroot::set_rustup_toolchain_env(&mut cargo_config, sysroot);
cargo_config.envs(extra_env);
cargo_config
.current_dir(current_dir)
Expand All @@ -440,7 +439,8 @@ impl WorkspaceBuildScripts {
if let Ok(it) = utf8_stdout(cargo_config) {
return Ok(it);
}
let mut cmd = Command::new(Sysroot::discover_tool(sysroot, Tool::Rustc)?);
let mut cmd = Command::new(Tool::Rustc.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
cmd.envs(extra_env);
cmd.args(["--print", "target-libdir"]);
utf8_stdout(cmd)
Expand Down
10 changes: 6 additions & 4 deletions crates/project-model/src/cargo_workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl CargoWorkspace {
let targets = find_list_of_build_targets(config, cargo_toml, sysroot);

let mut meta = MetadataCommand::new();
meta.cargo_path(Sysroot::discover_tool(sysroot, Tool::Cargo)?);
meta.cargo_path(Tool::Cargo.path());
meta.manifest_path(cargo_toml.to_path_buf());
match &config.features {
CargoFeatures::All => {
Expand Down Expand Up @@ -291,6 +291,7 @@ impl CargoWorkspace {

(|| -> Result<cargo_metadata::Metadata, cargo_metadata::Error> {
let mut command = meta.cargo_command();
Sysroot::set_rustup_toolchain_env(&mut command, sysroot);
command.envs(&config.extra_env);
let output = command.output()?;
if !output.status.success() {
Expand Down Expand Up @@ -500,7 +501,8 @@ fn rustc_discover_host_triple(
extra_env: &FxHashMap<String, String>,
sysroot: Option<&Sysroot>,
) -> Option<String> {
let mut rustc = Command::new(Sysroot::discover_tool(sysroot, Tool::Rustc).ok()?);
let mut rustc = Command::new(Tool::Rustc.path());
Sysroot::set_rustup_toolchain_env(&mut rustc, sysroot);
rustc.envs(extra_env);
rustc.current_dir(cargo_toml.parent()).arg("-vV");
tracing::debug!("Discovering host platform by {:?}", rustc);
Expand Down Expand Up @@ -528,8 +530,8 @@ fn cargo_config_build_target(
extra_env: &FxHashMap<String, String>,
sysroot: Option<&Sysroot>,
) -> Vec<String> {
let Ok(program) = Sysroot::discover_tool(sysroot, Tool::Cargo) else { return vec![] };
let mut cargo_config = Command::new(program);
let mut cargo_config = Command::new(Tool::Cargo.path());
Sysroot::set_rustup_toolchain_env(&mut cargo_config, sysroot);
cargo_config.envs(extra_env);
cargo_config
.current_dir(cargo_toml.parent())
Expand Down
13 changes: 6 additions & 7 deletions crates/project-model/src/rustc_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ fn get_rust_cfgs(
config: RustcCfgConfig<'_>,
) -> anyhow::Result<String> {
let sysroot = match config {
RustcCfgConfig::Cargo(sysroot, cargo_oml) => {
let cargo = Sysroot::discover_tool(sysroot, toolchain::Tool::Cargo)?;
let mut cmd = Command::new(cargo);
RustcCfgConfig::Cargo(sysroot, cargo_toml) => {
let mut cmd = Command::new(toolchain::Tool::Cargo.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
cmd.envs(extra_env);
cmd.current_dir(cargo_oml.parent())
cmd.current_dir(cargo_toml.parent())
.args(["rustc", "-Z", "unstable-options", "--print", "cfg"])
.env("RUSTC_BOOTSTRAP", "1");
if let Some(target) = target {
Expand All @@ -90,9 +90,8 @@ fn get_rust_cfgs(
RustcCfgConfig::Rustc(sysroot) => sysroot,
};

let rustc = Sysroot::discover_tool(sysroot, toolchain::Tool::Rustc)?;
tracing::debug!(?rustc, "using explicit rustc from sysroot");
let mut cmd = Command::new(rustc);
let mut cmd = Command::new(toolchain::Tool::Rustc.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
cmd.envs(extra_env);
cmd.args(["--print", "cfg", "-O"]);
if let Some(target) = target {
Expand Down
24 changes: 5 additions & 19 deletions crates/project-model/src/sysroot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@

use std::{env, fs, iter, ops, path::PathBuf, process::Command, sync::Arc};

use anyhow::{format_err, Context, Result};
use anyhow::{format_err, Result};
use base_db::CrateName;
use itertools::Itertools;
use la_arena::{Arena, Idx};
use paths::{AbsPath, AbsPathBuf};
use rustc_hash::FxHashMap;
use toolchain::{probe_for_binary, Tool};
use toolchain::probe_for_binary;

use crate::{utf8_stdout, CargoConfig, CargoWorkspace, ManifestPath};

Expand Down Expand Up @@ -193,23 +193,9 @@ impl Sysroot {
Ok(Sysroot::load(sysroot_dir, Some(sysroot_src_dir), metadata))
}

pub fn discover_binary(&self, binary: &str) -> anyhow::Result<AbsPathBuf> {
toolchain::probe_for_binary(self.root.join("bin").join(binary).into())
.ok_or_else(|| anyhow::anyhow!("no rustc binary found in {}", self.root.join("bin")))
.and_then(|rustc| {
fs::metadata(&rustc).map(|_| AbsPathBuf::assert(rustc)).with_context(|| {
format!(
"failed to discover rustc in sysroot: {:?}",
AsRef::<std::path::Path>::as_ref(&self.root)
)
})
})
}

pub fn discover_tool(sysroot: Option<&Self>, tool: Tool) -> anyhow::Result<PathBuf> {
match sysroot {
Some(sysroot) => sysroot.discover_binary(tool.name()).map(Into::into),
None => Ok(tool.path()),
pub fn set_rustup_toolchain_env(cmd: &mut Command, sysroot: Option<&Self>) {
if let Some(sysroot) = sysroot {
cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(&sysroot.root));
}
}

Expand Down
8 changes: 4 additions & 4 deletions crates/project-model/src/target_data_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ pub fn get(
};
let sysroot = match config {
RustcDataLayoutConfig::Cargo(sysroot, cargo_toml) => {
let cargo = Sysroot::discover_tool(sysroot, toolchain::Tool::Cargo)?;
let mut cmd = Command::new(cargo);
let mut cmd = Command::new(toolchain::Tool::Cargo.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
cmd.envs(extra_env);
cmd.current_dir(cargo_toml.parent())
.args(["rustc", "--", "-Z", "unstable-options", "--print", "target-spec-json"])
Expand All @@ -48,8 +48,8 @@ pub fn get(
RustcDataLayoutConfig::Rustc(sysroot) => sysroot,
};

let rustc = Sysroot::discover_tool(sysroot, toolchain::Tool::Rustc)?;
let mut cmd = Command::new(rustc);
let mut cmd = Command::new(toolchain::Tool::Rustc.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
cmd.envs(extra_env)
.args(["-Z", "unstable-options", "--print", "target-spec-json"])
.env("RUSTC_BOOTSTRAP", "1");
Expand Down
30 changes: 17 additions & 13 deletions crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
//! metadata` or `rust-project.json`) into representation stored in the salsa
//! database -- `CrateGraph`.

use std::{
collections::VecDeque, fmt, fs, iter, path::PathBuf, process::Command, str::FromStr, sync,
};
use std::{collections::VecDeque, fmt, fs, iter, process::Command, str::FromStr, sync};

use anyhow::{format_err, Context};
use base_db::{
Expand All @@ -16,6 +14,7 @@ use paths::{AbsPath, AbsPathBuf};
use rustc_hash::{FxHashMap, FxHashSet};
use semver::Version;
use stdx::always;
use toolchain::Tool;
use triomphe::Arc;

use crate::{
Expand Down Expand Up @@ -163,12 +162,14 @@ impl fmt::Debug for ProjectWorkspace {

fn get_toolchain_version(
current_dir: &AbsPath,
cmd_path: Result<PathBuf, anyhow::Error>,
sysroot: Option<&Sysroot>,
tool: Tool,
extra_env: &FxHashMap<String, String>,
prefix: &str,
) -> Result<Option<Version>, anyhow::Error> {
let cargo_version = utf8_stdout({
let mut cmd = Command::new(cmd_path?);
let mut cmd = Command::new(tool.path());
Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
cmd.envs(extra_env);
cmd.arg("--version").current_dir(current_dir);
cmd
Expand Down Expand Up @@ -289,7 +290,8 @@ impl ProjectWorkspace {

let toolchain = get_toolchain_version(
cargo_toml.parent(),
Sysroot::discover_tool(sysroot_ref, toolchain::Tool::Cargo),
sysroot_ref,
toolchain::Tool::Cargo,
&config.extra_env,
"cargo ",
)?;
Expand Down Expand Up @@ -370,9 +372,13 @@ impl ProjectWorkspace {
let sysroot_ref = sysroot.as_ref().ok();
let cfg_config = RustcCfgConfig::Rustc(sysroot_ref);
let data_layout_config = RustcDataLayoutConfig::Rustc(sysroot_ref);
let rustc = Sysroot::discover_tool(sysroot_ref, toolchain::Tool::Rustc).map(Into::into);
let toolchain = match get_toolchain_version(project_json.path(), rustc, extra_env, "rustc ")
{
let toolchain = match get_toolchain_version(
project_json.path(),
sysroot_ref,
toolchain::Tool::Rustc,
extra_env,
"rustc ",
) {
Ok(it) => it,
Err(e) => {
tracing::error!("{e}");
Expand Down Expand Up @@ -1615,10 +1621,8 @@ fn cargo_config_env(
extra_env: &FxHashMap<String, String>,
sysroot: Option<&Sysroot>,
) -> FxHashMap<String, String> {
let Ok(program) = Sysroot::discover_tool(sysroot, toolchain::Tool::Cargo) else {
return Default::default();
};
let mut cargo_config = Command::new(program);
let mut cargo_config = Command::new(Tool::Cargo.path());
Sysroot::set_rustup_toolchain_env(&mut cargo_config, sysroot);
cargo_config.envs(extra_env);
cargo_config
.current_dir(cargo_toml.parent())
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,7 @@ fn run_rustfmt(

let mut command = match snap.config.rustfmt() {
RustfmtConfig::Rustfmt { extra_args, enable_range_formatting } => {
// FIXME: This should use the sysroot's rustfmt if its loaded
// FIXME: Set RUSTUP_TOOLCHAIN
let mut cmd = process::Command::new(toolchain::rustfmt());
cmd.envs(snap.config.extra_env());
cmd.args(extra_args);
Expand Down
39 changes: 20 additions & 19 deletions crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use ide_db::{
use itertools::Itertools;
use load_cargo::{load_proc_macro, ProjectFolders};
use proc_macro_api::ProcMacroServer;
use project_model::{ProjectWorkspace, Sysroot, WorkspaceBuildScripts};
use project_model::{ProjectWorkspace, WorkspaceBuildScripts};
use rustc_hash::FxHashSet;
use stdx::{format_to, thread::ThreadIntent};
use triomphe::Arc;
Expand Down Expand Up @@ -468,16 +468,20 @@ impl GlobalState {
None => ws.find_sysroot_proc_macro_srv()?,
};

let env = match ws {
ProjectWorkspace::Cargo { cargo_config_extra_env, .. } => {
cargo_config_extra_env
.iter()
.chain(self.config.extra_env())
.map(|(a, b)| (a.clone(), b.clone()))
.collect()
}
_ => Default::default(),
};
let env =
match ws {
ProjectWorkspace::Cargo { cargo_config_extra_env, sysroot, .. } => {
cargo_config_extra_env
.iter()
.chain(self.config.extra_env())
.map(|(a, b)| (a.clone(), b.clone()))
.chain(sysroot.as_ref().map(|it| {
("RUSTUP_TOOLCHAIN".to_owned(), it.root().to_string())
}))
.collect()
}
_ => Default::default(),
};
tracing::info!("Using proc-macro server at {path}");

ProcMacroServer::spawn(path.clone(), &env).map_err(|err| {
Expand Down Expand Up @@ -620,7 +624,7 @@ impl GlobalState {
0,
Box::new(move |msg| sender.send(msg).unwrap()),
config,
toolchain::cargo(),
None,
self.config.root_path().clone(),
)],
flycheck::InvocationStrategy::PerWorkspace => {
Expand All @@ -631,7 +635,7 @@ impl GlobalState {
ProjectWorkspace::Cargo { cargo, sysroot, .. } => Some((
id,
cargo.workspace_root(),
Sysroot::discover_tool(sysroot.as_ref().ok(), toolchain::Tool::Cargo),
sysroot.as_ref().ok().map(|sysroot| sysroot.root().to_owned()),
)),
ProjectWorkspace::Json { project, sysroot, .. } => {
// Enable flychecks for json projects if a custom flycheck command was supplied
Expand All @@ -640,23 +644,20 @@ impl GlobalState {
FlycheckConfig::CustomCommand { .. } => Some((
id,
project.path(),
Sysroot::discover_tool(
sysroot.as_ref().ok(),
toolchain::Tool::Cargo,
),
sysroot.as_ref().ok().map(|sysroot| sysroot.root().to_owned()),
)),
_ => None,
}
}
ProjectWorkspace::DetachedFiles { .. } => None,
})
.map(|(id, root, cargo)| {
.map(|(id, root, sysroot_root)| {
let sender = sender.clone();
FlycheckHandle::spawn(
id,
Box::new(move |msg| sender.send(msg).unwrap()),
config.clone(),
cargo.unwrap_or_else(|_| toolchain::cargo()),
sysroot_root,
root.to_path_buf(),
)
})
Expand Down