Skip to content
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

protobuf: add Python 3.7 compatibility #29660

Merged
merged 2 commits into from
Jul 4, 2018
Merged

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