Cover Bazel build of 2 initial rs_bindings_from_cc targets in GitHub CI. - #1704
Cover Bazel build of 2 initial rs_bindings_from_cc targets in GitHub CI.#1704anforowicz wants to merge 3 commits into
Conversation
|
We're quite close to having GCS setup, see #1682 which moves our CMake to use GCS. I need to make a bucket for our bazel stuff and then I plan to move cpp_api_from_rust's bazel stuff to use GCS as well. This CL looks reasonable, but given I think we could have GCS online later today (hopefully), do we still need the conditional GCS usage or can we simplify and assume the GCS support is present?
I'm always in favor of more public work rather than less, so this seems great. |
| HAS_BAZEL_CACHE_SECRET: ${{ secrets.BAZEL_CACHE_GCP_SERVICE_ACCOUNT_KEY != '' }} | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 |
There was a problem hiding this comment.
Google turned on https://github.com/zizmorcore/zizmor across all of it's repos, so I believe undoing these commit pins will violate those checks and block submitting this PR. That's why we had to swap to the pins in the first place
There was a problem hiding this comment.
Ack. This should be fixed now - let's see what GitHub workflows say...
There was a problem hiding this comment.
Actually... it still fails... let me take another look...
Great - thanks!
I think we still want to keep it as conditional, so that the workflows continue to work in GitHub forks. |
Ah that's a good point I had not considered. SGTM |
e6427f2 to
7b97400
Compare
This PR makes a minimal, comment-only change to `MODULE.bazel` to verify that PRs from forks work ok. Initially this no-op change lead to GitHub CQ failure that is fixed by this PR: - Problem: `google-github-actions/auth` failed on a fork PR because the `GCP_SA_KEY` secret was empty/missing. - Fix: added conditional guards to the authentication and `sccache` installation steps in `rust.yml`. If the secret is missing, we skip these steps and build CMake without `sccache` compiler launchers. When testing `cargo test` locally (where I have `rustup`-installed support for targeting the Fuchsia platform) additional failures were discovered and fixed: - Problem: The `cargo-cpp_api_from_rust` tool was searching `sysroot/lib/rustlib/` and picking the first directory containing `.rmeta` files to find the standard library. Because `x86_64-unknown-fuchsia` comes alphabetically before `x86_64-unknown-linux-gnu`, it picked the Fuchsia standard library instead of the host Linux one (which is present on your machine and potentially on the GHA runner). - Fix: querying via `rustc --print target-libdir` Note that Copybara-related failures should be fixed by the Google-internal cl/961319084.
The `LangItem` enum was moved from `rustc_hir::lang_items::LangItem` (or the root `rustc_hir::LangItem` in some contexts) to `rustc_hir::attrs::LangItem`. This update fixes the imports to match the new location. Fixes google#1722
7b97400 to
a950617
Compare
2 participants
PTAL?
The main point of this PR is to provide minimal initial step for re-enabling of Bazel builds of
rs_bindings_from_cc. This PR only covers building of 2 targets that depend only on Abseil. Other targets (i.e. ones depending on Rust and/or crates.io, ones depending on Clang/LLVM, etc.) don't work yet.This PR:
bazelisk build ... rs_bindings_from_cc:bazel_types(and...:cmdline_flags) torust.yml. This is done in a way that configures Bazel to use GCS for caching of build artifactsMODULE.bazelbazel_deps inMODULE.bazeland afterwards tidy upMODULE.bazel.lockas neededI understand that we haven't yet set up GCS bucket. This seems ok for this PR which adds a relatively small build - after
bazelisk cleanI have runtime bazelisk build rs_bindings_from_cc:bazel_types rs_bindings_from_cc:cmdline_flagsand it build 207 targets in less than 5 seconds on my machine. Once the GCS auth secret is stored in GitHub, then future GitHub jobs should start using GCS for caching and should say so (seeechostatements added by this PR torust.yml).WDYT?
PS. I am authoring this as a GitHub PR, because this makes it easy to verify that local
bazeliskcommands work fine on Copybara-ed / public version of Crubit. Hopefully this workflow is ok.