----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66996/#review202621 -----------------------------------------------------------
3rdparty/Makefile.am Lines 326 (patched) <https://reviews.apache.org/r/66996/#comment284529> This variable name makes sense only 50% of the time, how about e.g., `GRPC_LIB_SUFFIX`? 3rdparty/Makefile.am Lines 341-350 (original), 339-348 (patched) <https://reviews.apache.org/r/66996/#comment284530> It is very hard to see what is actually being execute here in what environment. Could you reflow this to e.g., always pass the env after `make`, reorder the variables and reflow the code? $(MAKE) $(AM_MAKEFLAGS) \ $(LIB_GRPC:%=$(abs_builddir)/%) \ CC="$(CC)" \ CXX="$(CXX)" \ LD="$(CC)" \ LDXX="$(CXX)" \ CPPFLAGS="$(PROTOBUF_INCLUDE_FLAGS) \ $(SSL_INCLUDE_FLAGS) \ $(ZLIB_INCLUDE_FLAGS)" \ LDFLAGS="$(PROTOBUF_LINKER_FLAGS) \ $(SSL_LINKER_FLAGS) \ $(ZLIB_LINKER_FLAGS)" \ HAS_PKG_CONFIG=false \ NO_PROTOC=false \ PROTOC="$(PROTOC)" 3rdparty/Makefile.am Lines 341-342 (original), 339-340 (patched) <https://reviews.apache.org/r/66996/#comment284531> Why are we removing these extra flags? They don't seem to come from the normal flags? src/Makefile.am Lines 175 (patched) <https://reviews.apache.org/r/66996/#comment284532> Why are we removing these extra flags? They don't seem to come from the normal flags? src/Makefile.am Line 182 (original), 185 (patched) <https://reviews.apache.org/r/66996/#comment284533> It is not clear to me that this would always find the correct libraries. I see e.g., that the cmake build does not seem to use the `_unsecure` suffix even when the libraries where built without SSL. Do we need to explicitly discover these libraries during configure time to make sure we don't risk failing to link later? src/python/native_common/ext_modules.py.in Line 132 (original), 132 (patched) <https://reviews.apache.org/r/66996/#comment284534> This variable name makes sense only 50% of the time, how about e.g., `grpc_lib_suffix`? src/python/native_common/ext_modules.py.in Lines 151-152 (original) <https://reviews.apache.org/r/66996/#comment284535> How do we manage to link an object file (which has no dependency information) when SSL is enabled? - Benjamin Bannier On May 8, 2018, 5:23 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66996/ > ----------------------------------------------------------- > > (Updated May 8, 2018, 5:23 a.m.) > > > Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu. > > > Bugs: MESOS-8798 > https://issues.apache.org/jira/browse/MESOS-8798 > > > Repository: mesos > > > Description > ------- > > When the SSL build feature is disabled, Mesos now builds > `libgrpc_unsecure` and `libgrpc++_unsecure` instead of `libgrpc` and > `libgrpc++`, so the SSL headers and libraries are no longer required. > > NOTE: gRPC v1.10 no longer needs `-Wno-deprecated-declarations` and > `-Wno-unused-function` when building with OpenSSL v1.1. > > > Diffs > ----- > > 3rdparty/Makefile.am 8d9fa85dd65a94d91670d54dab36deea80d14996 > configure.ac 429797c35b93a6df69ba0cb0fc28cb66a3171074 > src/Makefile.am 7e91681e3b8b217f8b23fa5347e059640c62fc65 > src/python/native_common/ext_modules.py.in > 87387fd580c40b252fc82f98b5238b9b9120903a > > > Diff: https://reviews.apache.org/r/66996/diff/1/ > > > Testing > ------- > > This patch does not work standalone. Tests are done in the next patch. > > > Thanks, > > Chun-Hung Hsiao > >
