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

revert: upgrade to protobuf 27.0 and remove py_proto_library (#1933) #1948

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jun 11, 2024

This reverts commit d0e25cf.

We have the following concerns with the associated change:

  • sphinxdocs is excluded entirely. Because protobuf fails to compile?
    But why? Protos are needed as part of our docgen, but it is unclear
    how the docgen is still working. Maybe we can get some clarification
    here?
  • The py_proto_library tests in the bzlmod example are excluded from
    CI. Because protos also fail to compile? But why? It's an example and
    should Just Work.
  • Adding the copts needs to be done by all downstream users now. But
    fixing that in protobuf blocks removing legacy struct providers? I
    don't understand the connection.

Also @alexeagle noted extra
regression
being caused by the associated PR.

Reverting in order to unblock a new release of rules_python and then
we can work together with @comius on reverting the revert.

Reverts #1933

…ild#1933)

This reverts commit d0e25cf.

We have the following concerns with the associated change:

- `sphinxdocs` is excluded entirely. Because protobuf fails to compile?
  But why? Protos are needed as part of our docgen, but it is unclear
  how the docgen is still working. Maybe we can get some clarification
  here?
- The `py_proto_library` tests in the `bzlmod` example are excluded from
  CI. Because protos also fail to compile? But why? It's an example and
  should Just Work.
- Adding the `copts` needs to be done by all downstream users now. But
  fixing that in protobuf blocks removing legacy struct providers? I
  don't understand the connection.

Also @alexeagle noted extra
[regression](bazelbuild#1933 (comment))
being caused by the associated PR.

Reverting in order to unblock a new release of `rules_python` and then
we can work together with @comius on reverting the revert.
@aignas aignas requested a review from rickeylev as a code owner June 11, 2024 05:25
@rickeylev rickeylev added this pull request to the merge queue Jun 11, 2024
Merged via the queue into bazelbuild:main with commit 8b0eaed Jun 11, 2024
4 checks passed
aignas added a commit that referenced this pull request Jun 12, 2024
Reintroduce the changes in #1933, which got reverted in
commit 8b0eaed (#1948).
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.

2 participants