I’d like to propose a patch to allow external modules to skip the creation of the ITKData target. We might want to discuss this here first.
The use case is that RTK embeds another external module internally: ITKCudaCommon. While it is possible to have a remote module depending on another remote (i.e building in ITK), the only limitation when building externally is that the inner module (ITKCudaCommon) will create the ITKData target before the top-level module (RTK). Resulting in RTK missing the test data.
I would completely understand if such a patch was rejected, because the right way would be to pull ITKCudaCommon outside of RTK. What do you think @dzenanz ?
So, when we build ITKCudaCommon as a remote module and RTK as a remote module we do not run into a target conflict, but when we build ITKCudaCommon as a remote module and RTK externally, we run into a target conflict? Or, is RTK trying to use testing data from ITKCudaCommon without also adding the testing data to RTK? If that is the case, can we just also add the testing data to RTK?
At this stage, we have not tried to take ITKCudaCommon out of RTK. If I remember correctly a recent conversation with @LucasGandel, what is blocking us from making it happen is the lack of separate tests for ITKCudaCommon, it is currently being tested via RTK’s tests and we do not have ressources to write a set of independent tests. Should it still be split and submitted to ITK without tests as a remote test? If yes, with which compliance level?
Not exactly. I believe @LucasGandel is still trying to work with ITKCudaCommon (written as an ITK module) as a subfolder of the RTK repository:
I think it should. It would be Compliance Level 2, with some (many?) checkmarks for higher compliance levels.
While we would gradually add tests to that remote over time, it would be good if the module were to be started with at least one test. That would probably make it compliance level 3.
Not exactly. I believe @LucasGandel is still trying to work with ITKCudaCommon (written as an ITK module) as a subfolder of the RTK repository:
Right! ITKCudaCommon and RTK are not decoupled for now. Building RTK as as remote builds ITKCudaCommon as a remote as well (everything goes smooth because both itk-module.cmake are found). But building RTK externally forces us to call add_subdirectory(ITKCudaCommon). This approach works well too, except for test data. We get a target conflict because both modules call ExternalDataAddTarget(ITKData) (in the same project cmake I guess).
I understand this is a ugly use case, and having a standalone ITKCudaCommon module is prefered here. But I think it’s worth discussing this as it could be used to have ITK remote modules splitted into submodules.
I think it should. It would be Compliance Level 2, with some (many?) checkmarks for higher compliance levels.
I think that would be many checkmarks, but this is a good way to move forward! If compliance level 2 or 3 are acceptable for now, then I can try to pull ITKCudaCommon out of RTK, clean it and add a test.
The DESTINATION should be “${ITK_INSTALL_PACKAGE_DIR}/Modules/Targets”
I’ll be happy to create a MR to fix this, but I’m not sure about the first point. @matt.mccormick it was introduced in 96b411f. Do you know if we still want this?
Then ITKUltrasound (and maybe some other modules) could optionally depend on ITKCudaCommon.