CMake adopt to latest eager runtime and reusable shared library in windows environment

[jackyko1991]

Due to some error in CLA checking #16394, a new pull request is made

I am currently working on the cmake files to generate shared library that allows users to develop with C++ interface.

It is easier to develop by adding export targets in tf_shared_lib.cmake, but I think in the future this function should be migrated to the main CMakeList.txt

[jackyko1991]

Accompany with this pull request is the following CMakeLists for C++ api hello world.

main.cxx can be found from https://www.tensorflow.org/api_guides/cc/guide

tested on windows 10 MSVC 2015 with CPU only, GPU version will be tested soon

cmake_minimum_required (VERSION 2.6)
project (tf_hello)

# Tensorflow
find_package(Tensorflow REQUIRED)
include_directories(${TENSORFLOW_INCLUDE_DIRS})

# compiler setting required by tensorflow, to be test on all compilers
# currently only tested on MSVC and GCC
if (${CMAKE_CXX_COMPILER_ID} STREQUAL MSVC) 
	add_definitions(-DCOMPILER_MSVC)
elseif (${CMAKE_CXX_COMPILER_ID} STREQUAL GNU)
	if (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS "3")
		add_definitions(-DCOMPILER_GCC3)
	else()
		add_definitions(-D__GNUC__)
	endif()
else()
	message(ERROR " compiler ${CMAKE_CXX_COMPILER_ID} not supported by this CMakeList.txt, under development")
endif()

add_executable(tf_hello main.cxx)
target_link_libraries(tf_hello ${TENSORFLOW_LIBRARIES})

[jackyko1991]

I am not very sure that this is good way to set correct platform. In typical cuda usage, the prepending cuda folder is not necessary since the cuda SDK is setted during dependency configuration.

[jackyko1991]

I am not familiar with bazel build, but this change get build pass with cmake.

[sarlinpe]

Would it be possible to have a better control over the exported symbols ? Currently we have a symbol conflict with OpenCV (because of libpng and libjpeg, see #14267).

Could we also work towards having GRPC optional on Linux ? The main issue would be with tensorflow/core/debug/debug_io_utils.h

[martinwicke]

It would be great to compile with all symbols hidden except those that need to be public. @allenlavoie, @gunan

[allenlavoie]

@skydes @martinwicke We already run a linker script for the C++ and C APIs on other platforms (we should do it here too). The issue with #14267 is that the C++ API doesn't include protocol buffer symbols, and to get those you need to link against something that needs to export the conflicting symbols (I've outlined a solution if you're interested, although it's probably not relevant to this PR since custom ops aren't supported).

[achalshah20]

@jackyko1991

It might break existing eigen dependent apps?. Shouldn't it be in include/tensorflow/xxxx/ as these are tensorflow specific?

# Eigen directory
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/eigen/src/eigen/Eigen/
        DESTINATION include/Eigen)
# external directory
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/external/eigen_archive/
        DESTINATION include/external/eigen_archive)
# third_party eigen directory
install(DIRECTORY ${tensorflow_source_dir}/third_party/eigen3/
        DESTINATION include/third_party/eigen3)
# unsupported Eigen directory
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/eigen/src/eigen/unsupported/Eigen/
        DESTINATION include/unsupported/Eigen)

[andreas-eberle]

It would be great to get this feature finished an merged. Having Windows CPU/GPU support could allow building the Go and Java APIs for Windows and even with GPU support. Please continue this work and hopefully the tensorflowlers can prioritize this.

[gunan]

With dropped cmake support, and bazel officially working, what is the latest on this PR?

[jackyko1991]

It contains windows API release with reusable cmake packages.

I am not sure if bazel build on windows get GPU support if I want to code with MSVC.

[gunan]

bazel build on windows does have GPU support, if that is what you are asking.
I think about this, we may be able to accept this PR, but we wont be running any continuous tests to see if the cmake build is healthy or not.
moreover, the cmakefiles will be removed in a few months, as contrib itself will be removed from the tensorflow repository.

[jackyko1991]

@gunan I still see there are updates on cmake build, though latest release already changed to bazel build on windows.

TF cc core is packed as a single .dll with cmake config. With this users can link TF with their own applications.

The C++ api solution is only provided in linux (https://www.tensorflow.org/guide/extend/cc) and also hard for users to get all the necessary dependencies. I believe cmake contrib should not be removed as this PR would give a better C++ api experience in windows.

[gunan]

Cmake build is deprecated, and the cmakefiles are going to be removed together with the removal of rest of contrib.

libtensorflow can be built on windows if you like to use the C++ API of TF. To pack things into a single DLL, you can simply use --config=monolithic with bazel build. So our bazel build is much more capable than the current cmake build. So I still do not understand what cmake build offers that bazel does not in terms of your requirements.

[jackyko1991]

So... should I close this PR?

[jackyko1991]

Btw will cmake TF build change to community supported? Most of cross platform C++ projects are on CMake, e.g. OpenCV, VTK, ITK. These are famous image processing toolkits and allow me to develop IO interfaces in specific domain application.

I am actually writing offline inference tools for medical image analysis, which greatly depends on cmake toolchains. But there is no way combing bazel output with cmake projects (e.g. header and dependency）. It is still worth to keep cmake.

[gunan]

You can reach out to [email protected] but the decision on cmake is unfortunately final.
There are volunteers in the mailing list that would like to keep cmake build working, but there is no official support for cmake going forward.

As for this pr, I am OK with accepting this, but I would like to see the changes in cmake folder and all the other changes (setup.py, C++ headers, etc) split to a different PR, to have better description on those. Because those changes have the potential to affect all of TF.

[tensorflowbutler]

Nagging Assignee @mrry: It has been 15 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.