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

rust_analyzer: unpredictable/broken behavior with generated sources #3126

Open
sam-mccall opened this issue Dec 20, 2024 · 1 comment · May be fixed by #3127
Open

rust_analyzer: unpredictable/broken behavior with generated sources #3126

sam-mccall opened this issue Dec 20, 2024 · 1 comment · May be fixed by #3127

Comments

@sam-mccall
Copy link
Contributor

At a3778c5, the rust-analyzer generated_srcs_test fails on my machine, complaining about missing source.include_dirs:

log
$ bazel run test/rust_analyzer:rust_analyzer_test

Building with Aspects...
INFO: Analyzed 15 targets (1 packages loaded, 19 targets configured).
INFO: Found 15 targets...
INFO: Elapsed time: 6.054s, Critical Path: 0.41s
INFO: 25 processes: 1 internal, 24 linux-sandbox.
INFO: Build completed successfully, 25 total actions
Testing 'generated_srcs_test'
Generating rust-project.json...
INFO: Analyzed target @@rules_rust+//tools/rust_analyzer:gen_rust_project (44 packages loaded, 3760 targets configured).
INFO: Found 1 target...
Target @@rules_rust+//tools/rust_analyzer:gen_rust_project up-to-date:
  bazel-bin/external/rules_rust+/tools/rust_analyzer/gen_rust_project
INFO: Elapsed time: 0.307s, Critical Path: 0.01s
INFO: 1 process: 110 action cache hit, 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/external/rules_rust+/tools/rust_analyzer/gen_rust_project
Building...
INFO: Analyzed 5 targets (0 packages loaded, 0 targets configured).
INFO: Found 5 targets...
INFO: Elapsed time: 0.344s, Critical Path: 0.21s
INFO: 9 processes: 3 action cache hit, 6 internal, 3 linux-sandbox.
INFO: Build completed successfully, 9 total actions
Testing...
INFO: Analyzed 5 targets (0 packages loaded, 0 targets configured).
FAIL: //:rust_project_json_test (Exit 101) (see /usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall/3852688809b88d0967f87b3807dc2120/execroot/_main/bazel-out/k8-fastbuild/testlogs/rust_project_json_test/test.log)
INFO: From Testing //:rust_project_json_test:
==================== Test output for //:rust_project_json_test:

running 1 test
test tests::test_generated_srcs ... FAILED

failures:

---- tests::test_generated_srcs stdout ----
{
  "sysroot": "/usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall/3852688809b88d0967f87b3807dc2120/external/rules_rust++rust+rust_analyzer_1.83.0_tools",
  "sysroot_src": "/usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall/3852688809b88d0967f87b3807dc2120/external/rules_rust++rust+rust_analyzer_1.83.0_tools/lib/rustlib/src/library",
  "crates": [
    {
      "display_name": "generated_srcs",
      "root_module": "/usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall/3852688809b88d0967f87b3807dc2120/execroot/_main/bazel-out/k8-fastbuild/bin/lib.rs",
      "edition": "2021",
      "deps": [],
      "is_workspace_member": true,
      "cfg": [
        "test",
        "debug_assertions"
      ],
      "target": "x86_64-unknown-linux-gnu",
      "env": {
        "CARGO_CFG_TARGET_ARCH": "x86_64",
        "CARGO_CFG_TARGET_OS": "linux",
        "CARGO_CRATE_NAME": "generated_srcs",
        "CARGO_MANIFEST_DIR": "/usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall/3852688809b88d0967f87b3807dc2120/execroot/_main",
        "CARGO_PKG_AUTHORS": "",
        "CARGO_PKG_DESCRIPTION": "",
        "CARGO_PKG_HOMEPAGE": "",
        "CARGO_PKG_NAME": "generated_srcs",
        "CARGO_PKG_VERSION": "0.0.0",
        "CARGO_PKG_VERSION_MAJOR": "0",
        "CARGO_PKG_VERSION_MINOR": "0",
        "CARGO_PKG_VERSION_PATCH": "0",
        "CARGO_PKG_VERSION_PRE": "",
        "REPOSITORY_NAME": "",
        "ZERO_AR_DATE": "1"
      },
      "is_proc_macro": false
    },
    [...]
  ]
}
output_base: /usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall
thread 'tests::test_generated_srcs' panicked at rust_project_json_test.rs:53:49:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::test_generated_srcs

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

================================================================================
INFO: Found 3 targets and 2 test targets...
INFO: Elapsed time: 0.207s, Critical Path: 0.07s
INFO: 3 processes: 4 linux-sandbox.
INFO: Build completed, 1 test FAILED, 3 total actions
//:generated_srcs_test                                                   PASSED in 0.0s
//:rust_project_json_test                                                FAILED in 0.0s
  /usr/local/google/home/sammccall/.cache/bazel/_bazel_sammccall/3852688809b88d0967f87b3807dc2120/execroot/_main/bazel-out/k8-fastbuild/testlogs/rust_project_json_test/test.log

Executed 2 out of 2 tests: 1 test passes and 1 fails locally.

Running the aspect by hand shows there are two crate specs produced, one for the library and one for the test:

  • the library spec has successfully remapped the root_module lib.rs from a generated file to a source file in srcs, and so has a __WORKSPACE__ path with a source.include_dir under __EXEC_ROOT__.
  • the test spec has not performed this remapping (there are no srcs here), and root_module has a __EXEC_ROOT__ path with no source.include_dir.
  • the "consolidate" logic picks one 'at random', and my machine is unlucky
generated_srcs.json
{
    "aliases": {},
    "cfg": [
        "test",
        "debug_assertions"
    ],
    "crate_id": "ID-bazel-out/k8-fastbuild/bin/test/rust_analyzer/generated_srcs_test/lib.rs",
    "crate_type": "rlib",
    "deps": [],
    "display_name": "generated_srcs",
    "edition": "2021",
    "env": {
        "CARGO_CFG_TARGET_ARCH": "x86_64",
        "CARGO_CFG_TARGET_OS": "linux",
        "CARGO_CRATE_NAME": "generated_srcs",
        "CARGO_MANIFEST_DIR": "${pwd}/test/rust_analyzer/generated_srcs_test",
        "CARGO_PKG_AUTHORS": "",
        "CARGO_PKG_DESCRIPTION": "",
        "CARGO_PKG_HOMEPAGE": "",
        "CARGO_PKG_NAME": "generated_srcs",
        "CARGO_PKG_VERSION": "0.0.0",
        "CARGO_PKG_VERSION_MAJOR": "0",
        "CARGO_PKG_VERSION_MINOR": "0",
        "CARGO_PKG_VERSION_PATCH": "0",
        "CARGO_PKG_VERSION_PRE": "",
        "REPOSITORY_NAME": ""
    },
    "is_workspace_member": true,
    "root_module": "__WORKSPACE__/test/rust_analyzer/generated_srcs_test/lib.rs",
    "source": {
        "exclude_dirs": [],
        "include_dirs": [
            "__EXEC_ROOT__/bazel-out/k8-fastbuild/bin/test/rust_analyzer/generated_srcs_test"
        ]
    },
    "target": "x86_64-unknown-linux-gnu"
}
generated_srcs_test.json
{
    "aliases": {},
    "cfg": [
        "test",
        "debug_assertions"
    ],
    "crate_id": "ID-bazel-out/k8-fastbuild/bin/test/rust_analyzer/generated_srcs_test/lib.rs",
    "crate_type": "bin",
    "deps": [],
    "display_name": "generated_srcs",
    "edition": "2021",
    "env": {
        "CARGO_CFG_TARGET_ARCH": "x86_64",
        "CARGO_CFG_TARGET_OS": "linux",
        "CARGO_CRATE_NAME": "generated_srcs",
        "CARGO_MANIFEST_DIR": "${pwd}/test/rust_analyzer/generated_srcs_test",
        "CARGO_PKG_AUTHORS": "",
        "CARGO_PKG_DESCRIPTION": "",
        "CARGO_PKG_HOMEPAGE": "",
        "CARGO_PKG_NAME": "generated_srcs",
        "CARGO_PKG_VERSION": "0.0.0",
        "CARGO_PKG_VERSION_MAJOR": "0",
        "CARGO_PKG_VERSION_MINOR": "0",
        "CARGO_PKG_VERSION_PATCH": "0",
        "CARGO_PKG_VERSION_PRE": "",
        "REPOSITORY_NAME": "",
        "ZERO_AR_DATE": "1"
    },
    "is_workspace_member": false,
    "root_module": "__EXEC_ROOT__/bazel-out/k8-fastbuild/bin/test/rust_analyzer/generated_srcs_test/lib.rs",
    "source": {
        "exclude_dirs": [],
        "include_dirs": []
    },
    "target": "x86_64-unknown-linux-gnu"
}                                                                                               

Digging deeper into this, it seems both are broken, and it's not clear that making the test pass is a real fix.

  • the reason we have generated sources: rustc requires a crate's sources to be in the same dir. If some are generated and others are not, rust_library will generate extra symlinks so all files are in the generated tree.
  • the reason we try to restore the original path: if rust-project.json describes a crate rooted at blaze-out/gen/foo.rs, but you open workspace/foo.rs in your editor, rust-analyzer doesn't know it's the same file.
  • but restoring the original path doesn't actually work, because rust-analyzer also requires the crate's sources to be in the same dir. When we open workspace/foo.rs and see pub mod generated, rust-analyzer looks for workspace/generated.rs, and can't find it, because it's actually blaze-out/gen/generated.rs.

So if we don't remap the path, we get a rust-project.json that "works" if you open all the files through their generated locations, but rust-analyzer doesn't know which crates your regular source files belong to.
If we do remap the path, we get a rust-project.json where rust-analyzer knows which crates your files are part of, but can't actually understand them because generated files are missing.

(The existing logic tries to point include_dirs to the generated directory when there might be sources there, but I don't think this actually achieves anything: this isn't a C-style #include path, rather each module is resolved relative to the current one.)


I'm not really sure what to do about this. If we have to choose, there are arguments for either behavior:

  • if you're not directly editing the crate with generated sources, but rather something that depends on it, using the generated sources is way better: you get correct semantic analysis.
  • if you are, then probably partly-broken results are better than none at all

Maybe there's a clean solution that someone smarter than me can find...

sam-mccall added a commit to sam-mccall/rules_rust that referenced this issue Dec 20, 2024
- deterministically prefer rust_library's crate spec over rust_test's.
  This means root_module gets remapped to a workspace path more often,
  and fixes flakiness of generated_srcs_test.
- document the reasons and tradeoffs for this remapping.
- stop adding `include_dirs` when remapping, it doesn't do anything to
  help with this problem, and is confusing.
- fix test to strictly assert the path, which I broke in
  74f164b

Partially fixes bazelbuild#3126
@sam-mccall
Copy link
Contributor Author

#3127 attempts to stabilize/document the current situation.
I don't have better ideas but happy to drop it if someone else does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant