-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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/grpc PR #88
Protobuf/grpc PR #88
Conversation
Hi @KindDragon, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
I use commit from master (not latest version) because it has advanced CMake script https://github.com/grpc/grpc/blob/ef5a2eb2678d38d35ff79c4d692bf48603e1c439/CMakeLists.txt |
Please help me if you can 😃 |
This may be related to #77. I'm going to work on fixing that issue first, then I'll take a look here. |
I believe I've tweaked protobuf into working; here's the portfile I ended up with:
[note: I needed to add a target extraction directory argument to the extract command, so you'll need to pull from master to use this] Essentially, I discard the built After using this version of I have not successfully built anything using this package (I'm not familiar enough with protobuf to prove to myself that it works); if this does work, could you either post back here (I'll check it in separately) or submit a separate PR? |
6d0a94c
to
aa21b48
Compare
Thank you, working on grpc |
aa21b48
to
84b7b42
Compare
84b7b42
to
b8a894f
Compare
Should work now, but with statically linked protobuf (because of protobuf bug) |
ec656ce
to
5efb41b
Compare
grpc create some exe tools too, but they do not publish tools on GitHub :( gen_hpack_tables.exe, gen_legal_metadata_characters.exe, gen_percent_encoding_tables.exe, grpc_cpp_plugin.exe, grpc_create_jwt.exe, grpc_csharp_plugin.exe, grpc_node_plugin.exe, grpc_objective_c_plugin.exe, grpc_print_google_default_creds_token.exe, grpc_python_plugin.exe, grpc_ruby_plugin.exe, grpc_verify_jwt.exe |
5efb41b
to
41aab79
Compare
Are any of these executables used for C++ code generation? Or are they general purpose diagnostics / other language integration? |
grpc_cpp_plugin.exe used for that, so we need at least that tool |
Since protobuf only works on x86 and x64 anyway, for now it should be fine to put the built grpc_cpp_plugin.exe into a In the future, it would be better to find prebuilt copies of the tools (otherwise, when targeting arm, we'll need to do multiple builds). |
41aab79
to
7af1d8d
Compare
Is there any chance of merging the protobuf PR separately as this is the last library I need for my projects and would love to not duplicate work. |
OPTIONS | ||
-Dprotobuf_BUILD_SHARED_LIBS=OFF | ||
-Dprotobuf_MSVC_STATIC_RUNTIME=OFF | ||
-Dprotobuf_WITH_ZLIB=ON |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for forcing ZLIB on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, we don't have a good story for optional dependencies (we're working on it!). Since zlib is so ubiquitous, it's cheap to require and provides more functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because ZLIB disabled only for MSVC
if (MSVC)
set(protobuf_WITH_ZLIB_DEFAULT OFF)
else (MSVC)...
I think this is only because protobuf authors don't have zlib on Windows, but we have now zlib in vcpkg.
Thanks for the extensive process here; I'm glad we're getting closer to having a really solid solution for tools. |
Cannot build gRPC for vcpkg
config-x86-windows-rel-out.log.txt
config-x86-windows-rel-err.log.txt