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

go: Add bb-remote-execution dependency #898

Merged
merged 2 commits into from
Apr 24, 2023

Conversation

minor-fixes
Copy link
Contributor

@minor-fixes minor-fixes commented Apr 23, 2023

github.com/buildbarn/bb-remote-execution defines an RPC service that we'd like to implement. We can:

  • depend on this repo as a dependency to use the protos within
  • copy the protos and their transitive imports into enkit and author our own BUILD files

This change attempts to do the former. This came with some complications:

  • Pulling in this dep upgraded GCP modules under cloud.google.com/go, which causes issues due to the genproto migration that is currently underway. To resolve this, this PR:
    • runs the fixer tool on our code to update import paths, plus gazelle to fix BUILD files
    • pulls in a fork of GoogleCloudPlatform/cloud-build-notifiers, with updated deps and import paths (once go: Update all dependencies to latest GoogleCloudPlatform/cloud-build-notifiers#163 is merged, this fork can be dropped)
    • pulls in an updated version of go_googleapis. Usually rules_go does this for us, but we needed new datastore protos to be compatible with the updated datastore module, and so we must specify the version ourselves and also generate the patches that rules_go usually does.
  • github.com/bazelbuild/remote-apis has BUILD files that reference @googleapis instead of @go_googleapis; this is usually patched in the buildbarn ecosystem, but we can't use that patch verbatim due to our updated version of @go_googleapis - so this change adds a slightly modified version of this patch.

Tested: bazel test //... -k works

This change updates the golang ecosystem, including:

* rules_go to latest release
* gazelle to latest release
* Go SDK to latest release (1.20.3)

This was hopefully going to fix an issue with gazelle and strict
dependencies in internal, but doesn't - however, it is still less risky
to be on the latest versions if there are no negative effects.

One of the unit tests testing byte slices is flaky for unknown reasons
(test can't decide if it wants strings in the golden cases or not) so
the comparer func is modified to normalize both the "got" and "want"
slices to sidestep this issue.

Tested:
* `bazel test //...` passes
* `bazel test //infra/cj/... --override_repository=enkit=<path to
enkit>` works in internal
@minor-fixes minor-fixes force-pushed the add_buildbarn_dep branch 2 times, most recently from dee1cc5 to 6e69319 Compare April 24, 2023 13:40
@minor-fixes minor-fixes changed the title Add buildbarn dep go: Add bb-remote-execution dependency Apr 24, 2023
@minor-fixes minor-fixes marked this pull request as ready for review April 24, 2023 14:27
@minor-fixes minor-fixes merged commit e26980b into enfabrica:master Apr 24, 2023
@ccontavalli ccontavalli mentioned this pull request May 5, 2023
This was referenced Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants