Nightly build TESTS failing for extensions with dependencies to other scriptable modules

Hi there,

today I noticed that (for a long time) our extension tests has been failing (generic, but also our custom tests)

e.g. http://slicer.cdash.org/testDetails.php?test=8407693&build=1124707

Our mpReview extension has a dependency to ‘SlicerDevelopmentToolbox’

As you will notice, there are no dependencies loaded as additional-module-paths in the following cli command:

   /Users/kitware/Dashboards/Nightly/Slicer-0-build/Slicer-build/Slicer \"--no-splash\" \"--testing\" 
\"--launcher-additional-settings\" \"/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview-build/AdditionalLauncherSettings.ini\" 
\"--no-main-window\" \"--disable-cli-modules\" \"--additional-module-path\" 
\"/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview-build/lib/Slicer-4.9/qt-scripted-modules\" 
\"--additional-module-paths\" \"/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview-build/lib/Slicer-4.9/qt-scripted-modules\" 
\"/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview-build/lib/Slicer-4.9/cli-modules\" 
\"/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview-build/\" 
\"--python-code\" \"import slicer.testing; slicer.testing.runUnitTest([\'/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview-build\', 
\'/Users/kitware/Dashboards/Nightly/S-0-E-b/mpReview\'], \'qSlicermpReviewModuleGenericTest\')\"

Solution:
Slicer nightly tests just needs to have those additional dependencies loaded for running tests. Otherwise this doesn’t mirror the real world where I need to install an extension from the ExtensionManager where the downloaded extensions (dependencies) automatically get added to the Slicer “Additional module paths”. And after restarting the extension (optimally) works like a charm!

Hope we can find a simple and quick solution for that.

Thanks in advance.

Does your additional launcher settings file contain all the necessary paths?

SlicerDevelopmentToolbox is definitely missing. When building an extension, would this file automatically get extended by the missing dependencies? That’s what I would expect.

The AdditionalLauncherSettings.ini of my local mpReview build looks like the following:

[General]
additionalPathVariables=PYTHONPATH

[LibraryPaths]
1\path=/Users/christian/sources/py/mpReview/Build/lib/Slicer-4.7/cli-modules/.
2\path=/Users/christian/sources/py/mpReview/Build/lib/Slicer-4.7/qt-loadable-modules/.
size=2

[Paths]
1\path=/Users/christian/sources/py/mpReview/Build/lib/Slicer-4.7/cli-modules/.
2\path=/Users/christian/sources/py/mpReview/Build/bin/.
size=2

[EnvironmentVariables]

[PYTHONPATH]
1\path=/Users/christian/sources/py/mpReview/Build/lib/Slicer-4.7/qt-scripted-modules
2\path=/Users/christian/sources/py/mpReview/Build/lib/Slicer-4.7/qt-loadable-modules/.
3\path=/Users/christian/sources/py/mpReview/Build/lib/Slicer-4.7/qt-loadable-modules/Python
size=3

For reference I added a Mantis issue about the same thing
https://issues.slicer.org/view.php?id=4472
I haven’t figured it out yet either.

1 Like

Oh yeah. When building an extension which depends on other extensions I would assume the AdditionalLauncherSettings.ini to be extended by the dependencies, which probably would fix that issue.

In my case the extension GelDosimetryAnalysis depends on SlicerRT. The problem locally is that the SlicerRT_DIR CMake variable is not set. Instead it gives me this

CMake warning message

CMake Warning at C:/d/Slicer4/Extensions/CMake/SlicerBlockAdditionalLauncherSettings.cmake:48 (MESSAGE):
Dependent extension SlicerRT cannot be found by CMake find_package(),
therefore paths variables cannot be imported from this extension. The
problem can be resolved by generating SlicerRTConfig.cmake by adding
include(${Slicer_EXTENSION_GENERATE_CONFIG}) to the top-level
CMakeLists.txt of the dependent exension.
Call Stack (most recent call first):
C:/d/S4R/Slicer-build/UseSlicer.cmake:281 (include)
CMakeLists.txt:24 (include)

However include(${Slicer_EXTENSION_GENERATE_CONFIG}) is in the top-level CMakeLists, and SlicerRT does generate a SlicerRTConfig.cmake.

If I define SlicerRT_DIR manually, then it works nicely. It’s not done on the factory unfortunately, and this is where I am stuck now. We have a factory machine and was going to take a look at the issue on it, but I haven’t gotten to that point yet.

There is also something wrong here. The attribute name is not split into three depending extensions

image

Having more than one dependency was causing issues since I didn’t add those dependencies correctly within the CMakeLists.txt.

image

The following at leasts splits the dependencies into separate CMake attributes

set(EXTENSION_DEPENDS SlicerDevelopmentToolbox DCMQI PETDICOMExtension)

instead of

set(EXTENSION_DEPENDS "SlicerDevelopmentToolbox DCMQI PETDICOMExtension")

My mistake. But it still doesn’t resolve the dependencies. Not sure where find_package() is looking

1 Like

Yep :slight_smile: I reported the same thing to @jcfr but haven’t added a Mantis issue for this one. See screenshot

20171027_MultipleExtensionDependencies

Oh so if you don’t put them between quotation marks then it works? Thanks!!

1 Like

Don’t put it in quotation marks or separate it with semicolon:

Both work for me:

set(EXTENSION_DEPENDS dependency1 dependency2 dependency3)

set(EXTENSION_DEPENDS "dependency1;dependency2;dependency3")

Right. The template mentions the space separation, so that was fine. However the example “NA” is within quotes, and that made me put them between quotes. Maybe if we remove the quotes from the template or explain this in the same comment, then it’s less error-prone.

1 Like

Additional directories are collected recursively from all extensions that your extension depends on, and added to the additional launcher settings. Does it work until this point?

Without manually specifying SlicerRT_DIR in the GelDosimetryAnalysis CMake, it is not found, see explanation in earlier comment

Adding of _DIR for all required extensions is implemented in SlicerBlockBuildPackageAndUploadExtensions.cmake:

Maybe you could add a couple of MESSAGE prints in this file to see where things go wrong. If everything looks fine then you can go one step further and check if the generated CMake cache file content is correct (contains _DIR and the value is correct). If not, you can add some MESSAGE prints here:

As mentioned in Developer FAQ / Extensions / Can an extension depend on other extensions ?, the build system should build extensions with the expected <ExtensionName>_DIR variables.

This is implemented here:

https://github.com/Slicer/Slicer/blob/4edfa38fe94abf5b8c88ea336682e55efbf906a6/Extensions/CMake/SlicerBlockUploadExtension.cmake#L100-L105

and here:

https://github.com/Slicer/Slicer/blob/4edfa38fe94abf5b8c88ea336682e55efbf906a6/Extensions/CMake/SlicerBlockBuildPackageAndUploadExtension.cmake#L159-L164

mpReview extension has a dependency to ‘SlicerDevelopmentToolbox’

Looking at the description file confirms that.

GelDosimetry dependencies - See Automatic test for extension cannot find dependency on factory · Issue #4472 · Slicer/Slicer · GitHub

As we can see in the CMakeLists.txt, the dependency is specified as SlicerRT

But looking at the CMakeCache.txt of both extension, the path is not properly set:

kitware@factory-south-ubuntu:~/Dashboards/Nightly/S-0-E-b/mpReview-build$ cat CMakeCache.txt | grep SlicerDevelopmentToolbox_DIR
SlicerDevelopmentToolbox_DIR:PATH=SlicerDevelopmentToolbox_DIR-NOTFOUND
kitware@factory-south-ubuntu:~/Dashboards/Nightly/S-0-E-b/GelDosimetryAnalysis-build$ cat CMakeCache.txt | grep SlicerRT_DIR
SlicerRT_DIR:PATH=SlicerRT_DIR-NOTFOUND

There is definitively a problem.

Also worth noting that the current test check that _DIR variables are effectively set:

The issue are the following:

I’ve fixed this already earlier today (https://github.com/Slicer/Slicer/commit/9d4634fada0d428e698de7c4a68d43db25d242ef).

1 Like

Also pushed a fix to SlicerRT (https://github.com/SlicerRt/SlicerRT/commit/eb5995488eeddcc9fb1a6be789dc63ac0e2448fa). SlicerRTConfig.cmake was generated too early (before any modules were configured), so no module paths were added to SlicerRT’s AdditionalLauncherSettings.ini file.