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] Use an object library for STIR_REGISTRIES #1141

Merged
merged 7 commits into from Feb 6, 2023

Conversation

KrisThielemans
Copy link
Collaborator

Using an object library means that the registries get compiled only once, therefore speeding up the build process quite a lot.

It's also easier to use when building a project that uses STIR as we just need to depend on STIR_LIBRARIES. However, we do set the STIR_REGISTRIES variable empty at export, such that external libraries can still use

  add_executable(bla ${STIR_REGISTRIES})

Fixes #83

Using an object library means that the registries get compiled only once,
therefore speeding up the build process quite a lot.

It's also easier to use when building a project that uses STIR as
we just need to depend on STIR_LIBRARIES. However, we do set
the STIR_REGISTRIES variable empty at export, such that external
libraries can still use
  add_executable(bla ${STIR_REGISTRIES})
@KrisThielemans
Copy link
Collaborator Author

This worked and resulted in great speed-up. Sadly, it doesn't work for SIRF as its executables now do not have the stir registries at all. The reason is that target_link_libraries doesn't use transitive behaviour for OBJECT libraries. Extensive discussion at https://gitlab.kitware.com/cmake/cmake/-/issues/18090

(I also have some linking errors with the current STIR commit related to HDF5, but not sure if these are related)

We still need to use the STIR_REGISTRIES variable as
target_link_libraries does not use object libraries transitively.
@KrisThielemans
Copy link
Collaborator Author

The previous commit re-enables the requirement to use ${STIR_REGISTRIES} in external projects to avoid problems with the transitive behaviour. STIR_REGISTRIES is now set (in STIRConfig.cmake) to $<TARGET_OBJECTS:stir_registries>` which works fine on my Linux VM. Fingers crossed.

@KrisThielemans
Copy link
Collaborator Author

Currently, the exported STIRConfig.cmake will either work for CMAKE < 3.12 or > 3.12, but not both.

@KrisThielemans
Copy link
Collaborator Author

This is finished (aside from release notes). @danieldeidda @markus-jehl @robbietuk @NikEfth could you try this one out please? Especially if you have something that uses find_package(STIR).

@markus-jehl
Copy link
Contributor

Just tested it and it works fine, as far as I can tell.

@KrisThielemans KrisThielemans merged commit 593c069 into UCL:master Feb 6, 2023
@KrisThielemans KrisThielemans deleted the OBJECT_library branch February 6, 2023 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

speed-up building using object libraries
2 participants