-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
refactor: add MODULE.bazel file for bzlmod #16013
Conversation
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.
These files copied from the last release: https://github.com/bazelbuild/bazel-central-registry/tree/main/modules/protobuf/23.1 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.
I also started this #15976 earlier ;) |
Yes and also to test the waters to see what the maintainers plan to do |
Hey @haberman could we get some feedback from maintainers? |
Hey Alex, Sandy's OOO until next week, but I can try digging into this in the meantime. QQ: does any of this take effect immediately or does it only go public after our next release? |
Oh, you're going to have a lot of questions since Bazel is so different from Blaze, and you have no repository rules or module extensions or MODULE.bazel files... |
bazel_dep(name = "rules_cc", version = "0.0.1") | ||
bazel_dep(name = "rules_proto", version = "4.0.0") | ||
bazel_dep(name = "rules_java", version = "4.0.0") | ||
bazel_dep(name = "rules_pkg", version = "0.7.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these dependencies seem older than what we're using now. Why are we downgrading?
module( | ||
name = "protobuf", | ||
compatibility_level = 1, | ||
version = "23.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're currently working on 26.0 (you could say main is on 27.0), it seems like all of these dependencies are stale
IIUC, this is a direct port from the patches of 23.1 from BCR. While adding these changes incrementally makes sense, I don't see the value in submitting broken configs that we can't even test. Ideally we would have bzlmod CI setup and make these changes correctly from the start |
You'd need to add a new test fixture in this repo to verify the next release will work in bzlmod. The BCR presubmit test only runs after the release is cut and the artifact is published. |
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
Yea I'd like to add new tests either with this PR or right after, but my point is more that this config is incorrect and won't work IIUC. It's based on a very old version, so I don't see the value in submitting this as-is |
Okay, I'll close this then. I'll reach out to you on chat so we can figure out what shape you want these new tests to be. |
Replaced by #16319 |
…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
Step towards #14564. The strategy is:
In this PR, we remove the first patch from the stack of the most recent release: https://github.com/bazelbuild/bazel-central-registry/tree/main/modules/protobuf/23.1/patches
If the maintainers prefer, I could apply a number of these patches in one PR.