bazel: numerous fixes, get nearly all tests runnable #29188
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ParkMyCar
force-pushed
the
bazel/testing
branch
3 times, most recently
from
a6522a1 to
7c9de99
Compare
* changes cargo-gazelle to skip generating rust_library targets for crates that don't have one
* update cargo-gazelle to specify the crate_name for integration tests. This fixes some crates like `insta` that rely on the test name for path generation.
Fix tests that use `insta` for snapshotting * Update a few crates Cargo.toml so their generated BUILD.bazel files contain the right environment variables
* upgrade the version of rules_rust we use to get on the latest * run bin/bazel gen
* specify --experimental_inprocess_symlink_creation for macOS which is required for Rust Doc tests * specify --experimental_inmemory_sandbox_stashes for macOS to improve sandbox perf add some new settings to .bazelrc for sandbox perf
* with cargo-gazelle, specify the same set of `crate_features` for lib tests as the library target
* refine how we use cxx and thus link the protobuf-native crate. Fixes an "ambiguous library" error we were getting when linking.
* Define COCKROACH_URL as an environment variable that gets passed to the test runner
ParkMyCar
force-pushed
the
bazel/testing
branch
from
7c9de99 to
e93f530
Compare
ParkMyCar
changed the title
ParkMyCar
requested review from
benesch,
aljoscha,
danhhz,
jkosh44,
a team and
teskje
as code owners
antiguru
approved these changes
Aug 28, 2024
Comment on lines
+21
to
+27
| const PROTO_DIRECTORY: &str = { | ||
| if mz_build_tools::is_bazel_build() { | ||
| "src/catalog/protos" | ||
| } else { | ||
| "protos" | ||
| } | ||
| }; |
There was a problem hiding this comment.
Not great, but possible to live with I think!
There was a problem hiding this comment.
Yeah FWIW this is my least favorite part, I'll follow up with this!
bobbyiliev
approved these changes
Aug 28, 2024
benesch
approved these changes
Aug 28, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes a number of tweaks to our Bazel setup to get nearly all Rust tests runnable with Bazel. There is a second PR stacked on top of this one that gets all tests working.
All of the changes are logically grouped by commit, they're all relatively small and shouldn't require thorough review, they are:
rust_librarytargets for crates that aren't a library, e.g.catalog-debugorclusterd.crate_nameattribute. This specifies the name that should be used for the generated binary so it matches what Cargo does.instacrate for snapshotting by specifying the data that they need and an env var soinstacan find the snapshots.rules_rustto the latest..bazelrcto specify some Mac only options so doc tests work, also ideally it should improve performance of the Bazel Sandbox.cxxcrate withprotobuf-nativeto fix an "ambiguous library" linker error. We were previously depending on two different versions ofcxx.COCKROACH_URLas an environment variable that gets passed through to tests.Motivation
Progress towards https://github.com/MaterializeInc/database-issues/issues/7923
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.