-
Notifications
You must be signed in to change notification settings - Fork 21.7k
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
[submodule] update protobuf to tag v3.19.6 #95128
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/95128
Note: Links to docs will display an error until the docs builds have been completed. ❌ 4 FailuresAs of commit a8de144: NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
I was going to get around to this at some point. You will also want to update the pip install pinned versions of protobuf to the 21.12 as well. (do a grep through the yaml and requirement files). |
There is also this issue requiring protobuf < 3.2.0 for tensorboard to work: #90374. I propose there that torch.utils.tensorboard should not fail on import because of problematic protobuf dependencies |
No pip dependency found after a grep. Bazel files were also checked but no specific versions hard-wired. |
May be we need to upgrade tensorboard dependency as well. |
As far as I know, tensorboard still has this problem unfixed. So you could bump the min version of tensorboard, but it still wouldn't fix protobuf <3.2.0 requirement of tensorboard: tensorflow/tensorboard#6147 v3.19.6 seems to be the most recent supported version |
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.
For those kinds of changes, I want some rigorous and reproducible test plan, like a script that dumps public symbols on all 3 platforms(with priority on Linux and Windows) and showing that no new ones were added.
Another good test to add (and may be to CI): Install torch + different version of protobuf from pipy and make sure they do not conflict with each other, as all the ugly patching that you are removing was done in order to make protobuf not leak any symbols externally.
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
Fixes #ISSUE_NUMBER