Add Bazel rules for generating rust-project.json #1019
Conversation
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable, and 1 discussions need to be resolved (waiting on @aaronmondal)
BUILD.bazel line 77 at r1 (raw file):
name = "gen_rust_project", actual = "@rules_rust//tools/rust_analyzer:gen_rust_project", )
This was easier than making a genrule (or, whatever it is I would need to do) to wrap gen_rust_project, but I'm happy to change it if necessary.
Code quote:
alias(
name = "gen_rust_project",
actual = "@rules_rust//tools/rust_analyzer:gen_rust_project",
)
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and 2 discussions need to be resolved
BUILD.bazel line 77 at r1 (raw file):
Previously, caass (Cass Fridkin) wrote…
This was easier than making a genrule (or, whatever it is I would need to do) to wrap
gen_rust_project, but I'm happy to change it if necessary.
Instead of a target we could just drop a line like this into CONTRIBUTING.md. This way we save one build-level target and have documentation at the same time:
bazel run @rules_rust//tools/rust_analyzer:gen_rust_projectOr am I misunderstanding and this is supposed to run on every build?
FYI generally I'd say the alias approach is fine and actually more desirable than genrules. A genrule introduces an additional target that could potentially cause issues when run in remote execution. With an alias you save one level of indirection.
MODULE.bazel line 80 at r1 (raw file):
use_repo(crate, "crates") rust_analyzer = use_extension("@rules_rust//tools/rust_analyzer:extension.bzl", "rust_analyzer_dependencies")
Suggestion:
rust_analyzer = use_extension(
"@rules_rust//tools/rust_analyzer:extension.bzl",
"rust_analyzer_dependencies",
)
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and 2 discussions need to be resolved (waiting on @aaronmondal)
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and 2 discussions need to be resolved (waiting on @aaronmondal)
BUILD.bazel line 77 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Instead of a target we could just drop a line like this into CONTRIBUTING.md. This way we save one build-level target and have documentation at the same time:
bazel run @rules_rust//tools/rust_analyzer:gen_rust_projectOr am I misunderstanding and this is supposed to run on every build?
FYI generally I'd say the
aliasapproach is fine and actually more desirable than genrules. A genrule introduces an additional target that could potentially cause issues when run in remote execution. With an alias you save one level of indirection.
We don't need to have it built in here, but it is nice to be able to rebuild -- basically, every time you add something for bazel to build (i.e. a new rust file, new dependency, etc.) you need to rebuild the rust-project.json
MODULE.bazel line 80 at r1 (raw file):
use_repo(crate, "crates") rust_analyzer = use_extension("@rules_rust//tools/rust_analyzer:extension.bzl", "rust_analyzer_dependencies")
Done.
caass
force-pushed
the
caass/generate-rust-project-json
branch
from
e568ad9 to
4e51cbe
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 3 discussions need to be resolved (waiting on @aaronmondal)
BUILD.bazel line 77 at r1 (raw file):
Previously, caass (Cass Fridkin) wrote…
We don't need to have it built in here, but it is nice to be able to rebuild -- basically, every time you add something for bazel to build (i.e. a new rust file, new dependency, etc.) you need to rebuild the
rust-project.json
lets avoid alias for now, in a prior life alias looks innocent and friendly but has caused a world of complexity due to abuse. We don't need pointer redirection atm for this
CONTRIBUTING.md line 305 at r2 (raw file):
### Setting up rust-analyzer [rust-analyzer](https://rust-analyzer.github.io/) works reasonably well out of the box due to picking up the manifest for the `nativelink` crate, but it isn't integrated with Bazel by default. In order to generate a project configuration for rust-analyzer,
Will we have problems with vscode rust analyzer competing with bazel rust analyzer in vscode?
caass
force-pushed
the
caass/generate-rust-project-json
branch
from
4e51cbe to
ce675f5
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Bazel Dev / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, Vercel, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), pre-commit-checks, ubuntu-20.04 / stable, vale, and 2 discussions need to be resolved (waiting on @aaronmondal)
CONTRIBUTING.md line 305 at r2 (raw file):
Previously, adam-singer (Adam Singer) wrote…
Will we have problems with vscode rust analyzer competing with bazel rust analyzer in vscode?
No, because you'll configure VS Code rust analyzer to talk to bazel by setting .vscode/settings.json#rust-analyzer.linkedProjects to [rust-project.json
BUILD.bazel line 77 at r1 (raw file):
Previously, adam-singer (Adam Singer) wrote…
lets avoid alias for now, in a prior life alias looks innocent and friendly but has caused a world of complexity due to abuse. We don't need pointer redirection atm for this
Totally
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 1 discussions need to be resolved (waiting on @aaronmondal)
caass
dismissed
aaronmondal’s stale review
Made the requested change, and now it's stale
There was a problem hiding this comment.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, and 1 discussions need to be resolved
rust-analyzer can parse Cargo.toml files or rust-project.json files to generate context about the projects it's run on. rust-project.json files are a format intended to be generated by build tools other than Cargo, such as Bazel. With this setup we can tell rust-analyzer to build the project with Bazel instead of Cargo.
caass
force-pushed
the
caass/generate-rust-project-json
branch
from
ce675f5 to
e31b5ac
Compare
There was a problem hiding this comment.
Dismissed @aaronmondal from a discussion.
Reviewable status:complete! 1 of 1 LGTMs obtained
There was a problem hiding this comment.
Reviewed 1 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status:
Description
rust-analyzer can parse Cargo.toml files or rust-project.json files to
generate context about the projects it's run on. rust-project.json files
are a format intended to be generated by build tools other than Cargo,
such as Bazel. With this setup we can tell rust-analyzer to build the
project with Bazel instead of Cargo.
Type of change
Please delete options that aren't relevant.
not work as expected)
How Has This Been Tested?
N/A
Checklist
bazel test //...passes locallygit amendsee some docsThis change is