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

Set bob install dir to CMAKE_INSTALL_LIBDIR #451

Merged
merged 3 commits into from
Sep 8, 2024
Merged

Conversation

bobpaw
Copy link
Collaborator

@bobpaw bobpaw commented Aug 29, 2024

Set bob install dir to CMAKE_INSTALL_LIBDIR

  • Fixes issues on 64bit systems when LIBDIR=lib64. CMake would relink to $ORIGIN:$ORIGIN/../lib64 but all the libs would be in lib.

Copy link
Contributor

@cwsmith cwsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you.

What is the issue with relinking and $ORIGIN? Is that for capstone?

I see a handful of locations in bob.cmake where lib is used explicitly. I'm guessing those should also be changed.

cmake/bob.cmake Outdated Show resolved Hide resolved
@bobpaw
Copy link
Collaborator Author

bobpaw commented Aug 29, 2024

Looks good. Thank you.

What is the issue with relinking and $ORIGIN? Is that for capstone?

Yes. Capstone builds SCOREC/core and provides the shared libraries with capstone. That capstone library can be linked again to SCOREC (which will be especially necessary if the plugin is provided by default), but the issue I was having was that ld would find the shared libraries (ma.so, mds.so, gmi.so) in the capstone folder instead of the ones I had installed and built (with cmake --install). Using the build tree was easy, but trying to use the install tree was having this lib64 issue due to relinking.

I see a handful of locations in bob.cmake where lib is used explicitly. I'm guessing those should also be changed.

I believe those are only CMake targets. I don't know the convention, but the Capstone package includes those in lib.

@cwsmith
Copy link
Contributor

cwsmith commented Aug 29, 2024

Thank you. I think I get the gist of the capstone problem.

It looks like the Professional CMake book (19th edition) suggests having a consistent directory for the libraries, <packageName>Config.cmake, and target files.

The install destination of that file (<packageName>Config.cmake) should use the variables defined by the GNUInstallDirs, which simplifies customization by Linux distributions and other packaging systems. [pg 541]

I didn't find a direct quote for the target files, but all the examples I could find (see pg 524) use the GNUInstallDirs variables.

I can help with these modifications and testing if needed.

@bobpaw
Copy link
Collaborator Author

bobpaw commented Sep 6, 2024

Ok, that makes sense. I will update the remaining lib to be CMAKE_INSTALL_LIBDIR. I will also update bin per the same guideline.

cmake/bob.cmake Outdated Show resolved Hide resolved
@cwsmith
Copy link
Contributor

cwsmith commented Sep 6, 2024

Thank you. I'm going to add a CI test to check that downstream packages can use our cmake package successfully.

@cwsmith
Copy link
Contributor

cwsmith commented Sep 7, 2024

@bobpaw Can you rebase on develop? It now has a CI test for a user build using the installed cmake config file?

- name: Build User Project
# only need to test with a single build config if the installed cmake config files work
if: matrix.compiler.name == 'GNU' && matrix.build_type == 'Release'
env:
MPICH_CXX: ${{matrix.compiler.CXX}}
MPICH_CC: ${{matrix.compiler.CC}}
run: |
cmake -S ${{github.workspace}}/doc -B ${{github.workspace}}/buildExample -DCMAKE_CXX_COMPILER=mpicxx -DSCOREC_PREFIX=${{github.workspace}}/build/install
cmake --build ${{github.workspace}}/buildExample

- Fixes issues on 64bit systems when LIBDIR=lib64. CMake would relink
  to $ORIGIN:$ORIGIN/../lib64 and all the libs would be in lib.

Signed-off-by: Aiden Woodruff <[email protected]>
- Update CMake exported target install dir to CMAKE_INSTALL_LIBDIR.
- Update executable destination to CMAKE_INSTALL_BINDIR.
- Add quotes to directory names in case of spaces.

Signed-off-by: Aiden Woodruff <[email protected]>
@cwsmith
Copy link
Contributor

cwsmith commented Sep 8, 2024

Thank you. The new test ran without issues.

https://github.com/SCOREC/core/actions/runs/10755354998/job/29826921049?pr=451#step:7:1

@cwsmith cwsmith merged commit 2d470f9 into develop Sep 8, 2024
4 checks passed
@cwsmith cwsmith deleted the apw/bob_libdir branch September 8, 2024 16:26
@cwsmith cwsmith added the v2.2.9 label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants