Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel: Migrate to bzlmod for dependency management #189

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minor-fixes
Copy link

@minor-fixes minor-fixes commented Nov 13, 2024

This change aims to fix the bazel build postsubmit workflow in a somewhat heavy-handed way: by migrating to use bzlmod to manage dependencies, and by updating said dependencies (and the supported version of bazel itself) to the newest version(s) feasible.

This involves:

  • Removing the old dependency fetching infra from WORKSPACE.bazel, and adding equivalent dependency fetches to MODULE.bazel. Each version used is the newest available on BCR, with the exception of:
    • rules_proto, which encounters a Starlark error with the latest version of 7.0.2
    • protobuf, which must match versions with the toolchain loaded by toolchains_protoc; the latest version of toolchains_protoc does not have a Starlark mapping for protobuf v29.0 yet.
  • Changing rules used to generate C++ protobuf/gRPC bindings from cc_grpc_library (provided by gRPC) to cpp_grpc_library (provided by rules_proto_grpc_cpp), which seems to be the current canonical ruleset for this. These rules generate proto, grpc, and mock bindings by default, so the corresponding attributes are dropped since they're no longer necessary.
  • Dropping gnmi_deps.bzl, as it is no longer used to fetch dependencies, and can only confuse downstream users that also load this file.

Additionally, there are some quality-of-life changes:

  • .bazelversion is added to pin the version of bazel used when invoked via bazelisk
  • .gitignore is added to avoid committing bazel-generated symlinks
  • Go protobuf generation is added for all protos

Tested: bazel build //... works on my system (Linux, amd64)


There are good reasons not to take this change, including:

  • C++ proto rule changes causing downstream breakage, as some targets are removed, and the semantics of remaining targets may have changed in an incompatible way
  • Downstream bazel repositories will need to migrate to bzlmod (at least partially) in order to pick up updates following this change

...and this change author has little context on the presence/severity of these issues - guidance here would be welcome!

This change aims to fix the `bazel build` postsubmit workflow in a
somewhat heavy-handed way: by migrating to use bzlmod to manage
dependencies, and by updating said dependencies (and the supported
version of bazel itself) to the newest version(s) feasible.

This involves:

* Removing the old dependency fetching infra from `WORKSPACE.bazel`, and
  adding equivalent dependency fetches to `MODULE.bazel`. Each version
  used is the newest available on
  [BCR](https://registry.bazel.build), with the exception of:
  - `rules_proto`, which encounters a Starlark error with the latest
    version of `7.0.2`
  - `protobuf`, which must match versions with the toolchain loaded by
    `toolchains_protoc`; the latest version of `toolchains_protoc` does
    not have a Starlark mapping for protobuf v29.0 yet.
* Changing rules used to generate C++ protobuf/gRPC bindings from
  `cc_grpc_library` (provided by gRPC) to `cpp_grpc_library` (provided
  by `rules_proto_grpc_cpp`), which seems to be the current canonical
  ruleset for this. These rules generate proto, grpc, and mock bindings
  separately by default, so the corresponding attributes are dropped.
* Dropping `gnmi_deps.bzl`, as it is no longer used to fetch
  dependencies, and can only confuse downstream users that also load
  this file.

Additionally, there are some quality-of-life changes:

* `.bazelversion` is added to pin the version of bazel used when invoked
  via `bazelisk`
* `.gitignore` is added to avoid committing bazel-generated symlinks
* Go protobuf generation is added for all protos

Tested: `bazel build //...` works on my system (Linux, amd64)

---

There are good reasons not to take this change, including:

* C++ proto rule changes causing downstream breakage, as some targets
  are removed, and the semantics of remaining targets may have changed
  in an incompatible way
* Downstream bazel repositories will need to migrate to bzlmod (at least
  partially) in order to pick up updates following this change

...and this change author has little context on the presence/severity of
these issues - guidance here would be welcome!
@minor-fixes minor-fixes marked this pull request as ready for review November 13, 2024 19:46
@minor-fixes
Copy link
Author

minor-fixes commented Nov 13, 2024

startblock:
    add reviewer @marcushines ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant