protobuf: apply patch for python 3.7 only when building with it #51808
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
samueldr
force-pushed
the
fix/protobuf-python37-patch
branch
2 times, most recently
from
982cdff
to
475d64f
Compare
|
See #51809 for a package this fixes. Since the patch won't apply with python 3.6, changing to python 3.6 is not enough to fix caffe2. |
| @@ -16,14 +16,14 @@ buildPythonPackage rec { | |||
| propagatedBuildInputs = [ protobuf google_apputils ]; | |||
| buildInputs = [ google_apputils pyext ]; | |||
|
|
|||
| patches = [ | |||
| patches = optional (versionAtLeast python.version "3.7") | |||
There was a problem hiding this comment.
Wouldn't pythonAtLeast make more sense here?
There was a problem hiding this comment.
Thanks! the things you can learn about the python ecosystem ;).
samueldr
force-pushed
the
fix/protobuf-python37-patch
branch
2 times, most recently
from
b24992a
to
53decb6
Compare
Change introduced in NixOS#50672. The patch does not apply on older protobuf like protobuf3_1 ``` $ nix-build -E 'with import ./. { }; python3Packages.protobuf.override { protobuf = protobuf3_1; }' unpacking sources unpacking source archive /nix/store/1zdyl0cxaa8ha2v1zp75zzdjd6j99d0m-source source root is source setting SOURCE_DATE_EPOCH to timestamp 315619200 of file source/util/python/BUILD patching sources applying patch /nix/store/yagx7hvylnnjq7lxbcia0y5lq1r736w3-0a59054c30e4f0ba10f10acfc1d7f3814c63e1a7.patch patching file google/protobuf/pyext/descriptor.cc Hunk #1 succeeded at 55 (offset -1 lines). patching file google/protobuf/pyext/descriptor_containers.cc patching file google/protobuf/pyext/descriptor_pool.cc Hunk #1 succeeded at 47 (offset -1 lines). patching file google/protobuf/pyext/extension_dict.cc Hunk #1 FAILED at 53. 1 out of 1 hunk FAILED -- saving rejects to file google/protobuf/pyext/extension_dict.cc.rej patching file google/protobuf/pyext/message.cc Hunk #1 succeeded at 82 (offset 3 lines). Hunk #2 succeeded at 1425 (offset -104 lines). ``` Since the patch isn't necessary on python versions older than 3.7, let's only apply it for version 3.7. This means that most things using older protobuf implementation will now be able to build when using an older pythonPackage set (as is most probably the case anyway). This still leaves protobuf 3.1 using packages hanging, but the errors will be localized to those that would be breaking anyway with the upgrade to 3.7 as default python.
samueldr
force-pushed
the
fix/protobuf-python37-patch
branch
from
53decb6
to
2ae8dd3
Compare
|
Updated with |
|
I'll merge #51809 once |
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.
Change introduced in #50672.
The patch does not apply on older protobuf like protobuf3_1
Since the patch isn't necessary on python versions older than 3.7, let's
only apply it for version 3.7. This means that most things using older
protobuf implementation will now be able to build when using an older
pythonPackage set (as is most probably the case anyway).
This still leaves protobuf 3.1 using packages hanging, but the errors
will be localized to those that would be breaking anyway with the
upgrade to 3.7 as default python.
This can be tested using:
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)cc @lopsided98 @worldofpeace