Skip to content

[ENH][packages/slicer] Introduce external packages #52

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

Closed
wants to merge 5 commits into from

Conversation

asiangoldfish
Copy link
Contributor

To minimise Slicer's footprint, packages should be installed separately. This is currently not implemented, and as a temporary solution packages are instead symlinked to Slicer's source tree. This patch uses Plots as an example.

To minimise Slicer's footprint, packages should be installed
separately. This is currently not implemented, and as a temporary
solution packages are instead symlinked to Slicer's source tree.
This patch uses Plots as an example.
@asiangoldfish asiangoldfish added the enhancement New feature or request label May 5, 2025
@asiangoldfish asiangoldfish self-assigned this May 5, 2025
Copy link
Contributor

@RafaelPalomar RafaelPalomar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asiangoldfish, modularity is one of the key selling points of SystoleOS, and also one of the most difficult to implement. This PR delivers on the most simple approach we discussed last week (modularity can be achieved by developers tweaking the package. e.g., commenting out, adding modules to the slicer pacakge).

@asiangoldfish, please add to this PR an external module. The one that will be immediately useful to us is the OpenIGTLinkIF. This could also help for testing of your external modules approach.

;; -----------------------------------------------------------------------------
;; Slicer Core Modules
;; -----------------------------------------------------------------------------
(define slicer-plots
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this won't lead to a binary installation, I would call this slicer-plots-src

oeramo added a commit to OUH-MESHLab/BrainlabMirror that referenced this pull request May 6, 2025
Progress up to this point tracked in SystoleOS/guix-systole#52
oeramo added a commit to OUH-MESHLab/BrainlabMirror that referenced this pull request May 6, 2025
Progress up to this point tracked in SystoleOS/guix-systole#52
@asiangoldfish
Copy link
Contributor Author

asiangoldfish commented May 6, 2025

@RafaelPalomar After symlinking OpenIGTLinkIF to Slicer's source tree, I noticed that OpenIGTLinkIO is required as build input. This project uses vtkMutexLock from VTK, which is deprecated from version 9.2. Seems like it also was removed from the project.

Error log:

/tmp/guix-build-openigtlinkio-a262c1f.drv-0/OpenIGTLinkIO-32feaaa5fcff433dec4f071cf7aaa9106e5e5124/Logic/igtlioCircularSectionBuffer.cxx:14:10: fatal error: vtkMutexLock.h: No such file or directory
   14 | #include <vtkMutexLock.h>
      |          ^~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [Logic/CMakeFiles/igtlioLogic.dir/build.make:135: Logic/CMakeFiles/igtlioLogic.dir/igtlioCircularSectionBuffer.cxx.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/tmp/guix-build-openigtlinkio-a262c1f.drv-0/OpenIGTLinkIO-32feaaa5fcff433dec4f071cf7aaa9106e5e5124/Logic/igtlioCircularBuffer.cxx:14:10: fatal error: vtkMutexLock.h: No such file or directory
   14 | #include <vtkMutexLock.h>
      |          ^~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [Logic/CMakeFiles/igtlioLogic.dir/build.make:121: Logic/CMakeFiles/igtlioLogic.dir/igtlioCircularBuffer.cxx.o] Error 1
/tmp/guix-build-openigtlinkio-a262c1f.drv-0/OpenIGTLinkIO-32feaaa5fcff433dec4f071cf7aaa9106e5e5124/Logic/igtlioConnector.cxx:36:10: fatal error: vtkMutexLock.h: No such file or directory
   36 | #include <vtkMutexLock.h>
      |          ^~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [Logic/CMakeFiles/igtlioLogic.dir/build.make:149: Logic/CMakeFiles/igtlioLogic.dir/igtlioConnector.cxx.o] Error 1
make[2]: Leaving directory '/tmp/guix-build-openigtlinkio-a262c1f.drv-0/build'
make[1]: *** [CMakeFiles/Makefile2:226: Logic/CMakeFiles/igtlioLogic.dir/all] Error 2
make[1]: Leaving directory '/tmp/guix-build-openigtlinkio-a262c1f.drv-0/build'
make: *** [Makefile:139: all] Error 2
error: in phase 'build': uncaught exception:

A snippet from igtlioCircularSectionBuffer.cxx:

// VTK includes
#include "igtlioCircularSectionBuffer.h"

#include <vtkObjectFactory.h>
#include <vtkMutexLock.h>
#include <vtksys/SystemTools.hxx>

@oeramo
Copy link
Contributor

oeramo commented May 6, 2025

@asiangoldfish Similar errors regarding vtkMutexLock were a recurring thing in the IGSIO package. You may want to look at how I patched it in https://github.com/SystoleOS/guix-systole/blob/SCRUM-112-Build-PlusToolkit/guix-systole/packages/patches/0024-BUG-packages-igsio-replace-mutex-lock.patch.

@asiangoldfish asiangoldfish marked this pull request as draft May 7, 2025 01:34
@asiangoldfish
Copy link
Contributor Author

@RafaelPalomar Hi. In commit facba45 I get the following error:

/tmp/guix-build-slicer-5.8-5.8.1.drv-0/Slicer-11eaf62e5a70b828021ff8beebbdd14d10d4f51c/Modules/External/OpenIGTLinkIF/OpenIGTLinkIF/MRML/vtkMRMLIGTLConnectorNode.cxx:40:10: fatal error: vtkMRMLMarkupsFiducialNode.h: No such file or directory
   40 | #include "vtkMRMLMarkupsFiducialNode.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [Modules/External/OpenIGTLinkIF/OpenIGTLinkIF/MRML/CMakeFiles/vtkSlicerOpenIGTLinkIFModuleMRML.dir/build.make:79: Modules/External/OpenIGTLinkIF/OpenIGTLinkIF/MRML/CMakeFiles/vtkSlicerOpenIGTLinkIFModuleMRML.dir/vtkMRMLIGTLConnectorNode.cxx.o] Error 1

I would think that linking against Slicer's vtkSlicerMarkupsModuleMRML would solve this problem, but that seems to not be the case.

@RafaelPalomar
Copy link
Contributor

I would think that linking against Slicer's vtkSlicerMarkupsModuleMRML would solve this problem, but that seems to not be the case.

@asiangoldfish, have a look a some of the patches for some of the "heavily" modified extensions in the gentoo-overlay. I remember having to add include and link dirs, etc. in some. I can think of: https://github.com/SystoleOS/gentoo-overlay/blob/master/Slicer-Extension/SlicerIGSIO/files/0001-ENH-Port-SlicerIGSIO-to-Systole-Slicer.patch

@asiangoldfish
Copy link
Contributor Author

asiangoldfish commented May 7, 2025

@RafaelPalomar Do you remember if you encountered errors related to DCMTK? Example:

CMake Warning (dev) at CMake/SlicerMacroBuildLoadableModule.cmake:191 (add_library):
  Policy CMP0028 is not set: Double colon in target name means ALIAS or
  IMPORTED target.  Run "cmake --help-policy CMP0028" for policy details.
  Use the cmake_policy command to set the policy and suppress this warning.

  Target "qSlicerOpenIGTLinkIFModule" links to target "DCMTK::ofstd" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  Modules/External/OpenIGTLinkIF/OpenIGTLinkIF/CMakeLists.txt:84 (slicerMacroBuildLoadableModule)
This warning is for project developers.  Use -Wno-dev to suppress it.

[..]

CMake Error at CMake/SlicerMacroBuildLoadableModule.cmake:191 (add_library):
  Target "qSlicerOpenIGTLinkRemoteModule" links to target "DCMTK::ofstd" but
  the target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  CMake/SlicerMacroBuildLoadableModule.cmake:347 (slicerMacroBuildLoadableModule)
  Modules/External/OpenIGTLinkIF/OpenIGTLinkRemote/CMakeLists.txt:48 (slicerMacroBuildQtModule)

In commit 3143a36 I used find_package(DCMTK REQUIRED COMPONENTS ofstd [..]) in the top level CMakeLists.txt and added DCMTK as libraries to link against. I tested this with the module of OpenIGTLinkIF, which was unsuccessful.

@asiangoldfish asiangoldfish force-pushed the symlink-modules-to-slicer-source branch from 025c95e to 3143a36 Compare May 7, 2025 18:40
@RafaelPalomar
Copy link
Contributor

This is superseded by #57 and #58

@asiangoldfish asiangoldfish deleted the symlink-modules-to-slicer-source branch May 23, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants