building SIRF picks-up old installed files

[KrisThielemans]

Order of include directories is currently problematic. Checking with make VERBOSE=ON shows that cstir is compiled with an order like various-sirf-dirs .../install/include .../hdf5/... sirf-reg-dirs STIR-install-dir/include ...

This means that if there's an old STIR version in .../install/include, it gets picked up before the actual STIR-install-dir/include that I pointed to.

I guess (but am not sure) this is because cgadgetron uses ISMRMRD_INCLUDE_DIR,

SIRF/src/xGadgetron/cGadgetron/CMakeLists.txt

Line 47 in dcf7a4d

target_include_directories(cgadgetron PUBLIC "${ISMRMRD_INCLUDE_DIR}")

Possible ways to resolve this:

• why does cstir depend on cgadgetron?
• not use ismrmrd headers directly in cgadgetron include files, but this seems a lot of work
• make sure that cstir uses STIR include paths first
• don't use target_include_directories but just depend on ISMRMRD:ISMRMRD although that will likely give the same dependencies.

[rijobro]

For your last point, we've already made the change such that we don't use target_include_directories and only use the target ISMRMRD::ISMRMRD:

SIRF/src/xGadgetron/cGadgetron/CMakeLists.txt

Line 75 in dcf7a4d

target_link_libraries(cgadgetron PUBLIC ISMRMRD::ISMRMRD)

[rijobro]

The problem is that the SB installs everything into the same location. We could modify the SB installation method for better compartmentalisation? The env_ccppetmr.sh would have to be slightly more complicated, but everything should be tidier. E.g,:

Install
    STIR
        include
        lib
        share
    Gadgetron
        include
        lib
        share
etc

[rijobro]

Lastly, could this be the problematic bit of code? Put in by me 17 months ago apparently. Not sure why it uses the install interface as we don't currently install our headers.

SIRF/src/xSTIR/cSTIR/CMakeLists.txt

Lines 39 to 42 in dcf7a4d

	target_include_directories(cstir PUBLIC
	"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>$<INSTALL_INTERFACE:include>")
	target_include_directories(cstir PUBLIC
	"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>$<INSTALL_INTERFACE:include>")

[KrisThielemans]

For your last point, we've already made the change such that we don't use target_include_directories and only use the target ISMRMRD::ISMRMRD:

yep, but we still have the target_include_directories in L47 above. Not needed? (if we can remove it, it'd have to be moved inside the if(WIN32) for now).

[KrisThielemans]

We could modify the SB installation method for better compartmentalisation?

we could do, and probably should be versioning as well, although I feel that that kind of thing should be handled by the packages. In any case I think this is a SIRF problem. SIRF itself has no control where things are going to be installed.

[KrisThielemans]

Lastly, could this be the problematic bit of code?

No, I don't think so. The INSTALL_INTERFACE stuff is there for exported CMake config (which we don't right now, but will soon). It is ignored during build time.

Another reason why I don't think this is the case is that I was originally building SIRF with BUILD_Gadgetron=OFF, therefore not including gadgetron. I didn't have this problem then. (I had another one down the line, but not relevant to this issue).

[KrisThielemans]

cstir depends on cgadgetron because of

SIRF/src/common/CMakeLists.txt

Lines 42 to 44 in dcf7a4d

	target_link_libraries(Syn PUBLIC cgadgetron cstir Reg)
	INSTALL(TARGETS Syn DESTINATION ${CMAKE_INSTALL_PREFIX}/lib)
	target_link_libraries(csirf PUBLIC iutilities Syn)

I think this is #622, so possibly the current issue will disapper once we fix that.

[rijobro]

Sorry, for your last code snippet, why does this mean that cstir depends on cgadgetron? I would have thought it meant that Syn depends on cstir and cgadgetron, but not that they depend on each other.