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] Protobuf no longer compiles with vs2019 Update 16.10 w/ c++latest #18251

Merged

Conversation

maxgolov
Copy link
Contributor

@maxgolov maxgolov commented Jun 2, 2021

Describe the pull request

  • What does your PR fix?

Temporary Workaround for protocolbuffers/protobuf#8688

The fix is needed to unblock compiling OpenTelemetry C++ SDK with vs2019-update16.10 . Note that the previous compiler was fine because 16.9 did not have support for constinit. I am not sure if it's a bug in Visual Studio 2019 support of constinit. Thus, the safest is to avoid using the feature in protobuf.

  • Which triplets are supported/not supported? Have you updated the [CI baseline]

All previous triplets supported. No changes intended. Verified that local build on Windows msvc2019-16.10 with C++20 passes all tests.

  • Does your PR follow the [maintainer guide]

Yes.

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes.

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

Opening it as a draft because it is my first PR in this repository.

Patch to turn off constinit with Visual Studio 2019 Update 16.10
Apply port_def.patch
Increment port-version
@maxgolov
Copy link
Contributor Author

maxgolov commented Jun 3, 2021

I'm not sure why the tests are failing. Seems like unrelated to my change? I'm converting this to ready-for-review. Please kindly let me know if I mess up something.

@maxgolov maxgolov marked this pull request as ready for review June 3, 2021 17:18
@maxgolov
Copy link
Contributor Author

maxgolov commented Jun 3, 2021

Fix has been merged in protobuf mainline in the master : protocolbuffers/protobuf#8690

But the current port, that uses slightly older release of protobuf still requires the patch here. Patch file needs to be removed once the port is updated to latest version of protobuf.

@maxgolov
Copy link
Contributor Author

maxgolov commented Jun 3, 2021

Apologies for this, I'm new to this process. I reran vcpkg x-add-version protobuf and added the change.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jun 9, 2021
@strega-nil-ms
Copy link
Contributor

Cool, this is good, thanks @maxgolov :)

@strega-nil-ms strega-nil-ms merged commit 7d472dd into microsoft:master Jun 10, 2021
@maxgolov maxgolov deleted the maxgolov/protobuf_vs2019_16.10 branch June 11, 2021 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants