fix: Fix metadata retrying eating original errors

This commit is contained in:
Lukas Wirth 2024-08-25 09:28:47 +02:00
parent 2025b43653
commit 191949eabe
9 changed files with 60 additions and 52 deletions

View file

@ -33,8 +33,6 @@ pub struct CargoWorkspace {
workspace_root: AbsPathBuf, workspace_root: AbsPathBuf,
target_directory: AbsPathBuf, target_directory: AbsPathBuf,
manifest_path: ManifestPath, manifest_path: ManifestPath,
// Whether this workspace was queried with `--no-deps`.
no_deps: bool,
} }
impl ops::Index<Package> for CargoWorkspace { impl ops::Index<Package> for CargoWorkspace {
@ -253,6 +251,9 @@ struct PackageMetadata {
} }
impl CargoWorkspace { impl CargoWorkspace {
/// Fetches the metadata for the given `cargo_toml` manifest.
/// A successful result may contain another metadata error if the initial fetching failed but
/// the `--no-deps` retry succeeded.
pub fn fetch_metadata( pub fn fetch_metadata(
cargo_toml: &ManifestPath, cargo_toml: &ManifestPath,
current_dir: &AbsPath, current_dir: &AbsPath,
@ -260,7 +261,7 @@ impl CargoWorkspace {
sysroot: &Sysroot, sysroot: &Sysroot,
locked: bool, locked: bool,
progress: &dyn Fn(String), progress: &dyn Fn(String),
) -> anyhow::Result<cargo_metadata::Metadata> { ) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
Self::fetch_metadata_(cargo_toml, current_dir, config, sysroot, locked, false, progress) Self::fetch_metadata_(cargo_toml, current_dir, config, sysroot, locked, false, progress)
} }
@ -272,7 +273,7 @@ impl CargoWorkspace {
locked: bool, locked: bool,
no_deps: bool, no_deps: bool,
progress: &dyn Fn(String), progress: &dyn Fn(String),
) -> anyhow::Result<cargo_metadata::Metadata> { ) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
let targets = find_list_of_build_targets(config, cargo_toml, sysroot); let targets = find_list_of_build_targets(config, cargo_toml, sysroot);
let cargo = sysroot.tool(Tool::Cargo); let cargo = sysroot.tool(Tool::Cargo);
@ -337,13 +338,17 @@ impl CargoWorkspace {
// unclear whether cargo itself supports it. // unclear whether cargo itself supports it.
progress("metadata".to_owned()); progress("metadata".to_owned());
(|| -> Result<cargo_metadata::Metadata, cargo_metadata::Error> { (|| -> anyhow::Result<(_, _)> {
let output = meta.cargo_command().output()?; let output = meta.cargo_command().output()?;
if !output.status.success() { if !output.status.success() {
let error = cargo_metadata::Error::CargoMetadata {
stderr: String::from_utf8(output.stderr)?,
}
.into();
if !no_deps { if !no_deps {
// If we failed to fetch metadata with deps, try again without them. // If we failed to fetch metadata with deps, try again without them.
// This makes r-a still work partially when offline. // This makes r-a still work partially when offline.
if let Ok(metadata) = Self::fetch_metadata_( if let Ok((metadata, _)) = Self::fetch_metadata_(
cargo_toml, cargo_toml,
current_dir, current_dir,
config, config,
@ -352,20 +357,23 @@ impl CargoWorkspace {
true, true,
progress, progress,
) { ) {
return Ok(metadata); return Ok((metadata, Some(error)));
} }
} }
return Err(error);
return Err(cargo_metadata::Error::CargoMetadata {
stderr: String::from_utf8(output.stderr)?,
});
} }
let stdout = from_utf8(&output.stdout)? let stdout = from_utf8(&output.stdout)?
.lines() .lines()
.find(|line| line.starts_with('{')) .find(|line| line.starts_with('{'))
.ok_or(cargo_metadata::Error::NoJson)?; .ok_or(cargo_metadata::Error::NoJson)?;
cargo_metadata::MetadataCommand::parse(stdout) Ok((cargo_metadata::MetadataCommand::parse(stdout)?, None))
})() })()
.map(|(metadata, error)| {
(
metadata,
error.map(|e| e.context(format!("Failed to run `{:?}`", meta.cargo_command()))),
)
})
.with_context(|| format!("Failed to run `{:?}`", meta.cargo_command())) .with_context(|| format!("Failed to run `{:?}`", meta.cargo_command()))
} }
@ -463,7 +471,6 @@ impl CargoWorkspace {
pkg_data.targets.push(tgt); pkg_data.targets.push(tgt);
} }
} }
let no_deps = meta.resolve.is_none();
for mut node in meta.resolve.map_or_else(Vec::new, |it| it.nodes) { for mut node in meta.resolve.map_or_else(Vec::new, |it| it.nodes) {
let &source = pkg_by_id.get(&node.id).unwrap(); let &source = pkg_by_id.get(&node.id).unwrap();
node.deps.sort_by(|a, b| a.pkg.cmp(&b.pkg)); node.deps.sort_by(|a, b| a.pkg.cmp(&b.pkg));
@ -483,14 +490,7 @@ impl CargoWorkspace {
let target_directory = AbsPathBuf::assert(meta.target_directory); let target_directory = AbsPathBuf::assert(meta.target_directory);
CargoWorkspace { CargoWorkspace { packages, targets, workspace_root, target_directory, manifest_path }
packages,
targets,
workspace_root,
target_directory,
manifest_path,
no_deps,
}
} }
pub fn packages(&self) -> impl ExactSizeIterator<Item = Package> + '_ { pub fn packages(&self) -> impl ExactSizeIterator<Item = Package> + '_ {
@ -572,10 +572,6 @@ impl CargoWorkspace {
fn is_unique(&self, name: &str) -> bool { fn is_unique(&self, name: &str) -> bool {
self.packages.iter().filter(|(_, v)| v.name == name).count() == 1 self.packages.iter().filter(|(_, v)| v.name == name).count() == 1
} }
pub fn no_deps(&self) -> bool {
self.no_deps
}
} }
fn find_list_of_build_targets( fn find_list_of_build_targets(

View file

@ -325,7 +325,7 @@ impl Sysroot {
"nightly".to_owned(), "nightly".to_owned(),
); );
let mut res = match CargoWorkspace::fetch_metadata( let (mut res, _) = match CargoWorkspace::fetch_metadata(
&library_manifest, &library_manifest,
sysroot_src_dir, sysroot_src_dir,
&cargo_config, &cargo_config,

View file

@ -34,6 +34,7 @@ fn load_cargo_with_overrides(
build_scripts: WorkspaceBuildScripts::default(), build_scripts: WorkspaceBuildScripts::default(),
rustc: Err(None), rustc: Err(None),
cargo_config_extra_env: Default::default(), cargo_config_extra_env: Default::default(),
error: None,
}, },
cfg_overrides, cfg_overrides,
sysroot: Sysroot::empty(), sysroot: Sysroot::empty(),
@ -58,6 +59,7 @@ fn load_cargo_with_fake_sysroot(
build_scripts: WorkspaceBuildScripts::default(), build_scripts: WorkspaceBuildScripts::default(),
rustc: Err(None), rustc: Err(None),
cargo_config_extra_env: Default::default(), cargo_config_extra_env: Default::default(),
error: None,
}, },
sysroot: get_fake_sysroot(), sysroot: get_fake_sysroot(),
rustc_cfg: Vec::new(), rustc_cfg: Vec::new(),
@ -294,6 +296,7 @@ fn smoke_test_real_sysroot_cargo() {
build_scripts: WorkspaceBuildScripts::default(), build_scripts: WorkspaceBuildScripts::default(),
rustc: Err(None), rustc: Err(None),
cargo_config_extra_env: Default::default(), cargo_config_extra_env: Default::default(),
error: None,
}, },
sysroot, sysroot,
rustc_cfg: Vec::new(), rustc_cfg: Vec::new(),

View file

@ -69,6 +69,8 @@ pub enum ProjectWorkspaceKind {
Cargo { Cargo {
/// The workspace as returned by `cargo metadata`. /// The workspace as returned by `cargo metadata`.
cargo: CargoWorkspace, cargo: CargoWorkspace,
/// Additional `cargo metadata` error. (only populated if retried fetching via `--no-deps` succeeded).
error: Option<Arc<anyhow::Error>>,
/// The build script results for the workspace. /// The build script results for the workspace.
build_scripts: WorkspaceBuildScripts, build_scripts: WorkspaceBuildScripts,
/// The rustc workspace loaded for this workspace. An `Err(None)` means loading has been /// The rustc workspace loaded for this workspace. An `Err(None)` means loading has been
@ -93,7 +95,7 @@ pub enum ProjectWorkspaceKind {
/// The file in question. /// The file in question.
file: ManifestPath, file: ManifestPath,
/// Is this file a cargo script file? /// Is this file a cargo script file?
cargo: Option<(CargoWorkspace, WorkspaceBuildScripts)>, cargo: Option<(CargoWorkspace, WorkspaceBuildScripts, Option<Arc<anyhow::Error>>)>,
/// Environment variables set in the `.cargo/config` file. /// Environment variables set in the `.cargo/config` file.
cargo_config_extra_env: FxHashMap<String, String>, cargo_config_extra_env: FxHashMap<String, String>,
}, },
@ -106,6 +108,7 @@ impl fmt::Debug for ProjectWorkspace {
match kind { match kind {
ProjectWorkspaceKind::Cargo { ProjectWorkspaceKind::Cargo {
cargo, cargo,
error: _,
build_scripts: _, build_scripts: _,
rustc, rustc,
cargo_config_extra_env, cargo_config_extra_env,
@ -256,7 +259,7 @@ impl ProjectWorkspace {
false, false,
progress, progress,
) { ) {
Ok(meta) => { Ok((meta, _error)) => {
let workspace = CargoWorkspace::new(meta, cargo_toml.clone()); let workspace = CargoWorkspace::new(meta, cargo_toml.clone());
let buildscripts = WorkspaceBuildScripts::rustc_crates( let buildscripts = WorkspaceBuildScripts::rustc_crates(
&workspace, &workspace,
@ -301,7 +304,7 @@ impl ProjectWorkspace {
tracing::error!(%e, "failed fetching data layout for {cargo_toml:?} workspace"); tracing::error!(%e, "failed fetching data layout for {cargo_toml:?} workspace");
} }
let meta = CargoWorkspace::fetch_metadata( let (meta, error) = CargoWorkspace::fetch_metadata(
cargo_toml, cargo_toml,
cargo_toml.parent(), cargo_toml.parent(),
config, config,
@ -324,6 +327,7 @@ impl ProjectWorkspace {
build_scripts: WorkspaceBuildScripts::default(), build_scripts: WorkspaceBuildScripts::default(),
rustc, rustc,
cargo_config_extra_env, cargo_config_extra_env,
error: error.map(Arc::new),
}, },
sysroot, sysroot,
rustc_cfg, rustc_cfg,
@ -404,10 +408,11 @@ impl ProjectWorkspace {
let cargo_script = let cargo_script =
CargoWorkspace::fetch_metadata(detached_file, dir, config, &sysroot, false, &|_| ()) CargoWorkspace::fetch_metadata(detached_file, dir, config, &sysroot, false, &|_| ())
.ok() .ok()
.map(|ws| { .map(|(ws, error)| {
( (
CargoWorkspace::new(ws, detached_file.clone()), CargoWorkspace::new(ws, detached_file.clone()),
WorkspaceBuildScripts::default(), WorkspaceBuildScripts::default(),
error.map(Arc::new),
) )
}); });
@ -440,10 +445,8 @@ impl ProjectWorkspace {
progress: &dyn Fn(String), progress: &dyn Fn(String),
) -> anyhow::Result<WorkspaceBuildScripts> { ) -> anyhow::Result<WorkspaceBuildScripts> {
match &self.kind { match &self.kind {
ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, None)), .. }
| ProjectWorkspaceKind::Cargo { cargo, .. } | ProjectWorkspaceKind::Cargo { cargo, error: None, .. } => {
if !cargo.no_deps() =>
{
WorkspaceBuildScripts::run_for_workspace(config, cargo, progress, &self.sysroot) WorkspaceBuildScripts::run_for_workspace(config, cargo, progress, &self.sysroot)
.with_context(|| { .with_context(|| {
format!("Failed to run build scripts for {}", cargo.workspace_root()) format!("Failed to run build scripts for {}", cargo.workspace_root())
@ -502,7 +505,7 @@ impl ProjectWorkspace {
pub fn set_build_scripts(&mut self, bs: WorkspaceBuildScripts) { pub fn set_build_scripts(&mut self, bs: WorkspaceBuildScripts) {
match &mut self.kind { match &mut self.kind {
ProjectWorkspaceKind::Cargo { build_scripts, .. } ProjectWorkspaceKind::Cargo { build_scripts, .. }
| ProjectWorkspaceKind::DetachedFile { cargo: Some((_, build_scripts)), .. } => { | ProjectWorkspaceKind::DetachedFile { cargo: Some((_, build_scripts, _)), .. } => {
*build_scripts = bs *build_scripts = bs
} }
_ => assert_eq!(bs, WorkspaceBuildScripts::default()), _ => assert_eq!(bs, WorkspaceBuildScripts::default()),
@ -593,6 +596,7 @@ impl ProjectWorkspace {
rustc, rustc,
build_scripts, build_scripts,
cargo_config_extra_env: _, cargo_config_extra_env: _,
error: _,
} => { } => {
cargo cargo
.packages() .packages()
@ -648,7 +652,7 @@ impl ProjectWorkspace {
include: vec![file.to_path_buf()], include: vec![file.to_path_buf()],
exclude: Vec::new(), exclude: Vec::new(),
}) })
.chain(cargo_script.iter().flat_map(|(cargo, build_scripts)| { .chain(cargo_script.iter().flat_map(|(cargo, build_scripts, _)| {
cargo.packages().map(|pkg| { cargo.packages().map(|pkg| {
let is_local = cargo[pkg].is_local; let is_local = cargo[pkg].is_local;
let pkg_root = cargo[pkg].manifest.parent().to_path_buf(); let pkg_root = cargo[pkg].manifest.parent().to_path_buf();
@ -703,7 +707,7 @@ impl ProjectWorkspace {
} }
ProjectWorkspaceKind::DetachedFile { cargo: cargo_script, .. } => { ProjectWorkspaceKind::DetachedFile { cargo: cargo_script, .. } => {
sysroot_package_len sysroot_package_len
+ cargo_script.as_ref().map_or(1, |(cargo, _)| cargo.packages().len()) + cargo_script.as_ref().map_or(1, |(cargo, _, _)| cargo.packages().len())
} }
} }
} }
@ -733,6 +737,7 @@ impl ProjectWorkspace {
rustc, rustc,
build_scripts, build_scripts,
cargo_config_extra_env: _, cargo_config_extra_env: _,
error: _,
} => ( } => (
cargo_to_crate_graph( cargo_to_crate_graph(
load, load,
@ -746,7 +751,7 @@ impl ProjectWorkspace {
sysroot, sysroot,
), ),
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => ( ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => (
if let Some((cargo, build_scripts)) = cargo_script { if let Some((cargo, build_scripts, _)) = cargo_script {
cargo_to_crate_graph( cargo_to_crate_graph(
&mut |path| load(path), &mut |path| load(path),
None, None,
@ -795,12 +800,14 @@ impl ProjectWorkspace {
rustc, rustc,
cargo_config_extra_env, cargo_config_extra_env,
build_scripts: _, build_scripts: _,
error: _,
}, },
ProjectWorkspaceKind::Cargo { ProjectWorkspaceKind::Cargo {
cargo: o_cargo, cargo: o_cargo,
rustc: o_rustc, rustc: o_rustc,
cargo_config_extra_env: o_cargo_config_extra_env, cargo_config_extra_env: o_cargo_config_extra_env,
build_scripts: _, build_scripts: _,
error: _,
}, },
) => { ) => {
cargo == o_cargo cargo == o_cargo
@ -813,12 +820,12 @@ impl ProjectWorkspace {
( (
ProjectWorkspaceKind::DetachedFile { ProjectWorkspaceKind::DetachedFile {
file, file,
cargo: Some((cargo_script, _)), cargo: Some((cargo_script, _, _)),
cargo_config_extra_env, cargo_config_extra_env,
}, },
ProjectWorkspaceKind::DetachedFile { ProjectWorkspaceKind::DetachedFile {
file: o_file, file: o_file,
cargo: Some((o_cargo_script, _)), cargo: Some((o_cargo_script, _, _)),
cargo_config_extra_env: o_cargo_config_extra_env, cargo_config_extra_env: o_cargo_config_extra_env,
}, },
) => { ) => {

View file

@ -667,7 +667,7 @@ impl GlobalStateSnapshot {
for workspace in self.workspaces.iter() { for workspace in self.workspaces.iter() {
match &workspace.kind { match &workspace.kind {
ProjectWorkspaceKind::Cargo { cargo, .. } ProjectWorkspaceKind::Cargo { cargo, .. }
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } => { | ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
let Some(target_idx) = cargo.target_by_root(path) else { let Some(target_idx) = cargo.target_by_root(path) else {
continue; continue;
}; };

View file

@ -343,7 +343,7 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
let package = match &ws.kind { let package = match &ws.kind {
project_model::ProjectWorkspaceKind::Cargo { cargo, .. } project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
| project_model::ProjectWorkspaceKind::DetachedFile { | project_model::ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _)), cargo: Some((cargo, _, _)),
.. ..
} => cargo.packages().find_map(|pkg| { } => cargo.packages().find_map(|pkg| {
let has_target_with_root = cargo[pkg] let has_target_with_root = cargo[pkg]

View file

@ -799,7 +799,7 @@ pub(crate) fn handle_parent_module(
.iter() .iter()
.filter_map(|ws| match &ws.kind { .filter_map(|ws| match &ws.kind {
ProjectWorkspaceKind::Cargo { cargo, .. } ProjectWorkspaceKind::Cargo { cargo, .. }
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } => { | ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
cargo.parent_manifests(&manifest_path) cargo.parent_manifests(&manifest_path)
} }
_ => None, _ => None,
@ -1839,7 +1839,7 @@ pub(crate) fn handle_open_docs(
let ws_and_sysroot = snap.workspaces.iter().find_map(|ws| match &ws.kind { let ws_and_sysroot = snap.workspaces.iter().find_map(|ws| match &ws.kind {
ProjectWorkspaceKind::Cargo { cargo, .. } ProjectWorkspaceKind::Cargo { cargo, .. }
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } => { | ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
Some((cargo, &ws.sysroot)) Some((cargo, &ws.sysroot))
} }
ProjectWorkspaceKind::Json { .. } => None, ProjectWorkspaceKind::Json { .. } => None,

View file

@ -180,16 +180,17 @@ impl GlobalState {
self.proc_macro_clients.iter().map(Some).chain(iter::repeat_with(|| None)); self.proc_macro_clients.iter().map(Some).chain(iter::repeat_with(|| None));
for (ws, proc_macro_client) in self.workspaces.iter().zip(proc_macro_clients) { for (ws, proc_macro_client) in self.workspaces.iter().zip(proc_macro_clients) {
if matches!( if let ProjectWorkspaceKind::Cargo { error: Some(error), .. }
&ws.kind, | ProjectWorkspaceKind::DetachedFile {
ProjectWorkspaceKind::Cargo { cargo, .. } | ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } cargo: Some((_, _, Some(error))), ..
if cargo.no_deps() } = &ws.kind
) { {
status.health |= lsp_ext::Health::Warning; status.health |= lsp_ext::Health::Warning;
format_to!( format_to!(
message, message,
"Failed to read Cargo metadata for `{}`, the `Cargo.toml` might be invalid or you have no internet connection.\n\n", "Failed to read Cargo metadata with dependencies for `{}`: {:#}\n\n",
ws.manifest_or_root() ws.manifest_or_root(),
error
); );
} }
if let Some(err) = ws.sysroot.error() { if let Some(err) = ws.sysroot.error() {
@ -805,7 +806,7 @@ impl GlobalState {
match &ws.kind { match &ws.kind {
ProjectWorkspaceKind::Cargo { cargo, .. } ProjectWorkspaceKind::Cargo { cargo, .. }
| ProjectWorkspaceKind::DetachedFile { | ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _)), cargo: Some((cargo, _, _)),
.. ..
} => (cargo.workspace_root(), Some(cargo.manifest_path())), } => (cargo.workspace_root(), Some(cargo.manifest_path())),
ProjectWorkspaceKind::Json(project) => { ProjectWorkspaceKind::Json(project) => {

View file

@ -20,6 +20,7 @@ fn load_cargo_with_fake_sysroot(file: &str) -> ProjectWorkspace {
build_scripts: WorkspaceBuildScripts::default(), build_scripts: WorkspaceBuildScripts::default(),
rustc: Err(None), rustc: Err(None),
cargo_config_extra_env: Default::default(), cargo_config_extra_env: Default::default(),
error: None,
}, },
sysroot: get_fake_sysroot(), sysroot: get_fake_sysroot(),
rustc_cfg: Vec::new(), rustc_cfg: Vec::new(),