View Issue Details

IDProjectCategoryView StatusLast Update
0004595Slicer4Core: Building (CMake, Superbuild)public2018-10-13 04:11
ReporterinortonAssigned Tojcfr 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version 
Target VersionSlicer 4.9.0Fixed in VersionSlicer 4.9.0 
Summary0004595: multiple rules generate slicer (ninja error)
Description

The latest ninja versions set '-w dupbuild=error' by default, which makes this long-standing warning into an error by default:

ninja: error: build.ninja:74853: multiple rules generate Slicer [-w dupbuild=err]

The offending lines in Slicer-build/build.ninja are:

build Slicer: phony bin/Slicer.app/Contents/MacOS/Slicer
build SlicerApp: phony bin/Slicer.app/Contents/MacOS/Slicer

Of course it can be worked around by passing '-w dupbuild=warn' as in the past, but would be nice to build cleanly ootb (I'm using the Kitware ninja binaries w/ jobserver).

TagsNo tags attached.

Relationships

child of 0004605 resolvedjcfr Finalize CTKAppLauncher update 

Activities

jcfr

jcfr

2018-08-07 22:17

administrator   ~0015942

Last edited: 2018-08-07 22:18

View 3 revisions

Considering the following example build.ninja:

rule sort
  command = cat input.txt | sort > output.txt

build output.txt: sort input.txt 

build foo: phony output.txt
build bar: phony output.txt

and its associated input.txt:

5
2
4
3
1

Running any of the following command succeed:

rm -f output.txt && ninja -w dupbuild=err
rm -f output.txt && ninja foo -w dupbuild=err
rm -f output.txt && ninja bar -w dupbuild=err

This indicates that having these two phony rules may not be the issue:

build Slicer: phony bin/Slicer.app/Contents/MacOS/Slicer
build SlicerApp: phony bin/Slicer.app/Contents/MacOS/Slicer

This is confirmed by skipping the launcher configuration in the application:

diff --git a/Applications/SlicerApp/CMakeLists.txt b/Applications/SlicerApp/CMakeLists.txt
index 1acd8ec..dd535ae 100755
--- a/Applications/SlicerApp/CMakeLists.txt
+++ b/Applications/SlicerApp/CMakeLists.txt
@@ -56,7 +56,7 @@ slicerMacroBuildAppLibrary(
 # Configure launcher only for the main application
 set(extra_args)
 if(${PROJECT_NAME} STREQUAL ${Slicer_MAIN_PROJECT})
-  set(extra_args CONFIGURE_LAUNCHER)
+#  set(extra_args CONFIGURE_LAUNCHER)
 endif()

 set(APP_SRCS

In that case, we still have the same two phony rules but no complain from ninja.

inorton

inorton

2018-08-07 22:30

developer   ~0015943

Last edited: 2018-08-08 08:43

View 3 revisions

The second line is the one ninja identified in the error message. Here is some related discussion: https://github.com/ninja-build/ninja/issues/931

It's actually a bit trickier than just -wdupbuild=warn, because that flag doesn't automatically get threaded down to the sub-builds; there's a reason ninja devs don't like nested projects : (

I think export NINJAFLAGS=-wdupbuild=warn will work, but right now I can't access the computer where I started the build that way. doesn't work, had to build ninja locally and revert the change.

inorton

inorton

2018-08-08 08:44

developer   ~0015944

In that case, we still have the same two phony rules but no complain from ninja.

Do you think ninja is reporting the wrong line number? Or this is a CMake bug?

jcfr

jcfr

2018-08-08 14:51

administrator   ~0015947

This sample project allows to reproduce the problem https://gist.github.com/jcfr/7e342d8ff786b82ff81873a96f66a292#gistcomment-2673574

inorton

inorton

2018-08-08 17:58

developer   ~0015948

Last edited: 2018-08-08 21:33

View 2 revisions

Thanks. So my understanding is that ninja is correct here, but only one of the phony lines is the problem, because we really do have an alias duplicating a real target.

One possible work-around: invert the order. Make add_custom_target depend only on the CTKAppLauncher_EXECUTABLE. Then change the command to add_custom_command(TARGET Configure{...}Launcher POST_BUILD. I think the end result should be the same, and it seems to work, see diff in comment:

https://gist.github.com/jcfr/7e342d8ff786b82ff81873a96f66a292#gistcomment-2673734

[edit: posted wrong link above]

jcfr

jcfr

2018-08-09 08:17

administrator   ~0015949

Suggested workaround makes sense.

Here is also the fix I suggested for the Ninja generator: https://gitlab.kitware.com/jcfr/cmake/commit/00a94baa695bcf951c0b6b8ce48617655cf97f54

jcfr

jcfr

2018-08-09 11:58

administrator   ~0015950

MR submitted to upstream CMake. See https://gitlab.kitware.com/cmake/cmake/merge_requests/2276

I will implement the workaround in Slicer

also, we may have to add workaround in other place ... see https://github.com/Kitware/SlicerSALT/issues/44

inorton

inorton

2018-08-09 13:00

developer   ~0015951

Great. btw I guess the work-around is technically equivalent to just removing the add_custom_command and putting the COMMANDs in add_custom_target -- add_custom_target runs for every build, so the add_custom_command does too.

jcfr

jcfr

2018-08-09 19:11

administrator   ~0015954

Fix (along with test) merged into app launcher: https://github.com/commontk/AppLauncher/commit/445c93b4043e80ee8efc5c65a2b4d47322e34408

(Before doing a new a release of the launcher, gonna have to create a new static build tree of Qt so that QProcess::setInputChannelMode is available. See https://github.com/commontk/AppLauncher/pull/101#issuecomment-411918208)

jcfr

jcfr

2018-10-13 04:11

administrator   ~0016117

Fixed in r27469
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=27469

Issue History

Date Modified Username Field Change
2018-08-07 13:01 inorton New Issue
2018-08-07 13:01 inorton Status new => assigned
2018-08-07 13:01 inorton Assigned To => jcfr
2018-08-07 13:12 jcfr Target Version => Slicer 4.9.0
2018-08-07 22:17 jcfr Note Added: 0015942
2018-08-07 22:18 jcfr Note Edited: 0015942 View Revisions
2018-08-07 22:18 jcfr Note Edited: 0015942 View Revisions
2018-08-07 22:30 inorton Note Added: 0015943
2018-08-08 08:41 inorton Note Edited: 0015943 View Revisions
2018-08-08 08:43 inorton Note Edited: 0015943 View Revisions
2018-08-08 08:44 inorton Note Added: 0015944
2018-08-08 14:51 jcfr Note Added: 0015947
2018-08-08 17:58 inorton Note Added: 0015948
2018-08-08 21:33 inorton Note Edited: 0015948 View Revisions
2018-08-09 08:17 jcfr Note Added: 0015949
2018-08-09 11:58 jcfr Note Added: 0015950
2018-08-09 13:00 inorton Note Added: 0015951
2018-08-09 19:11 jcfr Note Added: 0015954
2018-09-11 14:08 jcfr Relationship added child of 0004605
2018-10-13 04:11 jcfr Status assigned => resolved
2018-10-13 04:11 jcfr Resolution open => fixed
2018-10-13 04:11 jcfr Fixed in Version => Slicer 4.9.0
2018-10-13 04:11 jcfr Note Added: 0016117