Update CAFFE2_LINK_LOCAL_PROTOBUF functionality.

* Continuation of facebookarchive/caffe2#2306 and based on Yangqing's PR at facebookarchive/caffe2#2326
* Put caffe2_protos as static library and link it whole to libcaffe2.so
* For protobuf::libprotobuf, only link it to libcaffe2_protos (and hence libcaffe2.so), but not any downstream library. This avoids manipulating protobuf objects across dll boundaries.
* After the above, during linking one will receive complaint that fixed_address_empty_string is not found. This is because we compiled protobuf with hidden visibility, and the fact that the generated caffe2.pb.h has an inline function that invokes the inline function in protobuf GetEmptyStringAlreadyInited()
* Added sed-like commands to replace the generated header to use caffe2::GetEmptyStringAlreadyInited() instead. And, in proto_utils.cc, implement a function that essentially routes the function call to protobuf's internal one. The reason this works is that, caffe2::G... is visible globally, and libcaffe2.so is able to see the real protobuf one. This ensures that we are always calling protobuf functions that are inside libcaffe2.so.

CMakeLists.txt

@@ -26,7 +26,7 @@ set(CAFFE2_CMAKE_BUILDING_WITH_MAIN_REPO ON)
 include(CMakeDependentOption)
 option(BUILD_BINARY "Build C++ binaries" ON)
 option(BUILD_DOCS "Build documentation" OFF)
-option(BUILD_CUSTOM_PROTOBUF "If set, build Caffe2's own protobuf under third_party" OFF)
+option(BUILD_CUSTOM_PROTOBUF "Build and use Caffe2's own protobuf under third_party" OFF)
 option(BUILD_PYTHON "Build Python binaries" ON)
 option(BUILD_SHARED_LIBS "Build libcaffe2.so" ON)
 cmake_dependent_option(

caffe2/CMakeLists.txt

@@ -78,9 +78,8 @@ install(FILES ${PROJECT_BINARY_DIR}/caffe2/core/macros.h
 
 
 # ---[ List of libraries to link with
-add_library(caffe2_protos $<TARGET_OBJECTS:Caffe_PROTO> $<TARGET_OBJECTS:Caffe2_PROTO>)
+add_library(caffe2_protos STATIC $<TARGET_OBJECTS:Caffe_PROTO> $<TARGET_OBJECTS:Caffe2_PROTO>)
 add_dependencies(caffe2_protos Caffe_PROTO Caffe2_PROTO)
-# TODO: enable using lite protobuf.
 # If we are going to link protobuf locally inside caffe2 libraries, what we will do is
 # to create a helper static library that always contains libprotobuf source files, and
 # link the caffe2 related dependent libraries to it.
@@ -98,10 +97,10 @@ target_include_directories(caffe2_protos INTERFACE $<INSTALL_INTERFACE:include>)
 # separately deploy protobuf binaries - libcaffe2.so will contain all functionalities
 # one needs. One can verify this via ldd.
 #
-# Todo item in the future includes:
-# (1) Properly define public API that do not directly depend on protobuf itself.
-# (2) expose the libprotobuf.a file for dependent libraries to link to.
-# (3) build libprotobuf with -fvisibility=hidden to avoid version conflicts.
+# TODO item in the future includes:
+# (1) Enable using lite protobuf
+# (2) Properly define public API that do not directly depend on protobuf itself.
+# (3) Expose the libprotobuf.a file for dependent libraries to link to.
 #
 # What it means for users/developers?
 # (1) Users: nothing affecting the users, other than the fact that CAFFE2_LINK_LOCAL_PROTOBUF
@@ -110,11 +109,15 @@ target_include_directories(caffe2_protos INTERFACE $<INSTALL_INTERFACE:include>)
 #     nothing changes. If one has a dependent library that uses protobuf, then one needs to
 #     have the right protobuf version as well as linking to libprotobuf.a.
 target_link_libraries(caffe2_protos PUBLIC protobuf::libprotobuf)
-install(TARGETS caffe2_protos EXPORT Caffe2Targets DESTINATION lib)
 
 # Compile exposed libraries.
 add_library(caffe2 ${Caffe2_CPU_SRCS})
-target_link_libraries(caffe2 PUBLIC caffe2_protos ${Caffe2_PUBLIC_DEPENDENCY_LIBS})
+caffe2_interface_library(caffe2_protos caffe2_protos_whole)
+target_link_libraries(caffe2 PRIVATE caffe2_protos_whole)
+if (${CAFFE2_LINK_LOCAL_PROTOBUF})
+  target_link_libraries(caffe2 INTERFACE protobuf::libprotobuf)
+endif()
+target_link_libraries(caffe2 PUBLIC ${Caffe2_PUBLIC_DEPENDENCY_LIBS})
 target_link_libraries(caffe2 PRIVATE ${Caffe2_DEPENDENCY_LIBS})
 target_link_libraries(caffe2 PRIVATE ${Caffe2_DEPENDENCY_WHOLE_LINK_LIBS})
 target_include_directories(caffe2 INTERFACE $<INSTALL_INTERFACE:include>)
@@ -303,5 +306,3 @@ endif()
 
 # Finally, set the Caffe2_MAIN_LIBS variable in the parent scope.
 set(Caffe2_MAIN_LIBS ${Caffe2_MAIN_LIBS} PARENT_SCOPE)
-
-

cmake/MiscCheck.cmake

@@ -142,6 +142,14 @@ if (CAFFE2_COMPILER_SUPPORTS_AVX2_EXTENSIONS)
 endif()
 cmake_pop_check_state()
 
+# ---[ Checks if compiler supports -fvisibility=hidden
+check_cxx_compiler_flag("-fvisibility=hidden" COMPILER_SUPPORTS_HIDDEN_VISIBILITY)
+check_cxx_compiler_flag("-fvisibility-inlines-hidden" COMPILER_SUPPORTS_HIDDEN_INLINE_VISIBILITY)
+if (${COMPILER_SUPPORTS_HIDDEN_INLINE_VISIBILITY})
+  set(CAFFE2_VISIBILITY_FLAG "-fvisibility-inlines-hidden")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CAFFE2_VISIBILITY_FLAG}")
+endif()
+
 # ---[ If we are using msvc, set no warning flags
 # Note(jiayq): if you are going to add a warning flag, check if this is
 # totally necessary, and only add when you see fit. If it is needed due to

cmake/ProtoBuf.cmake

@@ -4,6 +4,7 @@ macro(custom_protobuf_find)
   message(STATUS "Use custom protobuf build.")
   option(protobuf_BUILD_TESTS "" OFF)
   option(protobuf_BUILD_EXAMPLES "" OFF)
+  option(protobuf_WITH_ZLIB "" OFF)
   if (APPLE)
     # Protobuf generated files triggers a deprecated atomic operation warning
     # so we turn it off here.
@@ -26,14 +27,27 @@ macro(custom_protobuf_find)
 
   if (${CAFFE2_LINK_LOCAL_PROTOBUF})
     # We will need to build protobuf with -fPIC.
-    set(__caffe2_protobuf_cmake_fpic ${CMAKE_POSITION_INDEPENDENT_CODE})
+    set(__caffe2_CMAKE_POSITION_INDEPENDENT_CODE ${CMAKE_POSITION_INDEPENDENT_CODE})
+    set(__caffe2_CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ${CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS})
+    set(__caffe2_CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
     set(CMAKE_POSITION_INDEPENDENT_CODE ON)
+    set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS OFF)
+    set(BUILD_SHARED_LIBS OFF)
+    if (${COMPILER_SUPPORTS_HIDDEN_VISIBILITY})
+      set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden")
+    endif()
+    if (${COMPILER_SUPPORTS_HIDDEN_INLINE_VISIBILITY})
+      set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility-inlines-hidden")
+    endif()
   endif()
 
   add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/protobuf/cmake)
 
   if (${CAFFE2_LINK_LOCAL_PROTOBUF})
-    set(CMAKE_POSITION_INDEPENDENT_CODE ${__caffe2_protobuf_cmake_fpic})
+    set(CMAKE_POSITION_INDEPENDENT_CODE ${__caffe2_CMAKE_POSITION_INDEPENDENT_CODE})
+    set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ${__caffe2_CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS})
+    set(BUILD_SHARED_LIBS ON)
+    set(CMAKE_CXX_FLAGS ${__caffe2_CMAKE_CXX_FLAGS})
   endif()
 
   # Protobuf "namespaced" target is only added post protobuf 3.5.1. As a
@@ -94,7 +108,7 @@ endif()
 # convention we will use SYSTEM inclusion path.
 get_target_property(__tmp protobuf::libprotobuf INTERFACE_INCLUDE_DIRECTORIES)
 message(STATUS "Caffe2 protobuf include directory: " ${__tmp})
-include_directories(SYSTEM ${__tmp})
+include_directories(BEFORE SYSTEM ${__tmp})
 
 # If Protobuf_VERSION is known (true in most cases, false if we are building
 # local protobuf), then we will add a protobuf version check in
@@ -152,22 +166,42 @@ function(caffe2_protobuf_generate_cpp_py srcs_var hdrs_var python_var)
     else()
       set(DLLEXPORT_STR "")
     endif()
-    add_custom_command(
-      OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.cc"
-             "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.h"
-             "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}_pb2.py"
-      WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
-      COMMAND ${CMAKE_COMMAND} -E make_directory "${CMAKE_CURRENT_BINARY_DIR}"
-      COMMAND ${CAFFE2_PROTOC_EXECUTABLE} -I${PROJECT_SOURCE_DIR} --cpp_out=${DLLEXPORT_STR}${PROJECT_BINARY_DIR} ${abs_fil}
-      COMMAND ${CAFFE2_PROTOC_EXECUTABLE} -I${PROJECT_SOURCE_DIR} --python_out "${PROJECT_BINARY_DIR}" ${abs_fil}
-      DEPENDS ${CAFFE2_PROTOC_EXECUTABLE} ${abs_fil}
-      COMMENT "Running C++/Python protocol buffer compiler on ${fil}" VERBATIM )
+
+    if (${CAFFE2_LINK_LOCAL_PROTOBUF})
+      # We need to rewrite the pb.h files to route GetEmptyStringAlreadyInited
+      # through our wrapper in proto_utils so the memory location test
+      # is correct.
+      add_custom_command(
+        OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.cc"
+               "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.h"
+               "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}_pb2.py"
+        WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+        COMMAND ${CMAKE_COMMAND} -E make_directory "${CMAKE_CURRENT_BINARY_DIR}"
+        COMMAND ${CAFFE2_PROTOC_EXECUTABLE} -I${PROJECT_SOURCE_DIR} --cpp_out=${DLLEXPORT_STR}${PROJECT_BINARY_DIR} ${abs_fil}
+        COMMAND ${CAFFE2_PROTOC_EXECUTABLE} -I${PROJECT_SOURCE_DIR} --python_out "${PROJECT_BINARY_DIR}" ${abs_fil}
+
+        # If we remove all reference to these pb.h files from external
+        # libraries and binaries this rewrite can be removed.
+        COMMAND ${CMAKE_COMMAND} -DFILENAME=${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.h -P ${PROJECT_SOURCE_DIR}/cmake/ProtoBufPatch.cmake
+
+        DEPENDS ${CAFFE2_PROTOC_EXECUTABLE} ${abs_fil}
+        COMMENT "Running C++/Python protocol buffer compiler on ${fil}" VERBATIM )
+    else()
+      add_custom_command(
+        OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.cc"
+               "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}.pb.h"
+               "${CMAKE_CURRENT_BINARY_DIR}/${fil_we}_pb2.py"
+        WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+        COMMAND ${CMAKE_COMMAND} -E make_directory "${CMAKE_CURRENT_BINARY_DIR}"
+        COMMAND ${CAFFE2_PROTOC_EXECUTABLE} -I${PROJECT_SOURCE_DIR} --cpp_out=${DLLEXPORT_STR}${PROJECT_BINARY_DIR} ${abs_fil}
+        COMMAND ${CAFFE2_PROTOC_EXECUTABLE} -I${PROJECT_SOURCE_DIR} --python_out "${PROJECT_BINARY_DIR}" ${abs_fil}
+        DEPENDS ${CAFFE2_PROTOC_EXECUTABLE} ${abs_fil}
+        COMMENT "Running C++/Python protocol buffer compiler on ${fil}" VERBATIM )
+    endif()
   endforeach()
 
   set_source_files_properties(${${srcs_var}} ${${hdrs_var}} ${${python_var}} PROPERTIES GENERATED TRUE)
   set(${srcs_var} ${${srcs_var}} PARENT_SCOPE)
   set(${hdrs_var} ${${hdrs_var}} PARENT_SCOPE)
   set(${python_var} ${${python_var}} PARENT_SCOPE)
 endfunction()
-
-

cmake/ProtoBufPatch.cmake

@@ -0,0 +1,22 @@
+# CMake file to replace the string contents in Caffe and Caffe2 proto.
+# Usage example:
+#   cmake -DFILENAME=caffe2.pb.h -P ProtoBufPatch.cmake
+
+file(READ ${FILENAME} content)
+string(
+  REPLACE
+  "::google::protobuf::internal::GetEmptyStringAlreadyInited"
+  "GetEmptyStringAlreadyInited"
+  content
+  "${content}")
+string(REPLACE
+	"namespace caffe2 {"
+	"namespace caffe2 { const ::std::string& GetEmptyStringAlreadyInited(); "
+	content
+	"${content}")
+string(REPLACE
+	"namespace caffe {"
+	"namespace caffe { const ::std::string& GetEmptyStringAlreadyInited(); "
+	content
+	"${content}")
+file(WRITE ${FILENAME} "${content}")

cmake/Summary.cmake

@@ -9,9 +9,6 @@ function (caffe2_print_configuration_summary)
   message(STATUS "  System                : ${CMAKE_SYSTEM_NAME}")
   message(STATUS "  C++ compiler          : ${CMAKE_CXX_COMPILER}")
   message(STATUS "  C++ compiler version  : ${CMAKE_CXX_COMPILER_VERSION}")
-  message(STATUS "  Protobuf compiler     : ${PROTOBUF_PROTOC_EXECUTABLE}")
-  message(STATUS "  Protobuf include path : ${PROTOBUF_INCLUDE_DIRS}")
-  message(STATUS "  Protobuf libraries    : ${PROTOBUF_LIBRARIES}")
   message(STATUS "  BLAS                  : ${BLAS}")
   message(STATUS "  CXX flags             : ${CMAKE_CXX_FLAGS}")
   message(STATUS "  Build type            : ${CMAKE_BUILD_TYPE}")
@@ -22,7 +19,11 @@ function (caffe2_print_configuration_summary)
   message(STATUS "  BUILD_BINARY          : ${BUILD_BINARY}")
   message(STATUS "  BUILD_CUSTOM_PROTOBUF : ${BUILD_CUSTOM_PROTOBUF}")
   if (${CAFFE2_LINK_LOCAL_PROTOBUF})
-    message(STATUS "  CAFFE2_LINK_LOCAL_PROTOBUF : ${CAFFE2_LINK_LOCAL_PROTOBUF}")
+    message(STATUS "    Link local protobuf : ${CAFFE2_LINK_LOCAL_PROTOBUF}")
+  else()
+    message(STATUS "    Protobuf compiler   : ${PROTOBUF_PROTOC_EXECUTABLE}")
+    message(STATUS "    Protobuf includes   : ${PROTOBUF_INCLUDE_DIRS}")
+    message(STATUS "    Protobuf libraries  : ${PROTOBUF_LIBRARIES}")
   endif()
   message(STATUS "  BUILD_DOCS            : ${BUILD_DOCS}")
   message(STATUS "  BUILD_PYTHON          : ${BUILD_PYTHON}")