Automate Protobuf Version Templating #24911
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gnossen
added
kind/bug
lang/Python
area/build
release notes: yes
jtattermusch
approved these changes
Dec 7, 2020
|
Btw the |
lidizheng
reviewed
Dec 7, 2020
lidizheng
approved these changes
Dec 7, 2020
gnossen
changed the title
apolcyn
approved these changes
Dec 7, 2020
|
Unrelated failure: #24937 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #24897Since both theprotobufandgrpcio-toolspackages includelibprotobufas a shared library, an application loading both of them has symbol conflicts that need to be resolved by the operating system's runtime linker. In most cases, the runtime linker uses the first loaded symbol, ignoring the second. In corner cases, such as in MacOS in #24897, some startup functions were run by the version fromgrpcio-toolswhile the version packaged withprotobufwas called at runtime. This resulted in corruption from the perspective of the version oflibprotobufpackaged by theprotobufwheel because internal data structures had changed between the two versions.By requiring an exact version, this is no longer an issue. This also brings Python into line with other wrapped languages, including Ruby, C#, and Objective-C.Since updating the Protobuf version has been a tedious manual process in the past, I went ahead and pulled all languages into the templating system so in the future, only one instance of the protobuf version string will have to be changed -- inbuild_handwritten.yaml.CC @jtattermusch since some of these changes will have to be reconciled with #24855. Once this is merged, I'll update the "Updating third_party/protobuf submodule in grpc" document accordingly.
CC @yulin-liang since this also touches Obj C files.
Update: @lidizheng made the good point that the protobuf Python package has much narrower platform support than we do, so pinning the version would be too restrictive. I've removed that portion of the PR so that this is now just an automation change for updaing the protobuf version. I'll address #24897 in a series of follow-up PRs.