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

Windows Artifact build failed for Python 2.7 after upgrade to Protobuf 3.12.0 #23011

Closed
stanley-cheung opened this issue May 20, 2020 · 3 comments

Comments

@stanley-cheung
Copy link
Contributor

stanley-cheung commented May 20, 2020

This stems from PR #22998

Problem

In that PR, we are trying to update third_party/protobuf to version v3.12.0. We ran into a Python test failure involving Python 2.7 Windows Artifact build.

Here's the error:

third_party\protobuf\src/google/protobuf/port_def.inc:457:54: error: expected primary-expression before ')' token
   PROTOBUF_EXPORT_TEMPLATE_##which##_##style(export, )
...
FAILED: build_artifact.python_windows_x64_Python27

Full Log: https://source.cloud.google.com/results/invocations/b95cf1f2-191b-4fdb-8554-413524992d91/targets/grpc%2Fcore%2Fpull_request%2Fwindows%2Fgrpc_build_artifacts/log

Investigation

Seems to be caused by this PR: protocolbuffers/protobuf#7344, and originally protocolbuffers/protobuf#6535

Same error encountered by other protobuf users in: protocolbuffers/protobuf#6659

Note: only Python 2.7 failed. Python 3.5 - 3.8 tests passed. We discovered the discrepancy is because we use mingw32 to compile for Python 2.7, but msvc for Python 3: https://github.com/grpc/grpc/blob/master/tools/run_tests/artifacts/artifact_targets.py#L168.

Alternative tried

We temporarily try to use msvc to compile for Python 2.7 but run into a different set of errors:

Error:

.\src/core/lib/iomgr/error.h(24) : fatal error C1083: Cannot open include file: 'inttypes.h': No such file or directory

Full Log: https://source.cloud.google.com/results/invocations/e40abd1d-971c-43bc-8eb3-8b958b9a4375/targets/grpc%2Fcore%2Fpull_request%2Fwindows%2Fgrpc_build_artifacts/log

Logging this issue separately to track this investigation.

Next steps

  • Either see if we can get mingw updated
  • Or see if we can switch to using msvc
@stanley-cheung
Copy link
Contributor Author

Update: after talking to the protobuf team, we may have identified a PR that may not be properly merged. So we are trying to re-apply that commit in protocolbuffers/protobuf#7539. After protobuf team merged that commit, I will re-try our tests.

@stanley-cheung
Copy link
Contributor Author

@gnossen Looks like the fix is working. Here's a success run of the Python Windows Artifacts build after I applied the protobuf fix: https://source.cloud.google.com/results/invocations/468f6fe9-73b9-4098-b35b-b48cc9133298/targets/grpc%2Fcore%2Fpull_request%2Fwindows%2Fgrpc_build_artifacts/log.

I will monitor the protobuf fix - but looks like we don't have to change anything on our side now.

Thanks for helping me look into this!

@stanley-cheung
Copy link
Contributor Author

This is fixed by protocolbuffers/protobuf#7539. I verified that the Python Windows Artifacts build are passing after that protobuf fix.

Closing this as there are no more work from our side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants