Rust Analyzer: Generate rust-project.json without explicit target list #1010
Conversation
UebelAndre
force-pushed
the
rust_analyzer
branch
5 times, most recently
from
a47ecde
to
eaa3fe7
Compare
|
@hlopko Hey, I'd really love to get these changes in so we can continue to get more feedback on |
UebelAndre
force-pushed
the
rust_analyzer
branch
from
eaa3fe7
to
4d9638b
Compare
|
@hlopko for context on my involvement here. I've been interested in these changes for a while and saw there was no activity on the original PR. I spent a few late nights to test the changes out and to write some integration tests, which to my understanding were the biggest missing component to the last PR. I agree with some of the changes you're proposing but since my experience with this code is mostly as a user, some of the proposals are not quick changes for me. Are these things that can be done post-merge, given that the |
|
That said, I can vouch for these changes in my workspaces and they're awesome. Even enabled it for |
There was a problem hiding this comment.
Thanks so much for picking this up @UebelAndre, let me know if you need any assistance finalizing this PR.
Use bazel-like space separated target specification
|
Super pumped about trying this out, good work! |
| echo "Testing..." | ||
| bazel test //... | ||
| echo "Building with Aspects..." | ||
| bazel build //... --config=strict |
There was a problem hiding this comment.
Do you need the build, test, and build --config=strict?
There was a problem hiding this comment.
Oh I think I see -- it copies in the WORKSPACE and BUILD.bazel files and then the test //... ends up running the rust-analyzer manual tests because the manual tag gets stripped out.
|
|
||
| pushd "${workspace}" &> /dev/null | ||
| echo "Generating rust-project.json..." | ||
| bazel run "@rules_rust//tools/rust_analyzer:gen_rust_project" |
There was a problem hiding this comment.
This seems to just validate that gen_rust_project runs at all and doesn't validate the output, is that correct?
There was a problem hiding this comment.
The test targets the selves consume a rust-project.json which should be doing that validation. Otherwise what are the tests testing?
bazelbuild#1010) * Emit rust_analyzer_crate_spec files for all workspace crates * Regenerated cargo-raze outputs * Address PR feedback * Increase Bazel min test version * Addressed PR feedback * Make sysroot_src a data dependency * Fix runfiles path specifier * Use bazel-like space separated target specification * Fix out-of-date comment * Addressed PR feedback * Added additional testing * Added test comment Co-authored-by: David Marcin <david@metawork.com>
This is a follow up of #907 (special thanks to @djmarcin!!) but with some comments addressed and an integration test added.
This change increases the min supported Bazel version from
3.5.0to4.0.0(this is a factor from the original PR).