Skip to content

protobuf: add Python 3.7 compatibility#29660

Merged
ilovezfs merged 2 commits intoHomebrew:masterfrom
benmwebb:protobuf_fix
Jul 4, 2018
Merged

protobuf: add Python 3.7 compatibility#29660
ilovezfs merged 2 commits intoHomebrew:masterfrom
benmwebb:protobuf_fix

Conversation

@benmwebb
Copy link
Contributor

@benmwebb benmwebb commented Jul 2, 2018

If built using --with-python, compilation fails with
Python 3.7 because the Python folks changed their C
API such that PyUnicode_AsUTF8AndSize() now returns
a const char* rather than a char*. Add a patch to
work around.

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Copy link
Contributor

@ilovezfs ilovezfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @benmwebb. This needs to be submitted upstream as a PR and then we can use a patch do block in the formula to apply the upstream PR as a patch.

@ilovezfs ilovezfs added upstream issue An upstream issue report is needed python Python use is a significant feature of the PR or issue labels Jul 3, 2018
@benmwebb
Copy link
Contributor Author

benmwebb commented Jul 3, 2018

OK, I think I can do that. I was just following the example of OpenCV, which patches directly in the formula: 377ac4c. Figured that was the approved way to handle such issues.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 3, 2018

@benmwebb see #29707 :)

@benmwebb
Copy link
Contributor Author

benmwebb commented Jul 3, 2018

Upstream PR: protocolbuffers/protobuf#4862. I can update this PR once the protobuf folks merge that (or otherwise fix Python 3.7), if that works best for you guys.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 3, 2018

Just use "https://github.com/google/protobuf/pull/4862.patch?full_index=1"

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 3, 2018

i.e. no need to wait

If built using --with-python, compilation fails with
Python 3.7 because the Python folks changed their C
API such that PyUnicode_AsUTF8AndSize() now returns
a const char* rather than a char*. Add a patch to
work around.
@benmwebb
Copy link
Contributor Author

benmwebb commented Jul 3, 2018

OK, PR updated to use the upstream patch.

@ilovezfs ilovezfs merged commit d4f28ad into Homebrew:master Jul 4, 2018
@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 4, 2018

Thanks @benmwebb! Shipped.

@lock lock bot added the outdated PR was locked due to age label Aug 3, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2018
@benmwebb benmwebb deleted the protobuf_fix branch November 26, 2018 19:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age python Python use is a significant feature of the PR or issue upstream issue An upstream issue report is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants