Closed fasterthanlime closed 2 years ago
A clean fix for that would require https://github.com/rust-lang/cargo/issues/3946 to be tackled, but the workaround suggested in https://github.com/rust-lang/cargo/issues/3946#issuecomment-749645286 doesn't seem terrible to me:
$ pwd
/home/amos/bearcove/rust/src/tools/rust-analyzer/crates/ide/src/syntax_highlighting
$ cargo locate-project --workspace --message-format plain
/home/amos/bearcove/rust/src/tools/rust-analyzer/Cargo.toml
Running cargo from a OnceCell
initialization sounds a bit weird, though from the issue it does look like the only viable option... Given this is stuff that only runs in test environments it should be fine to swap to that nevertheless.
An alternative fix for our specific situation would be to simply set an environment variable (maybe named CARGO_WORKSPACE
) from x.py and use that if present :thinking:
That would also be a nice alternative, assuming the people responsible for x.py
are fine with adding that
As another alternative, this hack doesn't involve calling out to cargo: https://github.com/rust-lang/cargo/issues/3946#issuecomment-973132993
The idea would be to create a rust-analyzer/.cargo/config.toml
file with this:
[env]
CARGO_WORKSPACE_DIR = { value = "", relative = true }
Then the env var would be set when running cargo test
.
edit: This would still involve changes to expect-test
, but not to x.py
.
Yeah, I like that a bit more than changing x.py since it will also work when you just run cargo test
inside the rust-analyzer submodule.
Yeah, I like that a bit more than changing x.py since it will also work when you just run
cargo test
inside the rust-analyzer submodule.
Sounds good, I'll open an expect-test
PR.
... or we could merge #30, I guess? :thinking:
I don't think that PR fixes the issue, if I understand this right the NevermindCARGO_MANIFEST_DIR
env var is set to a cargo toml that is a lot further up the file tree which manifests this issue here?
... or we could merge #30, I guess? 🤔
Mhh I don't love the adhoc-toml-parsing there, it'll fail for (admittedly fringe) cases like this:
workspace = { members = [
"xtask/",
"lib/*",
"crates/*",
], exclude = [
"crates/proc-macro-test/imp",
] }
Mhh I don't love the adhoc-toml-parsing there
I'd like that PR more if it brought a real TOML parser with it but now that's overkill, and that energy is better spent pushing for the cargo bug to be fixed instead imho.
That's true. I'm fine with either (or both).
I think I prefer the .cargo/config.toml
approach, our heuristic should work fine for most usages of the lib, and where it breaks its a simple one line addition to the config.toml
to get around the heuristic.
Follow-up RA PR:
Running RA tests from within a
rust-lang/rust
checkout fails, becauseexpect-test
climbs all the way up torust/Cargo.toml
instead of being happy withrust/src/tools/rust-analyzer/Cargo.toml
, and then the paths are all wrong.See https://github.com/rust-lang/rust/pull/99444#issuecomment-1188844202 for real-world details.