grpc-sys: Use grpc headers found by pkgconfig. #505
Conversation
|
|
ssloboda
force-pushed
the
ssloboda/fix-cross-compile-bindgen
branch
from
a419fea
to
3c66e70
Compare
Previously the bundled grpc headers were always used to generate bindings even in the case where pkgconfig was used to locate gRPC to link against. This caused errors at runtime, e.g., SEGFAULT, rather than a compile-time error. To fix this, use the headers found by pkgconfig to generate bindings rather than than the bundled headers if pkgconfig is already being used. Signed-off-by: Steven Sloboda <ssloboda@starry.com>
ssloboda
force-pushed
the
ssloboda/fix-cross-compile-bindgen
branch
from
3c66e70
to
2c15028
Compare
|
Can you explain more about the error? The headers should be the same files across platform. |
|
From the commit message:
This is not a problem that I expect to come up often however it will make things easier for those who dynamically link against a system installation of gRPC that may not match the bundled version exactly. This allows for semver compatible version bumps of gRPC independent of the Does that clear things up? |
grpc-sys/build.rs
Outdated
| @@ -344,7 +344,7 @@ fn bindgen_grpc(file_path: &PathBuf) { | |||
| // Determine if need to update bindings. Supported platforms do not | |||
| // need to be updated by default unless the UPDATE_BIND is specified. | |||
| // Other platforms use bindgen to generate the bindings every time. | |||
| fn config_binding_path() { | |||
| fn config_binding_path(#[allow(unused_variables)] grpc_include_dir: &PathBuf) { | |||
There was a problem hiding this comment.
You can name it _grpc_include_dir to get around the warning.
| @@ -307,7 +307,7 @@ fn bindgen_grpc(file_path: &PathBuf) { | |||
| let cfg = config | |||
| .header("grpc_wrap.cc") | |||
| .clang_arg("-xc++") | |||
| .clang_arg("-I./grpc/include") | |||
| .clang_arg(format!("-I{}", grpc_include_dir.display())) | |||
There was a problem hiding this comment.
This doesn't seem to be correct. For example, include directory should be "/usr/local/include" instead of "/usr/local/include/grpc".
Everytime updating the bundled version, there has to be some adaption here or there. So I doubt if it can work correctly. Though I agree compile time error is better than runtime. |
|
Thanks for the review @BusyJay! I've address the issues you pointed out in a fixup commit for ease of reviewing, will squash once you give the okay. Let me know if there's anything else you want changed. |
Signed-off-by: Steven Sloboda <ssloboda@starry.com>
ssloboda
force-pushed
the
ssloboda/fix-cross-compile-bindgen
branch
from
099f9a1
to
cc29ee2
Compare
|
Hi @ssloboda do you have time to address the comment and sign off your commits? We may release next version soon, it will be nice to deliver your patch as well. |
Ran into this issue when cross compiling after updating libraries. I had originally fixed and tested this on v0.7.1 but it looks like a PR was just merged so I rebased. Here's the original fix in case you're interested. https://github.com/StarryInternet/grpc-rs/tree/ssloboda/fix-cross-compile-bindgen-v0.7.1
Happy to refactor if needed.