Patch protobuf and grpc dependencies to be Python 3.10 compatible. #5793
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
nfelt
reviewed
Jul 6, 2022
| # ``` | ||
| # | ||
| # Note that we choose tags/v1.27.0-pre1 as the base since the version in | ||
| # the archive is 1.27.0-dev. It's not necessarily an exact match but is |
There was a problem hiding this comment.
For future reference, the prefix of the archive that we strip (grpc-b54a5b338637f92bfcf4b0bc05e0f57a5fd8fadd) includes the commit hash, so we can always check out specifically that commit if you want an exact match.
There was a problem hiding this comment.
Thanks. I'm adjusting the instructions in #5795.
bmd3k
added a commit
that referenced
this pull request
Jul 11, 2022
As noted in a comment for #5793, the instructions for generating grpc.patch would be more accurate if they referred to the commit that corresponds to the grpc archive instead of referring to the nearest tag. I've updated the instructions to refer to the commit. I tested by following the updated instructions and copying grpc.patch to my tensorboard repo. git did not find a diff in the generated files.
nfelt
added a commit
that referenced
this pull request
Jan 19, 2023
Fixes #5708 and #5703. This updates our protobuf dependency to 3.19.6 in an attempt to address #5708, and provide a cleaner solution to #5703. The choice of 3.19.6 is meant to satisfy two competing constraints: - Current Python protobuf runtimes (the 4.x series) only support generated code from protoc versions 3.19.0+, as discussed in https://protobuf.dev/news/2022-05-06/. As a result, prior to this change, TensorBoard's pip package had to force its pip package dependency to `protobuf < 4` to avoid the errors seen in #5703. This PR lifts that restriction. - Current TensorFlow is still stuck on protobuf 3.x, the same as we have been, and as a result pins its pip package dependency using `protobuf < 3.20` (this could presumably be relaxed to `< 4` but that would require new TF releases). As a result, we must support at least one protobuf runtime version that also works with TF's constraints. Our previous attempt at this upgrade (to ~3.18 or so) caused test failures for Keras (which depends on TB, via TF, for the summary API code), apparently due to a protobuf runtime that was too old for our generated code. At the time, this was puzzling because they were pip-installing a protobuf runtime version that should have been recent enough - but I suspect now that this was a red herring, and bazel test was actually getting the protobuf runtime from the protobuf build dependency, not from the installed Python packages. If we see this failure mode again, we'll have to get Keras to update the protobuf Python runtime available in bazel tests. Lastly, this upgrade lets us clean up some additional issues we had to work around: - We can also upgrade gRPC now, to 1.48.2. I selected this version since it appears to be the most recent version prior to gRPC adopting protobuf 4.x (see grpc/grpc@41ec08c) - We can revert the backported fixes to protobuf and grpc from #5793, since the upgraded dependencies don't require patching - We can back out rules_apple reintroduction from #5561 that we only needed for gRPC
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
…ensorflow#5793) This PR allows TensorBoard to build and run with Python 3.10. There are incompatibilities in the protobuf and grpc dependencies, which are quite old but we are unable to upgrade (see: tensorflow#5561 , Googlers: http://b/219030239). We patch the necessary fixes from those projects into our older versions using the `patches` property of the `http_archive` rule. For grpc we patch grpc/grpc@dbe73c9 to avoid the following compile-time error: ``` Error in fail: Python Configuration Error: Problem getting python include path for /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python. <string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives <string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead Is the Python binary path set up right? (See ./configure or /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python.) Is distutils installed? Are Python headers installed? Try installing python-dev or python3-dev on Debian-based systems. Try python-devel or python3-devel on Redhat-based systems. ``` For protobuf we patch protocolbuffers/protobuf@9d61ead to avoid the following runtime error at startup: ``` File "/usr/local/google/home/bdubois/.cache/bazel/_bazel_bdubois/079646a57be11faea0b2bfefccb2a81a/execroot/org_tensorflow_tensorboard/bazel-out/k8-fastbuild/bin/tensorboard/dev.runfiles/com_google_protobuf/python/google/protobuf/descriptor.py", line 47, in <module> from google.protobuf.pyext import _message AttributeError: module 'collections' has no attribute 'MutableSequence' ``` The changes patched into message.cc are not the latest. I considered also patching protocolbuffers/protobuf@624d29d to remove the references to PY_MAJOR_VERSION but that change is wide in scope.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
As noted in a comment for tensorflow#5793, the instructions for generating grpc.patch would be more accurate if they referred to the commit that corresponds to the grpc archive instead of referring to the nearest tag. I've updated the instructions to refer to the commit. I tested by following the updated instructions and copying grpc.patch to my tensorboard repo. git did not find a diff in the generated files.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
Fixes tensorflow#5708 and tensorflow#5703. This updates our protobuf dependency to 3.19.6 in an attempt to address tensorflow#5708, and provide a cleaner solution to tensorflow#5703. The choice of 3.19.6 is meant to satisfy two competing constraints: - Current Python protobuf runtimes (the 4.x series) only support generated code from protoc versions 3.19.0+, as discussed in https://protobuf.dev/news/2022-05-06/. As a result, prior to this change, TensorBoard's pip package had to force its pip package dependency to `protobuf < 4` to avoid the errors seen in tensorflow#5703. This PR lifts that restriction. - Current TensorFlow is still stuck on protobuf 3.x, the same as we have been, and as a result pins its pip package dependency using `protobuf < 3.20` (this could presumably be relaxed to `< 4` but that would require new TF releases). As a result, we must support at least one protobuf runtime version that also works with TF's constraints. Our previous attempt at this upgrade (to ~3.18 or so) caused test failures for Keras (which depends on TB, via TF, for the summary API code), apparently due to a protobuf runtime that was too old for our generated code. At the time, this was puzzling because they were pip-installing a protobuf runtime version that should have been recent enough - but I suspect now that this was a red herring, and bazel test was actually getting the protobuf runtime from the protobuf build dependency, not from the installed Python packages. If we see this failure mode again, we'll have to get Keras to update the protobuf Python runtime available in bazel tests. Lastly, this upgrade lets us clean up some additional issues we had to work around: - We can also upgrade gRPC now, to 1.48.2. I selected this version since it appears to be the most recent version prior to gRPC adopting protobuf 4.x (see grpc/grpc@41ec08c) - We can revert the backported fixes to protobuf and grpc from tensorflow#5793, since the upgraded dependencies don't require patching - We can back out rules_apple reintroduction from tensorflow#5561 that we only needed for gRPC
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
…ensorflow#5793) This PR allows TensorBoard to build and run with Python 3.10. There are incompatibilities in the protobuf and grpc dependencies, which are quite old but we are unable to upgrade (see: tensorflow#5561 , Googlers: http://b/219030239). We patch the necessary fixes from those projects into our older versions using the `patches` property of the `http_archive` rule. For grpc we patch grpc/grpc@dbe73c9 to avoid the following compile-time error: ``` Error in fail: Python Configuration Error: Problem getting python include path for /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python. <string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives <string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead Is the Python binary path set up right? (See ./configure or /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python.) Is distutils installed? Are Python headers installed? Try installing python-dev or python3-dev on Debian-based systems. Try python-devel or python3-devel on Redhat-based systems. ``` For protobuf we patch protocolbuffers/protobuf@9d61ead to avoid the following runtime error at startup: ``` File "/usr/local/google/home/bdubois/.cache/bazel/_bazel_bdubois/079646a57be11faea0b2bfefccb2a81a/execroot/org_tensorflow_tensorboard/bazel-out/k8-fastbuild/bin/tensorboard/dev.runfiles/com_google_protobuf/python/google/protobuf/descriptor.py", line 47, in <module> from google.protobuf.pyext import _message AttributeError: module 'collections' has no attribute 'MutableSequence' ``` The changes patched into message.cc are not the latest. I considered also patching protocolbuffers/protobuf@624d29d to remove the references to PY_MAJOR_VERSION but that change is wide in scope.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
As noted in a comment for tensorflow#5793, the instructions for generating grpc.patch would be more accurate if they referred to the commit that corresponds to the grpc archive instead of referring to the nearest tag. I've updated the instructions to refer to the commit. I tested by following the updated instructions and copying grpc.patch to my tensorboard repo. git did not find a diff in the generated files.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
Fixes tensorflow#5708 and tensorflow#5703. This updates our protobuf dependency to 3.19.6 in an attempt to address tensorflow#5708, and provide a cleaner solution to tensorflow#5703. The choice of 3.19.6 is meant to satisfy two competing constraints: - Current Python protobuf runtimes (the 4.x series) only support generated code from protoc versions 3.19.0+, as discussed in https://protobuf.dev/news/2022-05-06/. As a result, prior to this change, TensorBoard's pip package had to force its pip package dependency to `protobuf < 4` to avoid the errors seen in tensorflow#5703. This PR lifts that restriction. - Current TensorFlow is still stuck on protobuf 3.x, the same as we have been, and as a result pins its pip package dependency using `protobuf < 3.20` (this could presumably be relaxed to `< 4` but that would require new TF releases). As a result, we must support at least one protobuf runtime version that also works with TF's constraints. Our previous attempt at this upgrade (to ~3.18 or so) caused test failures for Keras (which depends on TB, via TF, for the summary API code), apparently due to a protobuf runtime that was too old for our generated code. At the time, this was puzzling because they were pip-installing a protobuf runtime version that should have been recent enough - but I suspect now that this was a red herring, and bazel test was actually getting the protobuf runtime from the protobuf build dependency, not from the installed Python packages. If we see this failure mode again, we'll have to get Keras to update the protobuf Python runtime available in bazel tests. Lastly, this upgrade lets us clean up some additional issues we had to work around: - We can also upgrade gRPC now, to 1.48.2. I selected this version since it appears to be the most recent version prior to gRPC adopting protobuf 4.x (see grpc/grpc@41ec08c) - We can revert the backported fixes to protobuf and grpc from tensorflow#5793, since the upgraded dependencies don't require patching - We can back out rules_apple reintroduction from tensorflow#5561 that we only needed for gRPC
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.
This PR allows TensorBoard to build and run with Python 3.10.
There are incompatibilities in the protobuf and grpc dependencies, which are quite old but we are unable to upgrade (see: #5561 , Googlers: http://b/219030239). We patch the necessary fixes from those projects into our older versions using the
patchesproperty of thehttp_archiverule.For grpc we patch grpc/grpc@dbe73c9 to avoid the following compile-time error:
For protobuf we patch protocolbuffers/protobuf@9d61ead to avoid the following runtime error at startup:
The changes patched into message.cc are not the latest. I considered also patching protocolbuffers/protobuf@624d29d to remove the references to PY_MAJOR_VERSION but that change is wide in scope.