Build of the extensions index is blocking due to extension waiting for user input

[jcfr]

Summary

After connecting to the windows factory using VNC, I was able to observe that the ExtraMarkups extension was showing a popup dialog and expecting the user to click.

Steps to reproduce

NA

Expected behavior

No popup should be displayed.

Environment

• Observed in the context of the Preview build on Windows (overload factory) associated with fbc2ede

[jcfr]

This may be related to the following:

https://github.com/vmtk/SlicerExtension-VMTK/blob/5c31f18ba995fa97494b302e23f8a8d16cdc4f8d/StenosisMeasurement3D/qSlicerStenosisMeasurement3DModuleWidget.cxx#L101-L102

introduced through the following pull-request:

• Add 'Stenosis Measurement 3D' module vmtk/SlicerExtension-VMTK#73

[jcfr]

The function qSlicerStenosisMeasurement3DModuleWidget::installExtensionFromServer should be improved to skip the popup when running test mode.

https://github.com/vmtk/SlicerExtension-VMTK/blob/5c31f18ba995fa97494b302e23f8a8d16cdc4f8d/StenosisMeasurement3D/qSlicerStenosisMeasurement3DModuleWidget.cxx#L416-L466

Ideally, the function should be contributed to Slicer itself.

[jcfr]

cc: @chir-set @lassoan

[jcfr]

Once the function installExtensionFromServer is added to Slice core, the following should be updated:

Project	URL
SlicerExtension-VMTK	https://github.com/search?q=repo%3Avmtk%2FSlicerExtension-VMTK%20installExtensionFromServer&type=code
SlicerMorph	https://github.com/SlicerMorph/SlicerMorph/blob/c86b3dd507ab0df9ad0fef721b1da1d69cc37d43/SegmentEndocranium/SegmentEndocranium.py#L51-L61
slicer-notebook	https://github.com/Slicer/SlicerDocker/blob/4fc3be96fd28cf97677650a76ac83f209efe834b/slicer-notebook/install.sh
Slicer documentation	https://github.com/Punzo/Slicer/blob/08fe346009554f82e63e7d7aaaaec06c16f0a233/Docs/developer_guide/script_repository/gui.md?plain=1#L1137-L1147
DMRIInstall	https://github.com/Punzo/Slicer/blob/08fe346009554f82e63e7d7aaaaec06c16f0a233/Modules/Scripted/DMRIInstall/DMRIInstall.py#L111-L125
kaapana	https://github.com/kaapana/kaapana/blob/0a3b2e14f64510671c9c9dcb7ef73853002d8940/services/applications/slicer-workbench/docker/files/install-slicer-extension.py#L8

[jcfr]

Related discourse post(s):

• https://discourse.slicer.org/t/programatically-install-extensions/19241

[chir-set]

Ideally, the function should be contributed to Slicer itself.

Thank you for showing this issue.

-2. OK, contributing the function to Slicer is not a problem, and will be done secondarily.

-1. Fixing the popup showing itself seems immediately important. Referring to this resolution, would exiting the function at the very beginning by checking

qSlicerApplication::application()->layoutManager() == nullptr

be an acceptable solution ?

[jcfr]

Instead you could add something like this:

  bool isTestingEnabled = qSlicerApplication::testAttribute(qSlicerCoreApplication::AA_EnableTesting);
  if (isTestingEnabled)
    {
    // Do not block the execution with popup windows if testing mode is enabled.
    return;
    }

[chir-set]

Instead you could add something like this:

OK.

As for 'StenosisMeasurement3D' module in SlicerVMTK, I think we could just remove that function from there to install 'ExtraMarkups', since that extension is now marked as dependency in SlicerVMTK.

Secondarily, I'll request addition of that function, corrected, in Slicer : where in Slicer ? In which generic library ?

[lassoan]

At build time (and so during all testing performed at build time), extension dependencies are not installed (regardless of you list them in the dependencies or not). Therefore, you must exclude all tests from running automatically during build that require other extensions. See more details in this issue: #6529

[lassoan]

Ideally, the function should be contributed to Slicer itself.

Most extensions are implemented in Python and our recommendation has been to follow the example in the script repository. However, the example does not include the user confirmation popup, so users may forget about making it non-interactive in testing mode.

To allow harmonizing the behavior between C++ and Python and between all extensions, I agree that it would be better to add a helper function to the extensions manager. We could start from the helper function that in VMTK. @chir-set would you like to submit a pull request to Slicer that adds the method you implemented?

[chir-set]

@chir-set would you like to submit a pull request to Slicer that adds the method you implemented?

I can, but it is not a problem at all if it's not me.

Can you check this PR for a final resolution of this issue ?

[lassoan]

I've realized why this is not needed ever for C++ extensions!

A C++ module is not even loaded if dependent DLLs are not found. Therefore, it cannot install missing extensions. Therefore, the helper function can be kept in Python. No need to add it to the C++ implementation.

[chir-set]

In fine, what should I do for this issue ?

[lassoan]

All you need to do is to remove qSlicerStenosisMeasurement3DModuleWidget::installExtensionFromServer from your module.

We can later consider adding a Python function to harmonize how extensions are installed in Python.

[chir-set]

remove qSlicerStenosisMeasurement3DModuleWidget::installExtensionFromServer

Yes, simplest solution.

[jcfr]

re: install from c++

If functionality provided by an extension at runtime (e.g markups, dicom plugin,..) are relevant for a workflow.

Installing them at runtime without a hard requirement at build time is sensible.

While it doesn't have to happen to address this specific issue, I still think we should add the function to the extension manager, it will be useful when adding a metadata like runtime_depends

[lassoan]

Until now the need for installing an extension in C++ has not come up, but I agree that in theory such use cases are possible (for example, a C++ module using a module implemented in Python).

[jcfr]

Update documentation to mention that extensions should not be installed as part of the tests.

This means that dialog asking user to install extensions should be conditional implementing
code like the following:

  bool isTestingEnabled = qSlicerApplication::testAttribute(qSlicerCoreApplication::AA_EnableTesting);
  if (isTestingEnabled)
    {
    // Do not block the execution with popup windows if testing mode is enabled.
    return;
    }

[chir-set]

Update documentation to mention that extensions should not be installed as part of the tests.

This means that dialog asking user to install extensions should be conditional implementing code like the following:

  bool isTestingEnabled = qSlicerApplication::testAttribute(qSlicerCoreApplication::AA_EnableTesting);
  if (isTestingEnabled)
    {
    //...

Hi @jcfr,

I'm abroad right now without any PC. I'll look into that in 2 weeks. cdash doesn't report a build failure for SlicerVMTK nor ExtraMarkups AFAIU.

[chir-set]

This means that dialog asking user to install extensions should be conditional implementing code like the following:

  bool isTestingEnabled = qSlicerApplication::testAttribute(qSlicerCoreApplication::AA_EnableTesting);

Ideally, the function should be contributed to Slicer itself.

Hi @jcfr ,

Is this patch what you meant ?

Regards.

[lassoan]

The application should not work very differently in testing mode. Skipping display of an information/confirmation popup is a minor behavior change, which should be fine. However, silently not installing dependencies of an extension would be a major behavior change, which should not be done just because we are in testing mode.

[chir-set]

silently not installing dependencies of an extension... should not be done

The patch now shows dialog boxes when not in testing mode, and always installs a requested extension.

[jcfr]

Can you check this PR for a final resolution of this issue ?

@chir-set Thanks for creating the branch, the corresponding changes have been reviewed and contributed in a dedicated pull request:

• ENH: Add extension model API for downloading and installing an extension #7145