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

Add option to provide thirdparty dependencies externally #188

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Ylannl
Copy link
Member

@Ylannl Ylannl commented Oct 22, 2024

This adds a CMAKE option VAL3DITY_USE_INTERNAL_DEPS (default ON). When leaving this to the default (ON) it should build as before, ie. bundling the files in thirdparty as a val3dity_thirdparty lib and linking against that.

However, when set to OFF, it is expected that those 'thirdparty' dependencies (spdlog, nlohmann-json, tclap, pugixml) are available externally, eg. through vcpkg, and findable using CMAKE's find_package(). This is helpful in case val3dity is integrated as a library into another software that also depends those 'thirdparty' dependencies. I specifically had issues with spdlog, as this was publicly linked and it was a different version from what I used in my project, which can lead to undefined behaviour, unexpected crashes etc.

In addition

  1. VAL3DITY_LIBRARY is now an option, so can be easily toggled when using ccmake or cmake-gui.
  2. I restructered a bit the CMAKE file to make the above possible without making things too messy. Most notably I created an intermeditate val3dity_core object that contains most of the val3dity source code (all except main.cpp and val3dity.cpp). This object is reused both for the val3dity library and executable.
  3. I made linking as much as possible PRIVATE to prevent pulling unwanted includes into projects that use val3dity as a library.
  4. I had to change a few #include statements for nlohmann-json and pugixml to work in case they are externally provided. For the same reason I renamed the thirdparty/nlohmann folder.

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.

1 participant