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

next-release. Use CMAKE_PROJECT_DIR or CMAKE_CURRENT_SOURCE_DIR instead of CMAKE_SOURCE_DIR to be able to use HIPRT as a submodule? #13

Open
TomClabault opened this issue Jul 5, 2024 · 4 comments
Assignees

Comments

@TomClabault
Copy link

Maybe this is already planned but just in case:

Suppose I have this project structure:

MyProject/
├── CMakeLists.txt
└── HIPRT/
    ├── CMakeLists.txt
    ├── contrib
    ├── hiprt
    ├── scripts
    ├── test
    ├── tools
    └── ...

If I want to use HIPRT in one of my project as a subdirectory, I would use add_subdirectory(HIPRT). However, the CMakeLists.txt of HIPRT currently uses ${CMAKE_SOURCE_DIR } to refer to the HIPRT/ directory:

include(${CMAKE_SOURCE_DIR}/contrib/Orochi/Orochi/enable_cuew.cmake)

In the case where HIPRT's CMakeLists.txt is called from add_subdirectory(HIPRT), ${CMAKE_SOURCE_DIR } refers to the MyProject/ directory instead of HIPRT/ as expected.

A solution would be to use either ${CMAKE_CURRENT_SOURCE_DIR } or ${CMAKE_PROJECT_DIR}

@RichardGe RichardGe self-assigned this Jul 8, 2024
@RichardGe
Copy link
Collaborator

Hi @TomClabault
thanks for this catch.
I prepared a new cmake file that should manage your usecase correctly.

here is a preview of it, in case you want to test it:
CMakeLists.txt
Note that the binaries will still be inside :
${CMAKE_CURRENT_SOURCE_DIR}/dist/bin

will be included in our next release.

@TomClabault
Copy link
Author

TomClabault commented Jul 8, 2024

I tested it on Windows with the VS2022 generator.

I could configure my project using HIPRT as a submodule with the new CMake but:

  • HIPRT doesn't seem to be built automatically with my project (when building my project from Visual Studio) although that's what I would expect from a subdirectory (added with `add_subdirectory). It behaves as if HIPRT wasn't a dependency of my project
  • I can't find a target to link against HIPRT with target_link_libraries. Internally the target seems to be ${HIPRT_NAME} but it is not exposed outside of the HIPRT CMake project.

Here the CMakeLists.txt I used for my testings along with the very simple main.txt for the testing the linking with the library (that's a .cpp file but Github doesn't want attached CPP files).

@TomClabault
Copy link
Author

After further testing, it seems that the reason why HIPRT isn't automatically built alongside the project that adds it as a dependency with add_subdirectory() is because it is linked with target_link_libraries.

I added these two lines to the HIPRT CMakeLists.txt:

add_library(hiprtLib INTERFACE)
target_link_libraries(hiprtLib INTERFACE ${HIPRT_NAME})

below add_library(${HIPRT_NAME} SHARED) and I linked my TestProject (from the CMakeLists in my previous comment) against hiprtLib:

target_link_libraries(TestHIPRTExec hiprtLib)

Now HIPRT compiles and generate a .lib file under dist/bin/.

So maybe in the end what's needed is just a "public" name to link against outside of HIPRT's CMakeLists.txt.

@RichardGe
Copy link
Collaborator

Thanks Tom.
I need to review your changes carefully as we have several projects depending on HIPRT and I need to make sure I don't break anything with those cmake changes.

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

No branches or pull requests

2 participants