CMake: Enable installation of cpp-package headers #13339
Conversation
|
@mxnet-label-bot add [pr-awaiting-review] |
|
@marcoabreu @lebeg @larroy for review |
There was a problem hiding this comment.
Change looks good.
Does it work for you? I have some errors when I run install:
-- Installing: /usr/local/include/gtest/internal/gtest-string.h
-- Installing: /usr/local/include/gtest/internal/gtest-linked_ptr.h
-- Installing: /usr/local/include/gtest/internal/gtest-type-util.h.pump
-- Installing: /usr/local/include/gtest/internal/gtest-param-util.h
-- Installing: /usr/local/include/gtest/internal/gtest-death-test-internal.h
-- Installing: /usr/local/include/gtest/internal/gtest-internal.h
-- Installing: /usr/local/include/gtest/internal/gtest-type-util.h
-- Installing: /usr/local/include/gtest/internal/gtest-param-util-generated.h
-- Installing: /usr/local/include/gtest/internal/gtest-filepath.h
-- Installing: /usr/local/include/gtest/internal/gtest-param-util-generated.h.pump
-- Installing: /usr/local/include/gtest/internal/custom
-- Installing: /usr/local/include/gtest/internal/custom/gtest.h
-- Installing: /usr/local/include/gtest/internal/custom/gtest-port.h
-- Installing: /usr/local/include/gtest/internal/custom/gtest-printers.h
-- Installing: /usr/local/include/gtest/gtest-printers.h
-- Installing: /usr/local/include/gtest/gtest_pred_impl.h
-- Installing: /usr/local/include/gtest/gtest-param-test.h
-- Installing: /usr/local/include/gtest/gtest-death-test.h
-- Installing: /usr/local/include/gtest/gtest-test-part.h
-- Installing: /usr/local/lib/libdmlc.a
-- Up-to-date: /usr/local/./include
-- Installing: /usr/local/./include/dmlc
CMake Error at 3rdparty/dmlc-core/cmake_install.cmake:40 (file):
file INSTALL cannot set permissions on "/usr/local/./include/dmlc"
Call Stack (most recent call first):
cmake_install.cmake:68 (include)
| @@ -730,6 +730,7 @@ install(TARGETS ${MXNET_INSTALL_TARGETS} | |||
| ) | |||
|
|
|||
| install(DIRECTORY include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | |||
| install(DIRECTORY 3rdparty/tvm/nnvm/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | |||
There was a problem hiding this comment.
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 )
There was a problem hiding this comment.
That's a bit out of scope for what I intended with the PR, but sure.
You can find CMake's documentation here.
| @@ -730,6 +730,7 @@ install(TARGETS ${MXNET_INSTALL_TARGETS} | |||
| ) | |||
|
|
|||
| install(DIRECTORY include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | |||
| install(DIRECTORY 3rdparty/tvm/nnvm/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | |||
There was a problem hiding this comment.
Isn't mshadow needed as well?
There was a problem hiding this comment.
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.
Yes, I use these changes[1][2] in our vcpkg fork[3] on Windows (7, 10, Server 2016) where I build and deploy a plugin (shared library) using the CPP package, linked against mxnet's shared library. [1] https://git.imp.fu-berlin.de/bioroboticslab/robofish/vcpkg/blob/master/ports/mxnet/0003-install-cpp-package.patch |
|
It doesn't #13578 |
|
@MoritzMaxeiner Can you retrigger the build for this PR? It seems to be approved |
|
@Roshrini Sure, didn't realize that was needed in this case. |
|
@Roshrini All checks seem to have passed, now. |
|
@marcoabreu @KellenSunderland to review/merge this PR? |
marcoabreu
added
pr-awaiting-merge
* Allow CMake based installation of cpp-package * Add installation of missing nnvm headers * Add documentation as to where public headers will be installed
* Allow CMake based installation of cpp-package * Add installation of missing nnvm headers * Add documentation as to where public headers will be installed
This allows installing the cpp-package via CMake's install target: