Synergistic IO dependency fix#1194
Conversation
The synergistic library/executables/tests have now been adapted for DISABLE_Gadgetron etc. The work-around in common/utilities.* has therefore been removed. a job with DISABLE_Gadgetron has been added to GHA Fixes SyneRBI#622
4f97a0b to
dc8bbb6
Compare
|
now rebased on top of master |
|
When using STIR release, we get when building pystir https://github.com/SyneRBI/SIRF/actions/runs/5073309434/jobs/9112163305?pr=1194#step:8:14957 while with STIR which |
- removed some superflous include statements - removed some predeclarations and explicitly include iutilities.h
- add more conditionals to see if certain things can be built - split csirf library into 2: csirf is now the C++ library, while cinterface-sirf contains the C wrappers. This is probably more logical, but also avoids a circular dependency on ImageDataWrapper (which sits in csyn, but depends on creg etc). ImageDataWrapper is currently not using in csirf, but only in cinterface-sirf (i.e. Python/MATLAB) - remove the option DISABLE_Synergistic when building Python/MATLAB stuff as disabling it would lead to linking errors (as they need ImageDataWrapper)
|
The above problem is fixed now. However, I still have a Python problem such as @evgueni-ovtchinnikov |
Apparently it is - when I do "Go To Definition" on the prototype in |
cinterface-Reg depends on STIR for TextWriter Also fix some MATLAB things related to TextWriter (untested)
|
@evgueni-ovtchinnikov this is pretty much finished now. I've tried to give descriptive log messages, so maybe that helps in the review. Most important change is that
There are a few things that we could still do here:
These things would be nice, but we could also do them later. Would be cleanest to do the first bullet though (and it's very simple, so I can still do that). |
|
Main comment: I very much welcome suggested changes (thought about them myself, but never attempted for lack of understanding of CMake intricacies), but they look too dramatic to me for 3.5 - should this not go to 4.0? I need more time to study your PR, for now just one minor comment that cannot wait: frankly, I find |
Currently sits on top of #1166, sorry. but I'll rebase once that's merged to
master. i first wanted to see if this would work. It's really only the last commit at the moment)