Skip to content

[math libs] Fix CMAKE_INSTALL_PREFIX init behavior on Win32. #343

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

Open
9 tasks
ScottTodd opened this issue Apr 2, 2025 · 1 comment
Open
9 tasks

[math libs] Fix CMAKE_INSTALL_PREFIX init behavior on Win32. #343

ScottTodd opened this issue Apr 2, 2025 · 1 comment
Labels
patch Used to track patches

Comments

@ScottTodd
Copy link
Member

ScottTodd commented Apr 2, 2025

Summary

Per the guidance at https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT.html, CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT can be "used by project code to change the default without overriding a user-provided value".

Without these patches:

  1. Our setting here is ignored:

    "-DCMAKE_INSTALL_PREFIX=${_stage_destination_dir}"

  2. Subprojects install into the hardcoded C:/hipSDK path

  3. Subprojects that depend on other subprojects fail to find expected files since they are searching the wrong locations:

    [build] [hipFFT configure] CMake Error at D:/projects/TheRock/cmake/therock_subproject_dep_provider.cmake:51 (find_package):
    [build] [hipFFT configure]   Could not find a package configuration file provided by "rocfft" with any
    [build] [hipFFT configure]   of the following names:
    [build] [hipFFT configure]
    [build] [hipFFT configure]     rocfftConfig.cmake
    [build] [hipFFT configure]     rocfft-config.cmake
    

    In that case, C:\hipSDK\lib\cmake\rocfft\rocfft-config.cmake exists but TheRock\build\math-libs\rocFFT\dist\lib\cmake\rocfft\rocfft-config.cmake does not.

Link to patch

See #340

Sent to subprojects

No PRs sent yet

  • hipBLAS-common
  • hipBLAS
  • rocBLAS
@ScottTodd
Copy link
Member Author

ScottTodd commented Apr 8, 2025

Looks like I missed some math-libs/BLAS sub-projects. They will need similar patches from

if (WIN32)
  SET( CMAKE_INSTALL_PREFIX "C:/hipSDK" CACHE PATH "Install path" FORCE )

to

if (WIN32)
  if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
    SET( CMAKE_INSTALL_PREFIX "C:/hipSDK" CACHE PATH "Install path" FORCE )
  endif()

ScottTodd added a commit that referenced this issue Apr 9, 2025
See #343. On Windows, these
projects have been force setting `CMAKE_INSTALL_PREFIX` to `C:/hipSDK`,
regardless of what any superproject or user requests. The canonical way
to set a default install prefix is to first check
`CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT`:
https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT.html
and then _not_ use `FORCE`. These are minimal changes to stop the
sub-projects from grossly misbehaving.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Used to track patches
Projects
Development

No branches or pull requests

1 participant