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

CMake: Fix install not finding external arrow for dynamic linking #5375

Merged
merged 1 commit into from Mar 4, 2024

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Mar 2, 2024

Before this PR, if rerun_sdk was build with RERUN_DOWNLOAD_AND_BUILD_ARROW set to OFF and RERUN_ARROW_LINK_SHARED set to ON, the downstream build of a package that uses rerun_sdk via find_package(rerun_sdk) had the following problems:

  • A "Rerun got built with an automatically downloaded version of libArrow" warning was printed, even if rerun was built against a system version of arrow
  • If the arrow system installation did not shipped Arrow::arrow_static (as it is the case for conda-forge, at least for the libarrow package), the CMake failed as the non-existent Arrow::arrow_static target was referenced.

These two problems are solved in these PR by:

  • Only printing the "Rerun got built with an automatically downloaded version of libArrow" warning if @RERUN_DOWNLOAD_AND_BUILD_ARROW@ is OFF
  • Define the rerun_arrow_target target using the value of RERUN_ARROW_LINK_SHARED used when building rerun.

What

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

@Wumpf Wumpf added 🪳 bug Something isn't working 🌊 C++ API C or C++ logging API include in changelog labels Mar 4, 2024
@Wumpf Wumpf changed the title Fix Config.cmake.in logic for find arrow in case external shared arro… Fix Config.cmake.in logic for find arrow in case external shared arrow Mar 4, 2024
@Wumpf Wumpf changed the title Fix Config.cmake.in logic for find arrow in case external shared arrow Fix cmake install not finding external arrow Mar 4, 2024
@Wumpf Wumpf changed the title Fix cmake install not finding external arrow Fix cmake install not finding external arrow for dynamic arrow linking) Mar 4, 2024
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

great catch, thanks again!

@Wumpf Wumpf merged commit 29baeb3 into rerun-io:main Mar 4, 2024
35 of 40 checks passed
@traversaro traversaro changed the title Fix cmake install not finding external arrow for dynamic arrow linking) Fix cmake install not finding external arrow for dynamic arrow linking Mar 4, 2024
@traversaro
Copy link
Contributor Author

great catch, thanks again!

Thanks for the fast feedback!

@emilk emilk changed the title Fix cmake install not finding external arrow for dynamic arrow linking CMake: Fix install not finding external arrow for dynamic linking Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 🌊 C++ API C or C++ logging API include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants