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

TriBITS: cmake gets incorrect Trilinos_INCLUDE_DIRS on installed System #10100

Open
e4t opened this issue Jan 18, 2022 · 5 comments
Open

TriBITS: cmake gets incorrect Trilinos_INCLUDE_DIRS on installed System #10100

e4t opened this issue Jan 18, 2022 · 5 comments
Labels
DO_NOT_AUTOCLOSE This issue should be exempt from auto-closing by the GitHub Actions bot. TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework type: bug The primary issue is a bug in Trilinos code or tests

Comments

@e4t
Copy link

e4t commented Jan 18, 2022

Bug Report

@trilinos/TriBITS

Description

When <project_name>__INSTALL_LIB_DIR and <project_name>_INCLUDE_DIR are of different path type (ie relative/absolute) or have a different root paths, the logic in cmake/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake to determine the <package_name>_INCLUDE_DIRS for <package_name>Config_install.cmake may be wrong.
Reason: The path is calculated relative to ${CMAKE_CURRENT_LIST_DIR} by counting the number of directory elements in ${PROJECT_NAME}_INSTALL_LIB_DIR backwards. With <project_name>__INSTALL_LIB_DIR being an absolute path, this will take the path back to /. When <project_name>_INCLUDE_DIR is relative, this will simply be appended, ignoring any setting of CMAKE_INSTALL_INCLUDEDIR.
I'm afraid the logic to obtain a relocatable path will only work reliably when both paths are relative. Thus, if any one of these aren't one should probably give up on the idea to make the installation relocatable - or at least prepend the value of CMAKE_INSTALL_INCLUDEDIR to any relative path (this will still give the wrong results if <project_name>_INCLUDE_DIR is not below CMAKE_INSTALL_INCLUDEDIR).

Steps to Reproduce

  1. Release Tag: trilinos-release-13-2-0
  2. Run:
 cmake \
 [...]
  -DCMAKE_INSTALL_PREFIX=/usr/local \
  -DTrilinos_INSTALL_LIB_DIR:PATH=/usr/local/lib64 \
  -DTrilinos_INSTALL_INCLUDE_DIR:STRING=include/trilinos \
 [...]
  1. Looking at Kokkos for example, build/packages/kokkos/CMakeFiles/KokkosConfig_install.cmake will have:
...
## List of package include dirs
set(Kokkos_INCLUDE_DIRS "${CMAKE_CURRENT_LIST_DIR}/../../../../include/trilinos")

## List of package library paths
set(Kokkos_LIBRARY_DIRS "${CMAKE_CURRENT_LIST_DIR}/../../../..//usr/lib64")
...

The former (Kokkos_INCLUDE_DIRS) is clearly wrong with CMAKE_CURRENT_LIST_DIR being /usr/local/lib64/cmake, this will point to /include/trilinos.

  1. Run make; make install
  2. Create a cmake project with the line FindPackage(Trilinos)
  3. configure and observe the variable Trilinos_INCLUDE_DIRS
    Expected result: /usr/local/include/trilinos
    Actual result: /include/trilinos
@e4t e4t added the type: bug The primary issue is a bug in Trilinos code or tests label Jan 18, 2022
@jhux2
Copy link
Member

jhux2 commented Jan 18, 2022

@bartlettroscoe

@jhux2 jhux2 added the TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework label Jan 18, 2022
@bartlettroscoe
Copy link
Member

@e4t, thanks for reporting this. I am in the middle of refactoring this code as part of a larger refactoring (see Epic TriBITSPub/TriBITS#367) and I will take a look at this. But note that newer versions of TriBITS/Trilinos are moving to modern CMake targets where client CMake projects will just be linking to IMPORTED targets which know about their include directories. See:

But I will have a look. (I saw that code but I did not write it and was not sure what the logic was so I will write some unit tests and fix this.)

@e4t
Copy link
Author

e4t commented Jan 19, 2022

@bartlettroscoe, thanks for the feedback! I've read about the TriBITS restructure: from what I read in the CMake documentation using the export/import mechanism seems to address the relocatable package issue in a more consistent manner without requiring extra logic. So this issue may be moot.

@bartlettroscoe
Copy link
Member

@e4t, also related to this issue is the item "Use standard install locations defined by the standard GNUInstallDirs.cmake module" as part of the Epic TriBITSPub/TriBITS#411. Adopting that CMake standard will break backwards compatibility but will also be part of solving the relocatable installs problem.

@github-actions
Copy link

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Jan 21, 2023
@bartlettroscoe bartlettroscoe added DO_NOT_AUTOCLOSE This issue should be exempt from auto-closing by the GitHub Actions bot. and removed MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. labels Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO_NOT_AUTOCLOSE This issue should be exempt from auto-closing by the GitHub Actions bot. TriBITS Issues with the TriBITS framework itself, not usage of the TriBITS framework type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

3 participants