Skip to content

Commit c223013

Browse files
borsbors
committed
Auto merge of rust-lang#17956 - Veykril:metadata-err, r=Veykril
fix: Fix metadata retrying eating original errors
2 parents cba00a8 + d9d8d94 commit c223013

File tree

9 files changed

+60
-52
lines changed

9 files changed

+60
-52
lines changed

‎src/tools/rust-analyzer/crates/project-model/src/cargo_workspace.rs

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ pub struct CargoWorkspace {
3333
workspace_root: AbsPathBuf,
3434
target_directory: AbsPathBuf,
3535
manifest_path: ManifestPath,
36-
// Whether this workspace was queried with `--no-deps`.
37-
no_deps: bool,
3836
}
3937

4038
impl ops::Index<Package> for CargoWorkspace {
@@ -253,14 +251,17 @@ struct PackageMetadata {
253251
}
254252

255253
impl CargoWorkspace {
254+
/// Fetches the metadata for the given `cargo_toml` manifest.
255+
/// A successful result may contain another metadata error if the initial fetching failed but
256+
/// the `--no-deps` retry succeeded.
256257
pub fn fetch_metadata(
257258
cargo_toml: &ManifestPath,
258259
current_dir: &AbsPath,
259260
config: &CargoConfig,
260261
sysroot: &Sysroot,
261262
locked: bool,
262263
progress: &dyn Fn(String),
263-
) -> anyhow::Result<cargo_metadata::Metadata> {
264+
) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
264265
Self::fetch_metadata_(cargo_toml, current_dir, config, sysroot, locked, false, progress)
265266
}
266267

@@ -272,7 +273,7 @@ impl CargoWorkspace {
272273
locked: bool,
273274
no_deps: bool,
274275
progress: &dyn Fn(String),
275-
) -> anyhow::Result<cargo_metadata::Metadata> {
276+
) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
276277
let targets = find_list_of_build_targets(config, cargo_toml, sysroot);
277278

278279
let cargo = sysroot.tool(Tool::Cargo);
@@ -337,13 +338,17 @@ impl CargoWorkspace {
337338
// unclear whether cargo itself supports it.
338339
progress("metadata".to_owned());
339340

340-
(|| -> Result<cargo_metadata::Metadata, cargo_metadata::Error> {
341+
(|| -> anyhow::Result<(_, _)> {
341342
let output = meta.cargo_command().output()?;
342343
if !output.status.success() {
344+
let error = cargo_metadata::Error::CargoMetadata {
345+
stderr: String::from_utf8(output.stderr)?,
346+
}
347+
.into();
343348
if !no_deps {
344349
// If we failed to fetch metadata with deps, try again without them.
345350
// This makes r-a still work partially when offline.
346-
if let Ok(metadata) = Self::fetch_metadata_(
351+
if let Ok((metadata, _)) = Self::fetch_metadata_(
347352
cargo_toml,
348353
current_dir,
349354
config,
@@ -352,20 +357,23 @@ impl CargoWorkspace {
352357
true,
353358
progress,
354359
) {
355-
return Ok(metadata);
360+
return Ok((metadata, Some(error)));
356361
}
357362
}
358-
359-
return Err(cargo_metadata::Error::CargoMetadata {
360-
stderr: String::from_utf8(output.stderr)?,
361-
});
363+
return Err(error);
362364
}
363365
let stdout = from_utf8(&output.stdout)?
364366
.lines()
365367
.find(|line| line.starts_with('{'))
366368
.ok_or(cargo_metadata::Error::NoJson)?;
367-
cargo_metadata::MetadataCommand::parse(stdout)
369+
Ok((cargo_metadata::MetadataCommand::parse(stdout)?, None))
368370
})()
371+
.map(|(metadata, error)| {
372+
(
373+
metadata,
374+
error.map(|e| e.context(format!("Failed to run `{:?}`", meta.cargo_command()))),
375+
)
376+
})
369377
.with_context(|| format!("Failed to run `{:?}`", meta.cargo_command()))
370378
}
371379

@@ -463,7 +471,6 @@ impl CargoWorkspace {
463471
pkg_data.targets.push(tgt);
464472
}
465473
}
466-
let no_deps = meta.resolve.is_none();
467474
for mut node in meta.resolve.map_or_else(Vec::new, |it| it.nodes) {
468475
let &source = pkg_by_id.get(&node.id).unwrap();
469476
node.deps.sort_by(|a, b| a.pkg.cmp(&b.pkg));
@@ -483,14 +490,7 @@ impl CargoWorkspace {
483490

484491
let target_directory = AbsPathBuf::assert(meta.target_directory);
485492

486-
CargoWorkspace {
487-
packages,
488-
targets,
489-
workspace_root,
490-
target_directory,
491-
manifest_path,
492-
no_deps,
493-
}
493+
CargoWorkspace { packages, targets, workspace_root, target_directory, manifest_path }
494494
}
495495

496496
pub fn packages(&self) -> impl ExactSizeIterator<Item = Package> + '_ {
@@ -572,10 +572,6 @@ impl CargoWorkspace {
572572
fn is_unique(&self, name: &str) -> bool {
573573
self.packages.iter().filter(|(_, v)| v.name == name).count() == 1
574574
}
575-
576-
pub fn no_deps(&self) -> bool {
577-
self.no_deps
578-
}
579575
}
580576

581577
fn find_list_of_build_targets(

‎src/tools/rust-analyzer/crates/project-model/src/sysroot.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ impl Sysroot {
325325
"nightly".to_owned(),
326326
);
327327

328-
let mut res = match CargoWorkspace::fetch_metadata(
328+
let (mut res, _) = match CargoWorkspace::fetch_metadata(
329329
&library_manifest,
330330
sysroot_src_dir,
331331
&cargo_config,

‎src/tools/rust-analyzer/crates/project-model/src/tests.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ fn load_cargo_with_overrides(
3434
build_scripts: WorkspaceBuildScripts::default(),
3535
rustc: Err(None),
3636
cargo_config_extra_env: Default::default(),
37+
error: None,
3738
},
3839
cfg_overrides,
3940
sysroot: Sysroot::empty(),
@@ -58,6 +59,7 @@ fn load_cargo_with_fake_sysroot(
5859
build_scripts: WorkspaceBuildScripts::default(),
5960
rustc: Err(None),
6061
cargo_config_extra_env: Default::default(),
62+
error: None,
6163
},
6264
sysroot: get_fake_sysroot(),
6365
rustc_cfg: Vec::new(),
@@ -300,6 +302,7 @@ fn smoke_test_real_sysroot_cargo() {
300302
build_scripts: WorkspaceBuildScripts::default(),
301303
rustc: Err(None),
302304
cargo_config_extra_env: Default::default(),
305+
error: None,
303306
},
304307
sysroot,
305308
rustc_cfg: Vec::new(),

‎src/tools/rust-analyzer/crates/project-model/src/workspace.rs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ pub enum ProjectWorkspaceKind {
6969
Cargo {
7070
/// The workspace as returned by `cargo metadata`.
7171
cargo: CargoWorkspace,
72+
/// Additional `cargo metadata` error. (only populated if retried fetching via `--no-deps` succeeded).
73+
error: Option<Arc<anyhow::Error>>,
7274
/// The build script results for the workspace.
7375
build_scripts: WorkspaceBuildScripts,
7476
/// The rustc workspace loaded for this workspace. An `Err(None)` means loading has been
@@ -93,7 +95,7 @@ pub enum ProjectWorkspaceKind {
9395
/// The file in question.
9496
file: ManifestPath,
9597
/// Is this file a cargo script file?
96-
cargo: Option<(CargoWorkspace, WorkspaceBuildScripts)>,
98+
cargo: Option<(CargoWorkspace, WorkspaceBuildScripts, Option<Arc<anyhow::Error>>)>,
9799
/// Environment variables set in the `.cargo/config` file.
98100
cargo_config_extra_env: FxHashMap<String, String>,
99101
},
@@ -106,6 +108,7 @@ impl fmt::Debug for ProjectWorkspace {
106108
match kind {
107109
ProjectWorkspaceKind::Cargo {
108110
cargo,
111+
error: _,
109112
build_scripts: _,
110113
rustc,
111114
cargo_config_extra_env,
@@ -256,7 +259,7 @@ impl ProjectWorkspace {
256259
false,
257260
progress,
258261
) {
259-
Ok(meta) => {
262+
Ok((meta, _error)) => {
260263
let workspace = CargoWorkspace::new(meta, cargo_toml.clone());
261264
let buildscripts = WorkspaceBuildScripts::rustc_crates(
262265
&workspace,
@@ -301,7 +304,7 @@ impl ProjectWorkspace {
301304
tracing::error!(%e, "failed fetching data layout for {cargo_toml:?} workspace");
302305
}
303306

304-
let meta = CargoWorkspace::fetch_metadata(
307+
let (meta, error) = CargoWorkspace::fetch_metadata(
305308
cargo_toml,
306309
cargo_toml.parent(),
307310
config,
@@ -324,6 +327,7 @@ impl ProjectWorkspace {
324327
build_scripts: WorkspaceBuildScripts::default(),
325328
rustc,
326329
cargo_config_extra_env,
330+
error: error.map(Arc::new),
327331
},
328332
sysroot,
329333
rustc_cfg,
@@ -404,10 +408,11 @@ impl ProjectWorkspace {
404408
let cargo_script =
405409
CargoWorkspace::fetch_metadata(detached_file, dir, config, &sysroot, false, &|_| ())
406410
.ok()
407-
.map(|ws| {
411+
.map(|(ws, error)| {
408412
(
409413
CargoWorkspace::new(ws, detached_file.clone()),
410414
WorkspaceBuildScripts::default(),
415+
error.map(Arc::new),
411416
)
412417
});
413418

@@ -440,10 +445,8 @@ impl ProjectWorkspace {
440445
progress: &dyn Fn(String),
441446
) -> anyhow::Result<WorkspaceBuildScripts> {
442447
match &self.kind {
443-
ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. }
444-
| ProjectWorkspaceKind::Cargo { cargo, .. }
445-
if !cargo.no_deps() =>
446-
{
448+
ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, None)), .. }
449+
| ProjectWorkspaceKind::Cargo { cargo, error: None, .. } => {
447450
WorkspaceBuildScripts::run_for_workspace(config, cargo, progress, &self.sysroot)
448451
.with_context(|| {
449452
format!("Failed to run build scripts for {}", cargo.workspace_root())
@@ -502,7 +505,7 @@ impl ProjectWorkspace {
502505
pub fn set_build_scripts(&mut self, bs: WorkspaceBuildScripts) {
503506
match &mut self.kind {
504507
ProjectWorkspaceKind::Cargo { build_scripts, .. }
505-
| ProjectWorkspaceKind::DetachedFile { cargo: Some((_, build_scripts)), .. } => {
508+
| ProjectWorkspaceKind::DetachedFile { cargo: Some((_, build_scripts, _)), .. } => {
506509
*build_scripts = bs
507510
}
508511
_ => assert_eq!(bs, WorkspaceBuildScripts::default()),
@@ -593,6 +596,7 @@ impl ProjectWorkspace {
593596
rustc,
594597
build_scripts,
595598
cargo_config_extra_env: _,
599+
error: _,
596600
} => {
597601
cargo
598602
.packages()
@@ -648,7 +652,7 @@ impl ProjectWorkspace {
648652
include: vec![file.to_path_buf()],
649653
exclude: Vec::new(),
650654
})
651-
.chain(cargo_script.iter().flat_map(|(cargo, build_scripts)| {
655+
.chain(cargo_script.iter().flat_map(|(cargo, build_scripts, _)| {
652656
cargo.packages().map(|pkg| {
653657
let is_local = cargo[pkg].is_local;
654658
let pkg_root = cargo[pkg].manifest.parent().to_path_buf();
@@ -703,7 +707,7 @@ impl ProjectWorkspace {
703707
}
704708
ProjectWorkspaceKind::DetachedFile { cargo: cargo_script, .. } => {
705709
sysroot_package_len
706-
+ cargo_script.as_ref().map_or(1, |(cargo, _)| cargo.packages().len())
710+
+ cargo_script.as_ref().map_or(1, |(cargo, _, _)| cargo.packages().len())
707711
}
708712
}
709713
}
@@ -733,6 +737,7 @@ impl ProjectWorkspace {
733737
rustc,
734738
build_scripts,
735739
cargo_config_extra_env: _,
740+
error: _,
736741
} => (
737742
cargo_to_crate_graph(
738743
load,
@@ -746,7 +751,7 @@ impl ProjectWorkspace {
746751
sysroot,
747752
),
748753
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => (
749-
if let Some((cargo, build_scripts)) = cargo_script {
754+
if let Some((cargo, build_scripts, _)) = cargo_script {
750755
cargo_to_crate_graph(
751756
&mut |path| load(path),
752757
None,
@@ -795,12 +800,14 @@ impl ProjectWorkspace {
795800
rustc,
796801
cargo_config_extra_env,
797802
build_scripts: _,
803+
error: _,
798804
},
799805
ProjectWorkspaceKind::Cargo {
800806
cargo: o_cargo,
801807
rustc: o_rustc,
802808
cargo_config_extra_env: o_cargo_config_extra_env,
803809
build_scripts: _,
810+
error: _,
804811
},
805812
) => {
806813
cargo == o_cargo
@@ -813,12 +820,12 @@ impl ProjectWorkspace {
813820
(
814821
ProjectWorkspaceKind::DetachedFile {
815822
file,
816-
cargo: Some((cargo_script, _)),
823+
cargo: Some((cargo_script, _, _)),
817824
cargo_config_extra_env,
818825
},
819826
ProjectWorkspaceKind::DetachedFile {
820827
file: o_file,
821-
cargo: Some((o_cargo_script, _)),
828+
cargo: Some((o_cargo_script, _, _)),
822829
cargo_config_extra_env: o_cargo_config_extra_env,
823830
},
824831
) => {

‎src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ impl GlobalStateSnapshot {
667667
for workspace in self.workspaces.iter() {
668668
match &workspace.kind {
669669
ProjectWorkspaceKind::Cargo { cargo, .. }
670-
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } => {
670+
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
671671
let Some(target_idx) = cargo.target_by_root(path) else {
672672
continue;
673673
};

‎src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/notification.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
344344
let package = match &ws.kind {
345345
project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
346346
| project_model::ProjectWorkspaceKind::DetachedFile {
347-
cargo: Some((cargo, _)),
347+
cargo: Some((cargo, _, _)),
348348
..
349349
} => cargo.packages().find_map(|pkg| {
350350
let has_target_with_root = cargo[pkg]

‎src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ pub(crate) fn handle_parent_module(
799799
.iter()
800800
.filter_map(|ws| match &ws.kind {
801801
ProjectWorkspaceKind::Cargo { cargo, .. }
802-
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } => {
802+
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
803803
cargo.parent_manifests(&manifest_path)
804804
}
805805
_ => None,
@@ -1839,7 +1839,7 @@ pub(crate) fn handle_open_docs(
18391839

18401840
let ws_and_sysroot = snap.workspaces.iter().find_map(|ws| match &ws.kind {
18411841
ProjectWorkspaceKind::Cargo { cargo, .. }
1842-
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. } => {
1842+
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
18431843
Some((cargo, &ws.sysroot))
18441844
}
18451845
ProjectWorkspaceKind::Json { .. } => None,

‎src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -180,16 +180,17 @@ impl GlobalState {
180180
self.proc_macro_clients.iter().map(Some).chain(iter::repeat_with(|| None));
181181

182182
for (ws, proc_macro_client) in self.workspaces.iter().zip(proc_macro_clients) {
183-
if matches!(
184-
&ws.kind,
185-
ProjectWorkspaceKind::Cargo { cargo, .. } | ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _)), .. }
186-
if cargo.no_deps()
187-
) {
183+
if let ProjectWorkspaceKind::Cargo { error: Some(error), .. }
184+
| ProjectWorkspaceKind::DetachedFile {
185+
cargo: Some((_, _, Some(error))), ..
186+
} = &ws.kind
187+
{
188188
status.health |= lsp_ext::Health::Warning;
189189
format_to!(
190190
message,
191-
"Failed to read Cargo metadata for `{}`, the `Cargo.toml` might be invalid or you have no internet connection.\n\n",
192-
ws.manifest_or_root()
191+
"Failed to read Cargo metadata with dependencies for `{}`: {:#}\n\n",
192+
ws.manifest_or_root(),
193+
error
193194
);
194195
}
195196
if let Some(err) = ws.sysroot.error() {
@@ -822,7 +823,7 @@ impl GlobalState {
822823
match &ws.kind {
823824
ProjectWorkspaceKind::Cargo { cargo, .. }
824825
| ProjectWorkspaceKind::DetachedFile {
825-
cargo: Some((cargo, _)),
826+
cargo: Some((cargo, _, _)),
826827
..
827828
} => (cargo.workspace_root(), Some(cargo.manifest_path())),
828829
ProjectWorkspaceKind::Json(project) => {

‎src/tools/rust-analyzer/crates/rust-analyzer/tests/crate_graph.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ fn load_cargo_with_fake_sysroot(file: &str) -> ProjectWorkspace {
2020
build_scripts: WorkspaceBuildScripts::default(),
2121
rustc: Err(None),
2222
cargo_config_extra_env: Default::default(),
23+
error: None,
2324
},
2425
sysroot: get_fake_sysroot(),
2526
rustc_cfg: Vec::new(),

0 commit comments

Comments
 (0)