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

Mitigate & document problems with mixed generated sources #3127

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 17 additions & 5 deletions rust/private/rust_analyzer.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,24 @@ def _create_single_crate(ctx, attrs, info):
crate["root_module"] = path_prefix + info.crate.root.path
crate["source"] = {"exclude_dirs": [], "include_dirs": []}

# If some sources are generated and others are not, paths are complicated.
#
# bazel splits the files across bazel-out/package/* and workspace/package/*,
# but rustc requires the sources to all be in one directory.
# To address this, rules_rust adds symlinks: bazel-out/package/* => workspace/package/*
#
# If we specify one of the symlinks as our root_module, then rust-analyzer can index the crate,
# but won't recognize files in the workspace as being part of it, so features won't work.
# If we specify the path within the workspace, it won't manage to resolve the generated modules.
#
# There's no perfect solution here, for now we choose to try to find a workspace path.
# https://github.com/bazelbuild/rules_rust/issues/3126
if is_generated:
srcs = getattr(ctx.rule.files, "srcs", [])
src_map = {src.short_path: src for src in srcs if src.is_source}
if info.crate.root.short_path in src_map:
crate["root_module"] = _WORKSPACE_TEMPLATE + src_map[info.crate.root.short_path].path
crate["source"]["include_dirs"].append(path_prefix + info.crate.root.dirname)
# 'root' is a transformed source, likely a symlink.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be if is_generated and not is_external ?
Or this logic also applies to external repositories?

# 'srcs' are what the user originally specified, likely workspace paths.
for src in getattr(ctx.rule.files, "srcs", []):
if src.is_source and info.crate.root.short_path == src.short_path:
crate["root_module"] = _WORKSPACE_TEMPLATE + src.path

if info.build_info != None and info.build_info.out_dir != None:
out_dir_path = info.build_info.out_dir.path
Expand Down
32 changes: 9 additions & 23 deletions test/rust_analyzer/generated_srcs_test/rust_project_json_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,13 @@ mod tests {

#[derive(Deserialize)]
struct Project {
sysroot_src: String,
crates: Vec<Crate>,
}

#[derive(Deserialize)]
struct Crate {
display_name: String,
root_module: String,
source: Option<Source>,
}

#[derive(Deserialize)]
struct Source {
include_dirs: Vec<String>,
}

#[test]
Expand All @@ -31,27 +24,20 @@ mod tests {
let project: Project =
serde_json::from_str(&content).expect("Failed to deserialize project JSON");

// /tmp/_bazel/12345678/external/tools/rustlib/library => /tmp/_bazel
let output_base = project
.sysroot_src
.rsplitn(2, "/external/")
.last()
.unwrap()
.rsplitn(2, '/')
.last()
.unwrap();
println!("output_base: {output_base}");

let gen = project
.crates
.iter()
.find(|c| &c.display_name == "generated_srcs")
.unwrap();
assert!(gen.root_module.starts_with("/"));
assert!(gen.root_module.ends_with("/lib.rs"));

let include_dirs = &gen.source.as_ref().unwrap().include_dirs;
assert!(include_dirs.len() == 1);
assert!(include_dirs[0].starts_with(output_base));
// This target has mixed generated+plain sources, so rules_rust provides a
// directory where the root module is a symlink.
// However in the crate spec, we want the root_module to be a workspace path
// when possible.
let workspace_path = PathBuf::from(env::var("WORKSPACE").unwrap());
assert_eq!(
gen.root_module,
workspace_path.join("lib.rs").to_string_lossy()
);
}
}
2 changes: 1 addition & 1 deletion test/rust_analyzer/rust_analyzer_test_runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function rust_analyzer_test() {
echo "Building..."
bazel build //...
echo "Testing..."
bazel test //...
bazel test --test_env=WORKSPACE="${workspace}" //...
echo "Building with Aspects..."
bazel build //... --config=strict
popd &>/dev/null
Expand Down
21 changes: 12 additions & 9 deletions tools/rust_analyzer/aquery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,20 +176,23 @@ fn consolidate_crate_specs(crate_specs: Vec<CrateSpec>) -> anyhow::Result<BTreeS
for mut spec in crate_specs.into_iter() {
log::debug!("{:?}", spec);
if let Some(existing) = consolidated_specs.get_mut(&spec.crate_id) {
// Prefer rlib crates over bin crates (typically tests):
// - the display name that matches the crate name (foo vs foo_test)
// Rust Analyzer seems to use display_name for matching crate entries
// in rust-project.json against symbols in source files.
// https://github.com/bazelbuild/rules_rust/issues/1032
// - test crates may report a genfiles path as the root_module, rather
// than the original source file in the workspace.
// This causes rust-analyzer to not recognize the workspace file.
// https://github.com/bazelbuild/rules_rust/issues/3126
if spec.crate_type == "rlib" {
std::mem::swap(&mut spec, existing);
}
existing.deps.extend(spec.deps);

spec.cfg.retain(|cfg| !existing.cfg.contains(cfg));
existing.cfg.extend(spec.cfg);

// display_name should match the library's crate name because Rust Analyzer
// seems to use display_name for matching crate entries in rust-project.json
// against symbols in source files. For more details, see
// https://github.com/bazelbuild/rules_rust/issues/1032
if spec.crate_type == "rlib" {
existing.display_name = spec.display_name;
existing.crate_type = "rlib".into();
}

// For proc-macro crates that exist within the workspace, there will be a
// generated crate-spec in both the fastbuild and opt-exec configuration.
// Prefer proc macro paths with an opt-exec component in the path.
Expand Down
Loading