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

ISMRMRD target_include_dir only if no ISMRMRD::ISMRMRD target #663

Closed
wants to merge 2 commits into from

Conversation

rijobro
Copy link
Contributor

@rijobro rijobro commented May 20, 2020

Fixes #659 (maybe)

@rijobro rijobro changed the title only target include dir if no target ISMRMRD target_include_dir only if no ISMRMRD::ISMRMRD target May 20, 2020
@KrisThielemans
Copy link
Member

feels a bit weird actually to specify a link target to tell CMake what to do with include files. probably it'd be better to have

target_include_directories(cgadgetron PUBLIC ISMRMRD::ISMRMRD)

but the CMake doc doesn't make it clear if this can be done.

Seems I'm pushing my CMake knowledge with this one...

@rijobro
Copy link
Contributor Author

rijobro commented May 20, 2020

My understanding was that an imported library would be created during the *Targets.cmake, which would itself use the target_include_directories. Then you could use target_link_libraries in your project and it would already know which headers are needed.

E.g. in ISMRMRDTargets.cmake, I have:

add_library(ISMRMRD::ISMRMRD SHARED IMPORTED)

set_target_properties(ISMRMRD::ISMRMRD PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/ismrmrd"
  INTERFACE_LINK_LIBRARIES "/usr/local/Cellar/hdf5/1.12.0/lib/libhdf5.dylib;/usr/local/opt/szip/lib/libsz.dylib;/usr/lib/libz.dylib;/usr/lib/libdl.dylib;/usr/lib/libm.dylib"
)

@KrisThielemans KrisThielemans added this to the v2.3 milestone May 28, 2020
@KrisThielemans
Copy link
Member

superseded by #840

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.

building SIRF picks-up old installed files
2 participants