Skip to content

RPATH handling and installation paths fix#601

Merged
TysonRayJones merged 11 commits intoQuEST-Kit:develfrom
eessmann:rpath-fix
May 1, 2025
Merged

RPATH handling and installation paths fix#601
TysonRayJones merged 11 commits intoQuEST-Kit:develfrom
eessmann:rpath-fix

Conversation

@eessmann
Copy link
Contributor

Hi Tyson
This code adjusts the install interface paths to use CMAKE_INSTALL_INCLUDEDIR and adds RPATH handling using the helper function setup_quest_rpath. This ensures consistent RPATH behaviour across platforms.
Erich

Adjusts install interface paths to use CMAKE_INSTALL_INCLUDEDIR
and adds RPATH handling using helper function setup_quest_rpath.
Ensures consistent RPATH behavior across platforms
@TysonRayJones
Copy link
Member

Wew thanks very much for looking at this! Could you PR this instead to the devel branch? Then we can easily test it with changes made to the examples building, and bundle it with a v4.1 merge into main.

@eessmann eessmann changed the base branch from main to devel April 30, 2025 14:12
@eessmann
Copy link
Contributor Author

eessmann commented Apr 30, 2025

Sure, no problem
The RPATH fix should clear up some issues I have been having on my quest rust bindings side project

@eessmann eessmann marked this pull request as ready for review April 30, 2025 14:14
@TysonRayJones
Copy link
Member

TysonRayJones commented Apr 30, 2025

Thanks very much! Btw do you think it's worth testing execution of the executables compiled during CI? The compiled examples (at least, those in examples/isolated) run in mere seconds, but could reveal a build issue like the bundling of the DLLs (as per #592).

If that's worthwhile, we could bundle the workflow change into this PR to test these build changes "for free". It should be as simple as adding code like below to the bottom of compile.yml:

      # attempt to run the compiled executable, testing e.g. DLL location
      - name: Run
        if: ${{ matrix.os == 'windows-latest' }}
        shell: bash
        run: |
          ./${{ env.build_dir }}/examples/isolated/Release/complex_arithmetic_c.exe
      - name: Run
        if: ${{ matrix.os != 'windows-latest' }}
        run: |
          ./${{ env.build_dir }}/examples/isolated/complex_arithmetic_c

If you think it's a good idea, I'll PR to your rpath-fix branch and we can squash-commit this PR together. Alternatively, you can make the above above edits yourself to compile.yml, though that might prove a headache; it will require my manually re-triggering the CI (auto-permissions disabled until the paid runners aren't on my credit card hehe), and experiencing the general horror of tinkering with Github Actions.

@eessmann
Copy link
Contributor Author

eessmann commented Apr 30, 2025

I think it's a great idea to run an isolated exe for a few secs in CI. It should catch most link time errors

specifically the `complex_arithmetic_c` example. This serves just to check for link-time errors due to problems in the build
since 'complex_arithmetic_c' was actually disabled on Windows and so output nothing, hehe!
@TysonRayJones TysonRayJones merged commit 1c051cf into QuEST-Kit:devel May 1, 2025
@eessmann eessmann deleted the rpath-fix branch May 5, 2025 09:32
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

Successfully merging this pull request may close these issues.

2 participants