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 updates #602

Open
wants to merge 79 commits into
base: master
Choose a base branch
from
Open

Cmake updates #602

wants to merge 79 commits into from

Conversation

sjunges
Copy link
Contributor

@sjunges sjunges commented Aug 28, 2024

This changes the build process significantly, mostly by cleaning up the cmake files.

  • We now set the version of the SO files.
  • We associate headers with targets.
  • Changes how carl is included. It now fetches the content from a prespecified git repo at configure time. You can change the repo and the tag via the cmake options.
    If you want to use the sources from an existing carl installation, you can use
    -DFETCHCONTENT_SOURCE_DIR_CARL=~/carl or any other path.
  • Changes how Sylvan is included. It now handles sylvan as a subproject.
  • Changes the CI to test installations, checks flags from makefile rather than from storm output.

This also fixes #631 by no longer copying header files to the include path.
This also fixes #610 as installs are now tested

As it uses target_sources with file_sets, this requires cmake 3.23 and thus no longer supports ubuntu 22.04.

  • Requires release 1.10 before we continue.
  • Website needs update.
  • Generate adequate in-buildtree config files
  • Review TODOs in cmake
  • Handle sylvan assertion that fails in tests (currently disabled, see Sylvan raises assertion error when running tests in debug build #676 )
  • Handle sylvan warnings in debug mode (currently disabled)
  • The CI currently uses a storm-starter-project from my own repo/a specific branch. This should change.
  • I suggest to make GLPK optional to simplify the build script (Make glpk optional #678)

@sjunges
Copy link
Contributor Author

sjunges commented Nov 12, 2024

Also, it requires moves-rwth/carl-storm#46

@volkm volkm mentioned this pull request Jan 26, 2025
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.

clangd go-to-definition goes to include files Testing cmake installs
1 participant