View Issue Details

IDProjectCategoryView StatusLast Update
0004247Slicer4Core: Extensionspublic2019-01-18 00:39
ReporterpinterAssigned Tojcfr 
PriorityhighSeveritymajorReproducibilityN/A
Status resolvedResolutionreopened 
PlatformWindowsOSOS Version
Product VersionSlicer 4.5.0-1 
Target VersionSlicer 4.10.2Fixed in VersionSlicer 4.7.0 
Summary0004247: Extensions do not get built on Windows if any test in the dependent extensions fail
Description

Which is undesired, because if an extension has 60 tests and one fails (may it be a widget test that fails because the module widget is 5 pixels wider than the set threshold), it does not mean it's not operational, and it doesn't make sense not to even start building the extensions that depend on it.

The extensions that motivated this issue are GelDosimetry and FilmDosimetry, both of which depend on SlicerRT. Sometimes one of the lengthy python self tests fail, or a widget test due to width.

TagsNo tags attached.

Activities

lassoan

lassoan

2016-09-11 10:42

developer   ~0014088

In Extensions\CMake\SlicerBlockBuildPackageAndUploadExtension.cmake this line causes the target build step to fail:

ctest_test(
    BUILD ${EXTENSION_SUPERBUILD_BINARY_DIR}/${EXTENSION_BUILD_SUBDIRECTORY}
    PARALLEL_LEVEL ${CTEST_PARALLEL_LEVEL})

Build step (in which ctest.exe returns with error code if ctest_test finds that a test fails):

...
"c:\Program Files\CMake\bin\ctest.exe" -C $(Configuration) -DCTEST_BUILD_CONFIGURATION:STRING=$(Configuration) -DCTEST_MODEL:STRING=Experimental -DSCRIPT_ARGS_FILE:FILEPATH=C:/D/N/E-0/SlicerRT-upload-command-args.cmake -S C:/D/N/Slicer-0/Extensions/CMake/SlicerBlockBuildPackageAndUploadExtension.cmake -VV
...

How ctest.exe knows that ctest_test failed (from parsing XML output file)? How it could be prevented to return with error code?

pinter

pinter

2016-09-13 10:45

developer   ~0014093

For reference: SlicerRT ticket created for the workaround that needs to be applied due to this issue
https://app.assembla.com/spaces/slicerrt/tickets/838-failing-slicerrt-tests-cause-dependent-extensions-not-to-build/details

jcfr

jcfr

2016-10-13 00:05

administrator   ~0014194

This should be fixed by

http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=25440
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=25439
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=25438

For future reference the associated github topic as:

https://github.com/Slicer/Slicer/pull/605

pinter

pinter

2016-10-16 10:30

developer   ~0014197

I re-enabled the failing SlicerRT tests to see if this change fixes the issue and we get dependent extensions to build even with the failing tests.

Unfortunately the windows build for dependent tests still does not start if any test fails.

pinter

pinter

2016-12-23 16:57

developer   ~0014282

I'm increasing the priority of this issue, as no progress has been made over the lsat four months, and it considerably decreases the usability of the extension dependency mechanism.

jcfr

jcfr

2016-12-23 18:32

administrator   ~0014283

Thanks for the update.

I wonder how different is the SlicerRt extension configuration from the current tests.

The extension build system tests are added here:

https://github.com/Slicer/Slicer/blob/d7f501e925831b48393214c81a1533d316a810dd/Extensions/CMake/Testing/CMakeLists.txt#L65-L79

And for all test cases, extension B and C depend on extension A which has one failing test.
See https://github.com/Slicer/Slicer/blob/d7f501e925831b48393214c81a1533d316a810dd/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py#L283-L289

A mock web server also confirm that all CDash and Midas queries are submitted as expected.

Is SlicerRt test crashing/hanging forever and preventing the dependent extensions from being built ?

pinter

pinter

2017-01-05 11:19

developer   ~0014285

Thanks for investigating! Okay, so the issue is not so trivial and seems SlicerRT specific.
Andras also wanted to create a minimal test case, but it worked for him as well, and we didn't know how to proceed. There must be something in how SlicerRT is configured, I agree.

I don't think that the any test hangs or crashes. If I have a module widget generic test fail because the module is wider than the threshold it's checked against, the dependent extensions don't build.

jcfr

jcfr

2017-08-10 12:56

administrator   ~0015034

See https://github.com/Slicer/Slicer/pull/768

jcfr

jcfr

2017-08-10 14:59

administrator   ~0015041

This should be fixed in 26223
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=26223

pinter

pinter

2017-08-21 10:36

developer   ~0015066

Unfortunately nothing changed in this manner. I re-enabled the SlicerRT tests that failed or were prone to failing, and the dependent extension builds do not start on Windows, for example
http://slicer.cdash.org/index.php?project=Slicer4&filtercount=1&showfilters=1&field1=label&compare1=63&value1=GelDosimetryAnalysis

jcfr

jcfr

2017-08-22 23:29

administrator   ~0015069

I believe this has been fixed in r26291
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=26291

Looking at the dashboard of August 22nd, the GelDosimetry is configured, built and tested.

See http://slicer.cdash.org/index.php?project=Slicer4&date=2017-08-22&filtercount=1&showfilters=1&field1=label&compare1=63&value1=GelDosimetryAnalysis

lassoan

lassoan

2017-08-23 09:36

developer   ~0015072

This is great! Thanks a lot for fixing this.

pinter

pinter

2017-11-16 09:34

developer   ~0015406

Sorry I just saw this update somehow. I confirm that the dependent extensions build. Thanks a lot for the fix! Closing the issue.

pinter

pinter

2017-11-21 10:23

developer   ~0015425

Last edited: 2017-11-28 15:14

View 2 revisions

I have to reopen this issue. I re-enabled the tests that fail (generic tests with module dependencies) or tend to fail (very long python self test), and the dependent extensions son't build on Windows. See this dashboard filtered to SlicerRT and its dependent extensions. You'll see that SlicerRT builds on Windows, but none of the dependencies build on Windows.
http://slicer.cdash.org/index.php?project=Slicer4&date=2017-11-21&filtercount=4&showfilters=1&filtercombine=or&field1=label&compare1=63&value1=SlicerRT&field2=label&compare2=63&value2=GelDosimetry&field3=label&compare3=63&value3=FilmDosimetry&field4=label&compare4=63&value4=SegmentRegistration

pinter

pinter

2018-02-06 11:39

developer   ~0015509

Another occurrence of this issue:
http://slicer.cdash.org/index.php?project=Slicer4&display=project&filtercombine=and&filtercombine=and&filtercombine=or&filtercombine=or&filtercombine=or&filtercombine=or&filtercombine=or&filtercombine=or&filtercount=4&showfilters=1&filtercombine=or&field1=label&compare1=63&value1=SlicerRT&field2=label&compare2=63&value2=GelDosimetry&field3=label&compare3=63&value3=FilmDosimetry&field4=label&compare4=63&value4=SegmentRegistration

lassoan

lassoan

2018-12-09 09:43

developer   ~0016215

I confirm that this issue still exists today, in latest master version of Slicer on (SlicerPreview dashboard)!

MatlabBridge extension did not get build on Windows, it was simply missing from the dashboard (but it was built successfully on linux and mac):

http://slicer.cdash.org/index.php?project=SlicerPreview&date=2018-12-08&filtercount=2&showfilters=1&filtercombine=or&field1=buildname&compare1=63&value1=SlicerOpenIGTLink&field2=buildname&compare2=63&value2=MatlabBridge

Then, we disabled failing tests of SlicerOpenIGTLink extension (no other change was made in Slicer core or any extensions) and as a result, MatlabBridge got built successfully and appeared on the dashboard:

http://slicer.cdash.org/index.php?project=SlicerPreview&date=2018-12-09&filtercount=2&showfilters=1&filtercombine=or&field1=buildname&compare1=63&value1=SlicerOpenIGTLink&field2=buildname&compare2=63&value2=MatlabBridge

pinter

pinter

2018-12-09 10:14

developer   ~0016216

True. I thought this was fixed but indeed, this week a test failed in SlicerRT for a day and that day the dependencies didn't build:
http://slicer.cdash.org/index.php?project=SlicerPreview&date=2018-12-05&filtercount=4&showfilters=1&filtercombine=or&field1=label&compare1=63&value1=SlicerRT&field2=label&compare2=63&value2=GelDosimetry&field3=label&compare3=63&value3=FilmDosimetry&field4=label&compare4=63&value4=SegmentRegistration

sjh267

sjh267

2018-12-18 10:21

developer   ~0016228

A similar error is happening with the Sequences extension, without any test failures @jcfr. Have retargeted this to 4.10.1

Sunderlandkyl

Sunderlandkyl

2018-12-18 10:31

reporter   ~0016229

It's worth noting that none of the tests in Sequences are failing, although there is an error message in one (http://slicer.cdash.org/testDetails.php?test=9369351&build=1465199).

Extensions that are dependent on Sequences are still being built by factory.perklab (http://slicer.cdash.org/buildSummary.php?buildid=1464935, http://slicer.cdash.org/buildSummary.php?buildid=1464936)

Sunderlandkyl

Sunderlandkyl

2018-12-19 10:19

reporter   ~0016230

I changed the error message to debug in Sequences (https://github.com/SlicerRt/Sequences/commit/9b07bf1307e1667f5cc904e8809c2b0f1feb0c32), and now all dependent extensions are building again.

lassoan

lassoan

2019-01-15 00:20

developer   ~0016238

Another occurrence (where we had to disable a test to allow dependent extensions to be built):
https://github.com/SlicerIGT/SlicerIGT/pull/189

jcfr

jcfr

2019-01-15 08:56

administrator   ~0016239

Thanks for the note, I will have a look before cutting the 4.10.1 release.

jcfr

jcfr

2019-01-16 15:58

administrator   ~0016242

Last edited: 2019-01-16 15:58

View 2 revisions

For SlicerIGT test, I noticed that the dashboard had a pop asking to add a firewall rule for one of the test. I suspect this is what caused the blockage.

jcfr

jcfr

2019-01-16 16:01

administrator   ~0016243

Last edited: 2019-01-16 16:01

View 2 revisions

@pinter @arankin @lassoan Is it possible to isolate the exact test failure leading to the dependent extensions not being built ?

lassoan

lassoan

2019-01-16 17:48

developer   ~0016246

The most specific that we know so far is that if a test logs an error message (vtkErrorMacro and as far as I remember, even vtkWarningMacro) then dependent extensions are not even attempted to be built. Changing it to debug message (vtkDebugMacro) fixes the issue.

jcfr

jcfr

2019-01-17 19:27

administrator   ~0016248

I was able to update the ExtensionBuildSystem test to reproduce the problem. Now working on a fix.
See https://github.com/Slicer/Slicer/compare/master...jcfr:4247-extension-build-system-windows-dependent-extension

jcfr

jcfr

2019-01-18 00:39

administrator   ~0016249

This should now be fixed in r27944
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=27944

To prevent extension test displaying string "ERROR:" from being considered
as build failure and causing the overall build of all extensions to fail,
this commit updates the execution of the build wrapper script to ensure
its standard output and error streams are logged to file.

Note that this was only observed when using Visual Studio generator on
Windows because in that case, there are no CTest launcher.
See https://cmake.org/cmake/help/latest/module/CTestUseLaunchers.html

This commit also improves the extension build system tests to avoid future
regressions.

Related Changesets

Import 2017-06-07 23:51:09: master c226f731

2016-10-12 23:50:38

jcfr

Details Diff
BUG: Extension build system: Ignore extension CTest return code. Fixes 0004247

This commit ensures each "EXTENSION_UPLOAD_COMMAND" responsible to configure,
build, test and package any given extension is completely "sandboxed" by
using a wrapper script.

This is required because simply setting the RETURN_VALUE parameter
to the ctest_test() command is not enough to avoid the script from
exiting with error code.

#
# The following wrapper script is required to workaround issue 0004247
# and avoid the all extension build from failing if a test of
# extension fail.
#
# Note that as soon as CMake >= 3.6.7 is released, it should be possible
# to remove the wrapper script and simply specify CAPTURE_CMAKE_ERROR
# ctest_test option.
#
# See https://cmake.org/cmake/help/v3.7/command/ctest_test.html
#

Tested-by: Isaiah Norton <inorton@bwh.harvard.edu>
Tested-by: Johan Andruejol <johan.andruejol@kitware.com>
Tested-by: Nicole Aucoin <nicole@bwh.harvard.edu>
Tested-by: Steve Pieper <pieper@isomics.com>

Reported-by: Csaba Pinter <csaba.pinter@queensu.ca>
Reported-by: Andras Lasso <lasso@cs.queensu.ca>

Thanks: All of the above

git-svn-id: http://svn.slicer.org/Slicer4/trunk@25439 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Extensions/CMake/SlicerBlockBuildPackageAndUploadExtensions.cmake Diff File

Import 2017-06-07 23:51:09: master 41df9c1a

2016-10-12 23:50:40

jcfr

Details Diff
BUG: Extension build system: Fix use of wrapper script on windows. Fixes 0004247

This commit ensures the build configuration is passed to the wrapped
command.

It fixes error like this one:


8: SetMakeCommand:C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe --build . --config "$(Configuration)"
8:
8: SetCTestConfiguration:MakeCommand:C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe --build . --config "$(Configuration)"
8:
8: Build project
8:
8: MakeCommand:C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe --build . --config "$(Configuration)"
8: Run command: "C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe" "--build" "." "--config" "$(Configuration)"
8:
8: Microsoft (R) Build Engine version 12.0.40629.0
8: [Microsoft .NET Framework, version 4.0.30319.34209]
8: Copyright (C) Microsoft Corporation. All rights reserved.
8: Build started 10/12/2016 10:57:46 PM.
8:
8: Project "C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ALL_BUILD.vcxproj" on node 1 (default targets).
8: Project "C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ALL_BUILD.vcxproj" (1) is building "C:\D\N\Slic
er-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ZERO_CHECK.vcxproj" (2) on node 1 (default targets).
8:
8:
8: C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V120\Microsoft.Cpp.Platform.targets(61,5): error MSB8013: This project doesn't contain the Configuration an
d Platform combination of $(Configuration)|x64. [C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ZERO_CHECK.v
cxproj] [C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA.vcxproj]
8:

git-svn-id: http://svn.slicer.org/Slicer4/trunk@25440 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Extensions/CMake/SlicerBlockBuildPackageAndUploadExtensions.cmake Diff File
mod - Extensions/CMake/SlicerBlockUploadExtension.cmake Diff File

Issue History

Date Modified Username Field Change
2016-08-24 11:38 pinter New Issue
2016-08-24 11:38 pinter Status new => assigned
2016-08-24 11:38 pinter Assigned To => jcfr
2016-09-11 10:42 lassoan Note Added: 0014088
2016-09-13 10:45 pinter Note Added: 0014093
2016-10-12 11:45 jcfr Target Version Slicer 4.5.1 => Slicer 4.6.0
2016-10-13 00:05 jcfr Note Added: 0014194
2016-10-13 00:05 jcfr Status assigned => resolved
2016-10-13 00:05 jcfr Fixed in Version => Slicer 4.6.0
2016-10-13 00:05 jcfr Resolution open => fixed
2016-10-13 01:24 jcfr Product Version Slicer 4.5.1 => Slicer 4.5.0-1
2016-10-16 10:30 pinter Note Added: 0014197
2016-10-16 10:30 pinter Status resolved => confirmed
2016-12-23 16:57 pinter Note Added: 0014282
2016-12-23 16:57 pinter Priority normal => high
2016-12-23 18:32 jcfr Note Added: 0014283
2017-01-05 11:19 pinter Note Added: 0014285
2017-06-10 08:51 jcfr Changeset attached => Slicer master 41df9c1a
2017-06-10 08:51 jcfr Changeset attached => Slicer master c226f731
2017-08-10 12:56 jcfr Note Added: 0015034
2017-08-10 14:59 jcfr Status confirmed => resolved
2017-08-10 14:59 jcfr Fixed in Version Slicer 4.6.0 => Slicer 4.7.0
2017-08-10 14:59 jcfr Note Added: 0015041
2017-08-21 10:36 pinter Status resolved => assigned
2017-08-21 10:36 pinter Note Added: 0015066
2017-08-21 10:37 pinter Summary Extensions do not get built if any test in the dependent extensions fail => Extensions do not get built on Windows if any test in the dependent extensions fail
2017-08-22 23:29 jcfr Status assigned => resolved
2017-08-22 23:29 jcfr Note Added: 0015069
2017-08-23 09:36 lassoan Note Added: 0015072
2017-11-16 09:34 pinter Status resolved => closed
2017-11-16 09:34 pinter Note Added: 0015406
2017-11-21 10:23 pinter Status closed => feedback
2017-11-21 10:23 pinter Resolution fixed => reopened
2017-11-21 10:23 pinter Note Added: 0015425
2017-11-28 15:14 pinter Note Edited: 0015425 View Revisions
2018-02-06 11:39 pinter Note Added: 0015509
2018-02-06 11:39 pinter Status feedback => assigned
2018-12-09 09:43 lassoan Note Added: 0016215
2018-12-09 10:14 pinter Note Added: 0016216
2018-12-18 10:20 sjh267 Target Version Slicer 4.6.0 => Slicer 4.10.1
2018-12-18 10:21 sjh267 Note Added: 0016228
2018-12-18 10:31 Sunderlandkyl Note Added: 0016229
2018-12-19 10:19 Sunderlandkyl Note Added: 0016230
2019-01-15 00:20 lassoan Note Added: 0016238
2019-01-15 08:56 jcfr Note Added: 0016239
2019-01-16 15:58 jcfr Note Added: 0016242
2019-01-16 15:58 jcfr Note Edited: 0016242 View Revisions
2019-01-16 16:01 jcfr Note Added: 0016243
2019-01-16 16:01 jcfr Note Edited: 0016243 View Revisions
2019-01-16 16:20 jcfr Target Version Slicer 4.10.1 => Slicer 4.10.2
2019-01-16 17:48 lassoan Note Added: 0016246
2019-01-17 19:27 jcfr Note Added: 0016248
2019-01-18 00:39 jcfr Status assigned => resolved
2019-01-18 00:39 jcfr Note Added: 0016249