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

[develop branch] Standalone cmake issues #826

Closed
dholladay00 opened this issue May 19, 2017 · 14 comments
Closed

[develop branch] Standalone cmake issues #826

dholladay00 opened this issue May 19, 2017 · 14 comments
Labels
Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos)

Comments

@dholladay00
Copy link

dholladay00 commented May 19, 2017

It appears that commit 9583090 by @bartgol has caused issues with my standalone cmake.

These are the changes I made which allowed kokkos to build again w/in my project:

diff --git a/cmake/kokkos.cmake b/cmake/kokkos.cmake
index ecee74a..6cca8c5 100644
--- a/cmake/kokkos.cmake
+++ b/cmake/kokkos.cmake
@@ -981,10 +981,10 @@ ENDFOREACH()
 
 # set up include-directories
 SET (Kokkos_INCLUDE_DIRS
-    ${CMAKE_SOURCE_DIR}/core/src
-    ${CMAKE_SOURCE_DIR}/containers/src
-    ${CMAKE_SOURCE_DIR}/algorithms/src
-    ${CMAKE_BINARY_DIR}  # to find KokkosCore_config.h
+    ${CMAKE_CURRENT_SOURCE_DIR}/core/src
+    ${CMAKE_CURRENT_SOURCE_DIR}/containers/src
+    ${CMAKE_CURRENT_SOURCE_DIR}/algorithms/src
+    ${CMAKE_CURRENT_BINARY_DIR}  # to find KokkosCore_config.h
 )
 
 INCLUDE_DIRECTORIES(${Kokkos_INCLUDE_DIRS})
@@ -1160,11 +1160,11 @@ file(RELATIVE_PATH REL_INCLUDE_DIR "${INSTALL_CMAKE_DIR}"
    "${INSTALL_INCLUDE_DIR}")
 # ... for the build tree
 set(CONF_INCLUDE_DIRS "${Kokkos_SOURCE_DIR}" "${Kokkos_BINARY_DIR}")
-configure_file(${CMAKE_SOURCE_DIR}/cmake/KokkosConfig.cmake.in
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/KokkosConfig.cmake.in
   "${Kokkos_BINARY_DIR}/KokkosConfig.cmake" @ONLY)
 # ... for the install tree
 set(CONF_INCLUDE_DIRS "\${Kokkos_CMAKE_DIR}/${REL_INCLUDE_DIR}")
-configure_file(${CMAKE_SOURCE_DIR}/cmake/KokkosConfig.cmake.in
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/KokkosConfig.cmake.in
   "${Kokkos_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/KokkosConfig.cmake" @ONLY)
 
 # Install the KokkosConfig.cmake and KokkosConfigVersion.cmake

However, I am still not inheriting the kokkos include directories as I was before. Any ideas @dsunder, @bartgol?

@dholladay00 dholladay00 changed the title Standalone Cmake changes on develop branch [develop branch] Standalone cmake issues May 19, 2017
@dholladay00
Copy link
Author

Note: if there is a newer preferred way of doing things, that's fine, but also make sure those changes are reflected in /example/build_cmake such it always has the preferred method in it. Also, this is develop so I get what I deserve :-)

@bartgol
Copy link
Contributor

bartgol commented May 20, 2017

Ah! You're building kokkos as part of your project, correct? Yeah, I did not consider that. I guess for standalone nothing should change between CMAKE_SOURCE_DIR and CMAKE_CURRENT_SOURCE_DIR, but when kokkos is built inside another library, CMAKE_SOURCE_DIR refers to the "host" project. I think the cleanest thing to do is to use Kokkos_SOURCE_DIR, which can be defined once and for all at the beginning of kokkos' main CMakeLists.text.

@bartgol
Copy link
Contributor

bartgol commented May 20, 2017

As for the include directories not correctly inherited, I'll have to think about it. As soon as I can get my laptop out I'll check it out.

@bjoo
Copy link
Contributor

bjoo commented May 20, 2017

Hi All, I was looking at this... I have a partial fix in a fork

https://github.com/bjoo/kokkos/tree/cmake_changes

commit e77eab4

It involved changing CMAKE_SOURCE_DIR etc to Kokkos_SOURCE_DIR, CMAKE_BINARY_DIR to Kokkos_BINARY_DIR and also adding a bunch of include_directory() commands to pick up the includes both from Kokkos source and binary (which is where the config.h file came. However, it still has at least one flaw when building as part of my project. The kokkos include directories go into installdir/include, whereas it would be cleaner if they went into installdir/kokkos/include.

If you think it is adequate, I can create a pull req. OtherwiseI can wait for your solutions too.
Best wishes, B

@bartgol
Copy link
Contributor

bartgol commented May 20, 2017

Your fix sounds reasonable. As for the installation, we can perhaps do something like

SET(Kokkos_INSTALL_DIR ${CMAKE_INSTALL_PREFIX}/kokkos)

if kokkos is built as a sub project...

@bjoo
Copy link
Contributor

bjoo commented May 20, 2017 via email

@bartgol
Copy link
Contributor

bartgol commented May 20, 2017

I think it should be done from inside kokkos. One way to detect if kokkos is a subproject is to check whether CMAKE_PROJECT_NAME is already set...

@bartgol
Copy link
Contributor

bartgol commented May 20, 2017

Check if it is set before kokkos call to project.

(Sorry, writing from my phone)

@bjoo
Copy link
Contributor

bjoo commented May 20, 2017 via email

@bartgol
Copy link
Contributor

bartgol commented May 20, 2017

However, if you are installing kokkos, you may as well use ExternalProject_Add. The real difference is if you only build kokkos, and want to link to it without installing it. This is more difficult, and I don't think as it is it would work. Probably we would have to introduce the build/install interface generators to establish the include directories..

@bjoo
Copy link
Contributor

bjoo commented May 20, 2017

I couldn't get things to work with the Kokkos_INSTALL_DIR (maybe I did but it got overwritten later. However, I did manage to get the headers to install in include/kokkos for submodule builds with
commits 340cbc0 and then fixed up the ConfigKokkos.cmake also in commit 9ebe038 in my fork.

Libraries still get installed in CMAKE_INSTALL_DIR/lib and binaries in CMAKE_INSTALL_DIR/bin

This works for me. If Luca and Daniel are good with it I can submit a pull.

@mhoemmen
Copy link
Contributor

I guess for standalone nothing should change between CMAKE_SOURCE_DIR and CMAKE_CURRENT_SOURCE_DIR, but when kokkos is built inside another library, CMAKE_SOURCE_DIR refers to the "host" project.

This must be why TriBITS distinguishes between ${PROJECT} and ${PACKAGE}.

@bjoo bjoo mentioned this issue May 22, 2017
@ibaned
Copy link
Contributor

ibaned commented Jun 6, 2017

It looks like we forgot to label this issue decently, but #827 was merged and is now in master. Can this be closed?

@ibaned ibaned added Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) bug - fix pushed to develop branch labels Jun 6, 2017
@dholladay00
Copy link
Author

Fine with me

@ibaned ibaned closed this as completed Jun 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos)
Projects
None yet
Development

No branches or pull requests

5 participants