Skip to content
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

[CMake] Optionally support building TF as a shared lib #9124

Merged
merged 2 commits into from
Apr 11, 2017

Conversation

vit-stepanovs
Copy link

Currently, TF is already built as a shared library that is included in the Python package. However, that library:

  1. Is implicitly linked against Python libs, and thus expects Python to be present on the machine wherever it is used. It is undesirable for scenarios not requiring Python (e.g. native application that need TensorFlow).
  2. Does not include the TF C++ API.

This PR allows optionally building TF as a stand-alone DLL that does not have the above issues. I am also working on allowing CMake to link all C++ tests against that DLL, and will submit such changes in a separate PR.

As a bonus, this PR also fixes a build break for tf_tools.cmake when GPU is enabled.

@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@mrlzla
Copy link

mrlzla commented Apr 11, 2017

It seems like you save my life ;) Thank you!

@tomjaguarpaw
Copy link

@vit-stepanovs I should run cmake with -DBUILD_SHARED_LIB=ON but then what build target do I run?

@vit-stepanovs
Copy link
Author

@tomjaguarpw, the actual option name is tensorflow_BUILD_SHARED_LIB, not BUILD_SHARED_LIB. If you use that, cmake generates a project tensorflow.vcxproj (not tensorflow.sln!) which you can use to build the DLL.

@drpngx
Copy link
Contributor

drpngx commented Apr 11, 2017

Nice!

LGTM, @mrry what do you think?

@drpngx
Copy link
Contributor

drpngx commented Apr 11, 2017

Jenkins, test this please.

@drpngx drpngx requested a review from mrry April 11, 2017 19:17
Copy link
Contributor

@mrry mrry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too!

@drpngx
Copy link
Contributor

drpngx commented Apr 11, 2017

Thanks!

@ivan-aksamentov
Copy link

It works great on both Linux and Windows for me. Why is this PR removed from release 1.1.0? (it was there in 1.1.0-rc2)

@drpngx
Copy link
Contributor

drpngx commented May 2, 2017

It's probably because it didn't make it before the cutoff point when we forked the branch.

/CC: @av8ramit

@av8ramit
Copy link

av8ramit commented May 2, 2017

I don't believe this was in 1.1.0-rc2, but yes we decided to push it back to 1.2 since it was merged too late for 1.1 to test properly.

@mrlzla
Copy link

mrlzla commented May 11, 2017

Is there some way to build tensorflow as static lib?

@adennie
Copy link

adennie commented May 12, 2017

I tried this in conjunction with #9666 to try to build a debug DLL,, but the generated .def file had over 120K symbols, so it exceeded the 65535 limit. Any way to filter more aggressively?

@drpngx
Copy link
Contributor

drpngx commented May 12, 2017 via email

@girving
Copy link
Contributor

girving commented May 12, 2017

Nice. Is there a thread I should notify once we make progress here? This one seems merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet