Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

CMake: Enable installation of cpp-package headers #13339

Merged
merged 3 commits into from
Dec 31, 2018
Merged

CMake: Enable installation of cpp-package headers #13339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13b9c9b
Allow CMake based installation of cpp-package
Nov 20, 2018
fef210b
Add installation of missing nnvm headers
Nov 20, 2018
3968296
Add documentation as to where public headers will be installed
Dec 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,12 @@ install(TARGETS ${MXNET_INSTALL_TARGETS}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
)

# NOTE: Public headers will be installed into ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}, see
# https://cmake.org/cmake/help/v3.0/variable/CMAKE_INSTALL_PREFIX.html
# https://cmake.org/cmake/help/v3.0/module/GNUInstallDirs.html

install(DIRECTORY include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
install(DIRECTORY 3rdparty/tvm/nnvm/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add comments to where are we installing things by default? it's quite hidden in CMAKE documentation where they go. ( I had to dig a bit )

Copy link
Contributor Author

@MoritzMaxeiner MoritzMaxeiner Dec 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

That's a bit out of scope for what I intended with the PR, but sure.
You can find CMake's documentation here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See a005e9c

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't mshadow needed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I could tell, no public headers include it. At least, my C++ project importing the full cpp-package header works without mshadow being installed.

if (INSTALL_EXAMPLES)
install(DIRECTORY example DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME})
endif()
Expand Down
2 changes: 2 additions & 0 deletions cpp-package/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,6 @@ if(USE_CPP_PACKAGE)
add_subdirectory(example)
endif()

install(DIRECTORY include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})

endif()