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

feat: automate publishing releases to Bazel Central Registry #14565

Closed
wants to merge 1 commit into from

Conversation

alexeagle
Copy link
Contributor

@alexeagle alexeagle commented Oct 31, 2023

Note, it's also required that someone with sufficient permissions "installs" the Publish-to-BCR GitHub app to this repo.
FYI @thesayyn

Fixes #14564

@alexeagle
Copy link
Contributor Author

@zhangskz @googleberg looks like I can't get CI results for my PR since it originates from an untrusted fork. How should I proceed?
I plan to send more Bazel-related PRs so I'd like to have a solution that's not only specific to this one.

@alexeagle
Copy link
Contributor Author

alexeagle commented Oct 31, 2023

https://github.com/protocolbuffers/protobuf/blob/main/.github/workflows/test_runner.yml#L8-L10

In these cases, we
require a special "safe for tests" tag to be added to the pull request before
we start testing. This will be immediately removed, so that further commits
require their own stamp to test.

Looks like every time I push a commit, I'll have to ask a Googler to add a tag on my PR. Doesn't sound very productive, but I guess it's a start.

@googleberg googleberg added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 31, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 31, 2023
@zhangskz
Copy link
Member

Hi Alex, we should have just granted you some additional permissions that should allow you to push to a dev branch in the repo instead to bypass the "safe for test" tag requirement (or at least add the tag yourself for PRs from your fork). Let me know if that works to help speed things up a bit.

@zhangskz
Copy link
Member

Do you have instructions on what you need regarding installing the Publish-to-BCR app?

@alexeagle
Copy link
Contributor Author

@zhangskz yes, could you go to https://github.com/apps/publish-to-bcr and click Configure, then add it to this repository?

@zhangskz
Copy link
Member

@zhangskz yes, could you go to https://github.com/apps/publish-to-bcr and click Configure, then add it to this repository?

Done.

@zhangskz
Copy link
Member

zhangskz commented Jan 9, 2024

Hi Alex, have you been able to get unblocked on testing this? Let me know if this is ready for review, or if there's anything we can do to help unblock here..

@aran
Copy link

aran commented Feb 29, 2024

Would be useful to merge this!

@alexeagle
Copy link
Contributor Author

Thanks for the ping @aran - taking another look

@alexeagle
Copy link
Contributor Author

@zhangskz I'd like to understand why the BCR entries are accumulating a bunch of patches:
https://github.com/bazelbuild/bazel-central-registry/tree/main/modules/protobuf/23.1/patches

It seems like the protobuf project is forking, these patches represent some other branch where bzlmod-related changes are stacking up, while main represents WORKSPACE. Is this an intentional strategy, or an accident?

I think there are a couple sane paths forward:

  1. instead of adding patches for publishing, just make those changes in this repo. I'll send a trial PR to see if it gets accepted by maintainers before I sink a bunch of time into this.
  2. stop exposing protobuf as a Bazel project at all. protobuf already ships binary tools for each release, and rules_proto can provide all the starlark code to fetch these binaries from the GitHub releases page and make them work for Bazel users. This way the protobuf team can stop having this Bazel support drag.

alexeagle added a commit that referenced this pull request Mar 1, 2024
Step towards #14564. The strategy is:

1. Burndown the set of patches which are currently applied only when protobuf is published to BCR, by applying those patches to the repo.
2. In cases where a patch can't be accepted to the repo, it can be in the .bcr folder.
3. Finish #14565 by configuring the Publish-to-BCR github app.
Note, recent releases on the BCR have a patch set applied, and it seems these patches are developed
independently to "fix" each protobuf release, rather than make changes to protobuf repo.

The effect of this PR will be to create a *broken* publish to BCR for each protobuf release.
At least this red PR on BCR will be our indication that the patches need to be manually replayed there.

In parallel, starting with protocolbuffers#16013 I'll apply as many of those patches to the protobuf repo as possible.
That will reduce the manual effort for each release.
@alexeagle
Copy link
Contributor Author

looks like I have to close this to re-open a PR that comes from the upstream rather than my fork.

@zhangskz
Copy link
Member

zhangskz commented Mar 6, 2024

  1. I think the equivalent of these patches can and should be introduced into the protobuf repo itself -- this is likely just an artifact of lack of MODULE.bzl file / BCR support and current releases being contributed by third parties.
  2. We still want to have protobuf to have first-class bazel support, esp. for development / testing, though indeed users should generally be using our published release from Github. Are you suggesting a way to provide Bazel support for users via our Github binary releases and reducing some maintenance burden, or dropping this support entirely?

copybara-service bot pushed a commit that referenced this pull request Mar 12, 2024
Note, recent releases on the BCR have a patch set applied, and it seems these patches are developed independently to "fix" each protobuf release, rather than make changes to protobuf repo.

The effect of this PR will be to create a *broken* publish to BCR for each protobuf release. At least this red PR on BCR will be our indication that the patches need to be manually replayed there.

In parallel, starting with #16013 I'll apply as many of those patches to the protobuf repo as possible. That will reduce the manual effort for each release.

Replaces #14565 which originated from my fork so the tests wouldn't run.

Closes #16014

COPYBARA_INTEGRATE_REVIEW=#16014 from protocolbuffers:bcr e17d9c8
PiperOrigin-RevId: 615026796
deannagarcia pushed a commit to deannagarcia/protobuf that referenced this pull request Jun 20, 2024
…lbuffers#16014)

Note, recent releases on the BCR have a patch set applied, and it seems these patches are developed independently to "fix" each protobuf release, rather than make changes to protobuf repo.

The effect of this PR will be to create a *broken* publish to BCR for each protobuf release. At least this red PR on BCR will be our indication that the patches need to be manually replayed there.

In parallel, starting with protocolbuffers#16013 I'll apply as many of those patches to the protobuf repo as possible. That will reduce the manual effort for each release.

Replaces protocolbuffers#14565 which originated from my fork so the tests wouldn't run.

Closes protocolbuffers#16014

COPYBARA_INTEGRATE_REVIEW=protocolbuffers#16014 from protocolbuffers:bcr e17d9c8
PiperOrigin-RevId: 615026796
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.

[bazel] Automate publishing releases to Bazel's central registry
4 participants