Conversation
|
Thanks for the PR. We previously received #26173 but this PR looks more comprehensive. |
|
Looks like there are some CI errors. |
d1438b6 to
d260da7
Compare
|
@haberman fixed, can you rerun? |
|
Looks like that fixed some of the tests, but not all. |
d260da7 to
e8192d8
Compare
|
fixed that case which was caused by cmake attempting to fetch bcr tags which don't exist. Seems like the cmake build passes for me locally now so hopefully it passes this time! |
| matrix: | ||
| platform: ["debian10", "macos", "macos_arm64", "ubuntu2004", "windows"] | ||
| bazel: [8.x] | ||
| bazel: [8.x, 9.x] |
There was a problem hiding this comment.
This should match /.bazelci/presubmit.yml to runs the same tests in CI instead of just BCR presubmit so we can catch issues ahead of release.
This doesn't flip the default bazel version, but instead makes this repo compatible with the current version and bazel 9.x. The primary changes for 9.x support are adding new load statements for things that were previously built in. This cascaded into a few dep updates to pull in their missing load statement fixes.
e8192d8 to
8a35249
Compare
This doesn't flip the default bazel version, but instead makes this repo compatible with the current version and bazel 9.x. The primary changes for 9.x support are adding new load statements for things that were previously built in. This cascaded into a few dep updates to pull in their missing load statement fixes. Closes #26201 COPYBARA_INTEGRATE_REVIEW=#26201 from keith:ks/add-support-for-bazel-9.x e8192d8 FUTURE_COPYBARA_INTEGRATE_REVIEW=#26201 from keith:ks/add-support-for-bazel-9.x e8192d8 PiperOrigin-RevId: 878614534
|
How did you test the patch files for the compatibility tests? Were those changes needed to get the CI to pass? |
This doesn't flip the default bazel version, but instead makes this repo compatible with the current version and bazel 9.x. The primary changes for 9.x support are adding new load statements for things that were previously built in. This cascaded into a few dep updates to pull in their missing load statement fixes. Closes #26201 COPYBARA_INTEGRATE_REVIEW=#26201 from keith:ks/add-support-for-bazel-9.x e8192d8 FUTURE_COPYBARA_INTEGRATE_REVIEW=#26201 from keith:ks/add-support-for-bazel-9.x e8192d8 PiperOrigin-RevId: 878614534
zhangskz
left a comment
There was a problem hiding this comment.
How did you identify / generate these patches? Its correct that protobuf_v25 / protobuf_v29 don't support Bazel 9 without patches, but did you actually hit build breakages in presubmits without them?
These deps are just for compatibility / breaking change tests that aren't user-facing so making them work in Bazel 9 now would be nice as well, but I expected we should only need to patch parts of v25/v29 to make these work.
|
The added patches there were the minimal set required to get |
This doesn't flip the default bazel version, but instead makes this repo compatible with the current version and bazel 9.x. The primary changes for 9.x support are adding new load statements for things that were previously built in. This cascaded into a few dep updates to pull in their missing load statement fixes. Closes #26201 COPYBARA_INTEGRATE_REVIEW=#26201 from keith:ks/add-support-for-bazel-9.x e8192d8 PiperOrigin-RevId: 878654773
This doesn't flip the default bazel version, but instead makes this repo
compatible with the current version and bazel 9.x. The primary changes
for 9.x support are adding new load statements for things that were
previously built in. This cascaded into a few dep updates to pull in
their missing load statement fixes.