Skip to content

Commit d41ad2c

Browse files
VeykrilVeykril
authored
Merge pull request #18668 from Veykril/push-tpkmsyllunqv
fix: Fix sourceroot construction for virtual manifests
2 parents 5182170 + 0815dfb commit d41ad2c

File tree

7 files changed

+75
-75
lines changed

7 files changed

+75
-75
lines changed

‎src/tools/rust-analyzer/crates/load-cargo/src/lib.rs‎

Lines changed: 5 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ use ide_db::{
1515
};
1616
use itertools::Itertools;
1717
use proc_macro_api::{MacroDylib, ProcMacroServer};
18-
use project_model::{
19-
CargoConfig, PackageRoot, ProjectManifest, ProjectWorkspace, ProjectWorkspaceKind,
20-
};
18+
use project_model::{CargoConfig, PackageRoot, ProjectManifest, ProjectWorkspace};
2119
use span::Span;
2220
use vfs::{
2321
file_set::FileSetConfig,
@@ -244,6 +242,9 @@ impl ProjectFolders {
244242
}
245243
}
246244

245+
if dirs.include.is_empty() {
246+
continue;
247+
}
247248
vfs::loader::Entry::Directories(dirs)
248249
};
249250

@@ -258,43 +259,6 @@ impl ProjectFolders {
258259
fsc.add_file_set(file_set_roots)
259260
}
260261

261-
// register the workspace manifest as well, note that this currently causes duplicates for
262-
// non-virtual cargo workspaces! We ought to fix that
263-
for ws in workspaces.iter() {
264-
let mut file_set_roots: Vec<VfsPath> = vec![];
265-
let mut entries = vec![];
266-
267-
if let Some(manifest) = ws.manifest().map(|it| it.to_path_buf()) {
268-
file_set_roots.push(VfsPath::from(manifest.to_owned()));
269-
entries.push(manifest.to_owned());
270-
}
271-
272-
for buildfile in ws.buildfiles() {
273-
file_set_roots.push(VfsPath::from(buildfile.to_owned()));
274-
entries.push(buildfile.to_owned());
275-
}
276-
277-
// In case of detached files we do **not** look for a rust-analyzer.toml.
278-
if !matches!(ws.kind, ProjectWorkspaceKind::DetachedFile { .. }) {
279-
let ws_root = ws.workspace_root();
280-
let ratoml_path = {
281-
let mut p = ws_root.to_path_buf();
282-
p.push("rust-analyzer.toml");
283-
p
284-
};
285-
file_set_roots.push(VfsPath::from(ratoml_path.to_owned()));
286-
entries.push(ratoml_path.to_owned());
287-
}
288-
289-
if !file_set_roots.is_empty() {
290-
let entry = vfs::loader::Entry::Files(entries);
291-
res.watch.push(res.load.len());
292-
res.load.push(entry);
293-
local_filesets.push(fsc.len() as u64);
294-
fsc.add_file_set(file_set_roots)
295-
}
296-
}
297-
298262
if let Some(user_config_path) = user_config_dir_path {
299263
let ratoml_path = {
300264
let mut p = user_config_path.to_path_buf();
@@ -303,7 +267,7 @@ impl ProjectFolders {
303267
};
304268

305269
let file_set_roots = vec![VfsPath::from(ratoml_path.to_owned())];
306-
let entry = vfs::loader::Entry::Files(vec![ratoml_path.to_owned()]);
270+
let entry = vfs::loader::Entry::Files(vec![ratoml_path]);
307271

308272
res.watch.push(res.load.len());
309273
res.load.push(entry);

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ pub struct CargoWorkspace {
3333
workspace_root: AbsPathBuf,
3434
target_directory: AbsPathBuf,
3535
manifest_path: ManifestPath,
36+
is_virtual_workspace: bool,
3637
}
3738

3839
impl ops::Index<Package> for CargoWorkspace {
@@ -384,13 +385,20 @@ impl CargoWorkspace {
384385
.with_context(|| format!("Failed to run `{:?}`", meta.cargo_command()))
385386
}
386387

387-
pub fn new(mut meta: cargo_metadata::Metadata, manifest_path: ManifestPath) -> CargoWorkspace {
388+
pub fn new(
389+
mut meta: cargo_metadata::Metadata,
390+
ws_manifest_path: ManifestPath,
391+
) -> CargoWorkspace {
388392
let mut pkg_by_id = FxHashMap::default();
389393
let mut packages = Arena::default();
390394
let mut targets = Arena::default();
391395

392396
let ws_members = &meta.workspace_members;
393397

398+
let workspace_root = AbsPathBuf::assert(meta.workspace_root);
399+
let target_directory = AbsPathBuf::assert(meta.target_directory);
400+
let mut is_virtual_workspace = true;
401+
394402
meta.packages.sort_by(|a, b| a.id.cmp(&b.id));
395403
for meta_pkg in meta.packages {
396404
let cargo_metadata::Package {
@@ -429,12 +437,13 @@ impl CargoWorkspace {
429437
let is_local = source.is_none();
430438
let is_member = ws_members.contains(&id);
431439

432-
let manifest = AbsPathBuf::assert(manifest_path);
440+
let manifest = ManifestPath::try_from(AbsPathBuf::assert(manifest_path)).unwrap();
441+
is_virtual_workspace &= manifest != ws_manifest_path;
433442
let pkg = packages.alloc(PackageData {
434443
id: id.repr.clone(),
435444
name,
436445
version,
437-
manifest: manifest.clone().try_into().unwrap(),
446+
manifest: manifest.clone(),
438447
targets: Vec::new(),
439448
is_local,
440449
is_member,
@@ -468,7 +477,7 @@ impl CargoWorkspace {
468477
// modified manifest file into a special target dir which is then used as
469478
// the source path. We don't want that, we want the original here so map it
470479
// back
471-
manifest.clone()
480+
manifest.clone().into()
472481
} else {
473482
AbsPathBuf::assert(src_path)
474483
},
@@ -493,11 +502,14 @@ impl CargoWorkspace {
493502
packages[source].active_features.extend(node.features);
494503
}
495504

496-
let workspace_root = AbsPathBuf::assert(meta.workspace_root);
497-
498-
let target_directory = AbsPathBuf::assert(meta.target_directory);
499-
500-
CargoWorkspace { packages, targets, workspace_root, target_directory, manifest_path }
505+
CargoWorkspace {
506+
packages,
507+
targets,
508+
workspace_root,
509+
target_directory,
510+
manifest_path: ws_manifest_path,
511+
is_virtual_workspace,
512+
}
501513
}
502514

503515
pub fn packages(&self) -> impl ExactSizeIterator<Item = Package> + '_ {
@@ -579,6 +591,10 @@ impl CargoWorkspace {
579591
fn is_unique(&self, name: &str) -> bool {
580592
self.packages.iter().filter(|(_, v)| v.name == name).count() == 1
581593
}
594+
595+
pub fn is_virtual_workspace(&self) -> bool {
596+
self.is_virtual_workspace
597+
}
582598
}
583599

584600
fn find_list_of_build_targets(

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ impl TryFrom<AbsPathBuf> for ManifestPath {
2929
}
3030
}
3131

32+
impl From<ManifestPath> for AbsPathBuf {
33+
fn from(it: ManifestPath) -> Self {
34+
it.file
35+
}
36+
}
37+
3238
impl ManifestPath {
3339
// Shadow `parent` from `Deref`.
3440
pub fn parent(&self) -> &AbsPath {

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

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ use base_db::{
1111
};
1212
use cfg::{CfgAtom, CfgDiff, CfgOptions};
1313
use intern::{sym, Symbol};
14+
use itertools::Itertools;
1415
use paths::{AbsPath, AbsPathBuf};
15-
use rustc_hash::{FxHashMap, FxHashSet};
16+
use rustc_hash::FxHashMap;
1617
use semver::Version;
1718
use span::{Edition, FileId};
1819
use toolchain::Tool;
@@ -41,7 +42,9 @@ pub type FileLoader<'a> = &'a mut dyn for<'b> FnMut(&'b AbsPath) -> Option<FileI
4142
pub struct PackageRoot {
4243
/// Is from the local filesystem and may be edited
4344
pub is_local: bool,
45+
/// Directories to include
4446
pub include: Vec<AbsPathBuf>,
47+
/// Directories to exclude
4548
pub exclude: Vec<AbsPathBuf>,
4649
}
4750

@@ -553,17 +556,6 @@ impl ProjectWorkspace {
553556
}
554557
}
555558

556-
pub fn buildfiles(&self) -> Vec<AbsPathBuf> {
557-
match &self.kind {
558-
ProjectWorkspaceKind::Json(project) => project
559-
.crates()
560-
.filter_map(|(_, krate)| krate.build.as_ref().map(|build| build.build_file.clone()))
561-
.map(|build_file| self.workspace_root().join(build_file))
562-
.collect(),
563-
_ => vec![],
564-
}
565-
}
566-
567559
pub fn find_sysroot_proc_macro_srv(&self) -> anyhow::Result<AbsPathBuf> {
568560
self.sysroot.discover_proc_macro_srv()
569561
}
@@ -608,15 +600,25 @@ impl ProjectWorkspace {
608600
match &self.kind {
609601
ProjectWorkspaceKind::Json(project) => project
610602
.crates()
611-
.map(|(_, krate)| PackageRoot {
612-
is_local: krate.is_workspace_member,
613-
include: krate.include.clone(),
614-
exclude: krate.exclude.clone(),
603+
.map(|(_, krate)| {
604+
let build_files = project
605+
.crates()
606+
.filter_map(|(_, krate)| {
607+
krate.build.as_ref().map(|build| build.build_file.clone())
608+
})
609+
// FIXME: PackageRoots dont allow specifying files, only directories
610+
.filter_map(|build_file| {
611+
self.workspace_root().join(build_file).parent().map(ToOwned::to_owned)
612+
});
613+
PackageRoot {
614+
is_local: krate.is_workspace_member,
615+
include: krate.include.iter().cloned().chain(build_files).collect(),
616+
exclude: krate.exclude.clone(),
617+
}
615618
})
616-
.collect::<FxHashSet<_>>()
617-
.into_iter()
618619
.chain(mk_sysroot())
619-
.collect::<Vec<_>>(),
620+
.unique()
621+
.collect(),
620622
ProjectWorkspaceKind::Cargo {
621623
cargo,
622624
rustc,
@@ -671,6 +673,11 @@ impl ProjectWorkspace {
671673
exclude: Vec::new(),
672674
})
673675
}))
676+
.chain(cargo.is_virtual_workspace().then(|| PackageRoot {
677+
is_local: true,
678+
include: vec![cargo.workspace_root().to_path_buf()],
679+
exclude: Vec::new(),
680+
}))
674681
.collect()
675682
}
676683
ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, .. } => {

‎src/tools/rust-analyzer/crates/rust-analyzer/src/bin/main.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ fn actual_main() -> anyhow::Result<ExitCode> {
5151
}
5252
}
5353

54-
setup_logging(flags.log_file.clone())?;
54+
if let Err(e) = setup_logging(flags.log_file.clone()) {
55+
eprintln!("Failed to setup logging: {e:#}");
56+
}
5557

5658
let verbosity = flags.verbosity();
5759

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,7 @@ impl Config {
827827
let mut should_update = false;
828828

829829
if let Some(change) = change.user_config_change {
830+
tracing::info!("updating config from user config toml: {:#}", change);
830831
if let Ok(table) = toml::from_str(&change) {
831832
let mut toml_errors = vec![];
832833
validate_toml_table(
@@ -919,7 +920,7 @@ impl Config {
919920
RatomlFileKind::Crate => {
920921
if let Some(text) = text {
921922
let mut toml_errors = vec![];
922-
tracing::info!("updating ra-toml config: {:#}", text);
923+
tracing::info!("updating ra-toml crate config: {:#}", text);
923924
match toml::from_str(&text) {
924925
Ok(table) => {
925926
validate_toml_table(
@@ -961,6 +962,7 @@ impl Config {
961962
}
962963
RatomlFileKind::Workspace => {
963964
if let Some(text) = text {
965+
tracing::info!("updating ra-toml workspace config: {:#}", text);
964966
let mut toml_errors = vec![];
965967
match toml::from_str(&text) {
966968
Ok(table) => {

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,8 +417,10 @@ impl GlobalState {
417417
})
418418
.collect_vec();
419419

420-
for (file_id, (_change_kind, vfs_path)) in modified_ratoml_files {
420+
for (file_id, (change_kind, vfs_path)) in modified_ratoml_files {
421+
tracing::info!(%vfs_path, ?change_kind, "Processing rust-analyzer.toml changes");
421422
if vfs_path.as_path() == user_config_abs_path {
423+
tracing::info!(%vfs_path, ?change_kind, "Use config rust-analyzer.toml changes");
422424
change.change_user_config(Some(db.file_text(file_id)));
423425
continue;
424426
}
@@ -430,12 +432,14 @@ impl GlobalState {
430432

431433
if !sr.is_library {
432434
let entry = if workspace_ratoml_paths.contains(&vfs_path) {
435+
tracing::info!(%vfs_path, ?sr_id, "workspace rust-analyzer.toml changes");
433436
change.change_workspace_ratoml(
434437
sr_id,
435438
vfs_path.clone(),
436439
Some(db.file_text(file_id)),
437440
)
438441
} else {
442+
tracing::info!(%vfs_path, ?sr_id, "crate rust-analyzer.toml changes");
439443
change.change_ratoml(
440444
sr_id,
441445
vfs_path.clone(),
@@ -446,7 +450,7 @@ impl GlobalState {
446450
if let Some((kind, old_path, old_text)) = entry {
447451
// SourceRoot has more than 1 RATOML files. In this case lexicographically smaller wins.
448452
if old_path < vfs_path {
449-
span!(Level::ERROR, "Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect.");
453+
tracing::error!("Two `rust-analyzer.toml` files were found inside the same crate. {vfs_path} has no effect.");
450454
// Put the old one back in.
451455
match kind {
452456
RatomlFileKind::Crate => {
@@ -459,8 +463,7 @@ impl GlobalState {
459463
}
460464
}
461465
} else {
462-
// Mapping to a SourceRoot should always end up in `Ok`
463-
span!(Level::ERROR, "Mapping to SourceRootId failed.");
466+
tracing::info!(%vfs_path, "Ignoring library rust-analyzer.toml");
464467
}
465468
}
466469
change.change_source_root_parent_map(self.local_roots_parent_map.clone());

0 commit comments

Comments
 (0)