Adding a CI build to a Slicer extension

If an extension depends on another extensions then you need to pass the build folder of each dependency in <DependentExtensionName>_DIR CMake variable.

Alternatively, you can put all s4ext files of the extensions you want to build into a folder and build this list of extensions as described here.

FYI, I’ve built Slicer from slicer/slicer-base and pushed it to lassoan/slicer-built:20230730. It contains the full Slicer build tree, so it can be used for building any extension right away. It is quite big, though, so it may take a while for CI to download it. If it works then we could set up a repeating task to create a new image from latest Slicer source every night.

OK, but this applies to the SlicerDMRI extension: how do I tell the Slicer build to download and build the UKFTractography extension? Do I need to proceed the exact same way as with SlicerDMRI by first cloning UKFTractography, then configuring, building? Luckily, it looks like the latter does not depend on other extensions.

Alternatively, you can put all s4ext files of the extensions you want to build into a folder and build this list of extensions as described here.

Not sure if I follow this, or how the linked page responds to the question above.

Thanks for the effort. Will consider it at some point. For now, building Slicer from scratch is uncovering things that may need some attention, documenting, etc., so I would prefer to try to build it that way for now.

Exactly. The inconvenience is that you need to run check out source code and build multiple extensions in the right order. If you simply download the list of s4ext files that you want to build and put it in a folder and run the single CMake command described here then it downloads and builds all the extensions automatically (in the correct order, taking into account dependencies between them).

To get started, it may be simpler to follow the provided instructions and defaults. If you change defaults then you will get to non-well-documented territory and may discover problems, which may not be the best use of your time. We attempt to fix all issues, but it is not necessary (and not possible) to test and fix all the billions of combinations of build options.

Andras, I am turning off modules or options that are available as options in CMake: if they are necessary to build Slicer, I am fine, but then they should not be made available as options in CMake.

There are dozens of flags and of course not all combinations are tested and not all makes sense. Since it would be impractical to document (and maintain documentation) of all possible interplays between all options, we do not even attempt or plan to do this.

For example, turning off a Slicer core module is a very invasive action with unforeseeable consequences. There may be compile-time or runtime dependencies, there may be hard dependencies that would prevent some other modules from working at all, or absence of a module may just disable certain features in other modules. Some dependencies may be changed or removed by investing some work, if there is a good reason to do so. Even if we could explore and document some of the side effects of disabling a module in Slicer core, it is practically impossible to evaluate all the effects in all extensions continuously; or ask extension developers to perform this kind of impact analysis, document the results, and keep it up-to-date continuously.

Therefore, I would recommend to keep all options at the default value unless you have a strong reason to change one. Reducing build time or package size by changing default options may worth the effort for a well-funded commercial project, but probably not worth it for automated testing of an open-source extension.

It’s also worth noting, not just for Slicer but for any software, that configuration options that are not regularly tested should be considered experimental and/or expected not to work until proven. I’m sure that’s true for ITK and VTK for example, and I’m sure we’ve all encountered that. For Slicer this means that the configuration used on the factory machines with the documented build systems is the starting point for any experiments.

Perhaps we should make this statement explicit in the build documentation. We could even generate a cmake warning whenever a configuration is changed such that the build is going down and untested pathway.

If there are configurations that are important for key use cases then we should set up CI to keep them tested and working.

Agreed Steve: to me, the point is that if a module is part of the core, not sure why they should be optional. An intermediate step would be maybe to mark them as Advanced variables or grouped under a Core category.

Above all, I am grateful for all the work that you and the community has put and puts into this, and I am more than happy to continue helping, needless to say this. I have good reasons to set up a CI on GitHub for the extension at issue. We can discuss this on a call if necessary.

Thanks for the pointers. Had a look at this.

According to the extension index, SlicerDMRI seems not to depend on any other extension/module according to its extension index file:
https://github.com/Slicer/ExtensionsIndex/blob/main/SlicerDMRI.s4ext#L5

I now see that the configuration process, a few lines later,
ENH: Add GitHub actions build, test workflow file for CI · jhlegarreta/SlicerDMRI@67e378d · GitHub

says:

-- Setting EXTENSION_DEPENDS ...................: UKFTractography

Which I assume stems from the extension’s CMakeLists.txt: https://github.com/SlicerDMRI/SlicerDMRI/blob/610533480896d2884cb9ab0671c88874d237247d/CMakeLists.txt#L14

So there seems to be mismatch between what the extension states as its dependencies in its CMakeLists.txt file and the extension index s4ext file recipe.

Is there an automated way to keep these in sync?

Unfortunately not. The information in the CMakeLists.txt file and the s4ext file are redundant and may not be in sync - see more details here.

I like this idea :+1:

Just curious, does the nightly build of the SlicerDMRI extension across all platforms using the Slicer factory machines not suffice for the work that you are doing? Are you trying to avoid self-building the extension against a local build tree? Or are you also avoiding committing a change and then having to wait until the next day to see if the build is successful on the Slicer factory machines? Just wondering what development procedures others are using and how to improve things for development of other extensions

The standard for CI today is immediate build for pull requests. It would be nice to achieve this for Slicer, too. Currently, most extension developers rely on local builds and the nightly automatic build. The only exception is OpenDose3D, which has CI pipeline set up on gitlab.

Andras has already stated the point in his answer, but the idea is to be sure that any commit to the code (in this case, an extension) does not get merged unless (at least) tests are passing. The nightly builds are wonderful, and I am grateful for those of you who have set them up and maintain them, but they do not ensure this (and besides some maintainers, contributors rarely have a look at them, in my experience). This can save many hours of work when trying to collaborate with others, find bugs, etc.

Got it. So the desire is to gate keep more to make sure code doesn’t break things prior to integration. This certainly seems more ideal.

For Slicer core and Slicer extensions it has historically been an integrate changes first, then review the build and testing results the following day to see what unexpectedly broke. This primarily being because developers likely test on one platform say Windows, but then their changes break things on macOS or Linux. Are you planning on setting up CI builds for Linux, macOS and Windows?

Thanks for understanding James.

A CI running on all three OS, and assuming some essential tests check basic things, this risk would be minimized. A number of things would be discarded at least.

Long-term, ideal goal. Making this work on Linux would already be a first step. The rest can come later.

OK, I understand. Thanks Andras. Have gone through the repository, issue and documentation.

A few thoughts:

  • At first, I thought that adding a parser to generate the s4ext file from the top-level CMakeLists.txt file of each extension would be useful, but I see that this is being done here if I’m not mistaken using CMake, the issue then being the extension index repository not containing the same/reliable/duplicate information ?
  • Maybe such parser, e.g. a Python script, could still be useful to keep the information in sync ? It could directly read the extension’s CMakeLists.txt file (so the very same source the above CMake script uses) to populate the s4ext file? Ideally, the script could be executed nightly with a GHA workflow. The caveat would be that two different scripts, within different workflows, would be doing the same job.
  • Otherwise, a nightly run Slicer could push the generated s4ext files to the ExtensionsIndex repository and automatically open a PR if something has changed in the files ? This is maybe doable with little to reasonable effort ?

Last two builds are passing:
Actions · jhlegarreta/SlicerDMRI · GitHub

So the PR is ready to be merged:
ENH: Add GitHub actions build, test workflow file for CI by jhlegarreta · Pull Request #172 · SlicerDMRI/SlicerDMRI · GitHub

Thanks for all the pointers and thoughts.