Page MenuHomePhabricator

Build is broken when using Qt5 with OpenViewCore and QmlItems
Closed, ResolvedPublic

Description

Looks like these modules didn't get upgraded when the MITK_CREATE_MODULE build system changes were made.

Event Timeline

User zelzer has pushed new remote branch:

bug-17816-Build-is-broken-when-using-Qt5

[5af227]: Merge branch 'bug-17816-Build-is-broken-when-using-Qt5'

Merged commits:

2014-05-28 14:58:36 Sascha Zelzer [b0c66e]
Move DESIRED_QT_VERSION to the top.


2014-05-28 14:58:15 Sascha Zelzer [5966b8]
Added missing Qxt sources.


2014-05-28 01:54:08 Sascha Zelzer [27a453]
Fix CMake variable containing target names for modules and test drivers.


2014-05-28 01:53:06 Sascha Zelzer [cae512]
Fixed VTK Qt5 configuration and enabled Qwt and Qxt Qt5 builds.


2014-05-28 01:51:44 Sascha Zelzer [7fc8f2]
Use CMAKE_PREFIX_PATH instead of looking for qmake when using Qt5.

Bumped minimum CMake version for Qt5.


2014-05-23 17:53:21 Taylor Braun-Jones [c9cb63]
Updates for constructor changes in Mouse*Event classes

Signed-off-by: Taylor Braun-Jones <taylor@braun-jones.org>


2014-05-23 17:50:59 Taylor Braun-Jones [82105a]
Fix missing header

Signed-off-by: Taylor Braun-Jones <taylor@braun-jones.org>


2014-05-23 16:59:50 Taylor Braun-Jones [4e6e5c]
Fix broken symbol exports for OpenViewCore and QmlItems modules

Signed-off-by: Taylor Braun-Jones <taylor@braun-jones.org>

No problem. Thanks for merging and finishing the work! I have a couple questions:

(1) What feature does cmake 2.8.12 bring that is needed for Qt5 builds? The Qt cmake documentation lists 2.8.11 as the recommended minimum[1] in order to use the target_link_libraries command, which is convenient on RHEL6 since EPEL6 provides a cmake28 package which is at 2.8.11.2.

(2) On RHEL6, in order to support concurrent Qt4/Qt5 installations qmake is installed as qmake-qt4 and qmake-qt5. For Qt5 builds, to make the build configuration simpler, can we search for qmake-qt5 first and fallback to qmake? Something like this at SuperBuild.cmake:48:

find_program(QT_QMAKE_EXECUTABLE NAMES qmake-qt5 qmake)

In fact, is it even necessary to have both a QT_QMAKE_EXECUTABLE user option and a DESIRED_QT_VERSION user option? Couldn't qmake be searched for very early on (I guess it would have to be the very first thing in the top level CMakeLists.txt now) using something like this:

find_program(QT_QMAKE_EXECUTABLE NAMES qmake-qt4 qmake qmake-qt5)

And then setting QT_VERSION based on the results of ${QT_QMAKE_EXECUTABLE} -query QT_VERSION.

[1] http://qt-project.org/doc/qt-5/cmake-manual.html

Never mind about the cmake 2.8.12 dependency - I understand now.

But there is one more missing fix needed in the current master:

Modules/QmlItems/QmlMitkRenderWindowItem.cpp lines 59 and 139

Change Geometry2DDataMapper2D to PlaneGeometryDataMapper2D

(In reply to Taylor Braun-Jones from comment #6)

Never mind about the cmake 2.8.12 dependency - I understand now.

But there is one more missing fix needed in the current master:

Modules/QmlItems/QmlMitkRenderWindowItem.cpp lines 59 and 139

Change Geometry2DDataMapper2D to PlaneGeometryDataMapper2D

Thanks! We still need to get our continuous integration client for Qt5 up and running...

Thanks for the input. I still want to understand the use case better.

(In reply to Taylor Braun-Jones from comment #5)

(2) On RHEL6, in order to support concurrent Qt4/Qt5 installations qmake is
installed as qmake-qt4 and qmake-qt5. For Qt5 builds, to make the build
configuration simpler, can we search for qmake-qt5 first and fallback to
qmake? Something like this at SuperBuild.cmake:48:

find_program(QT_QMAKE_EXECUTABLE NAMES qmake-qt5 qmake)

In fact, is it even necessary to have both a QT_QMAKE_EXECUTABLE user option
and a DESIRED_QT_VERSION user option? Couldn't qmake be searched for very
early on (I guess it would have to be the very first thing in the top level
CMakeLists.txt now) using something like this:

find_program(QT_QMAKE_EXECUTABLE NAMES qmake-qt4 qmake qmake-qt5)

And then setting QT_VERSION based on the results of `${QT_QMAKE_EXECUTABLE}
-query QT_VERSION`.

In the Qt4 case, passing QT_QMAKE_EXECUTABLE was the standard solution to tell projects about a specific Qt4 installation. With Qt5 this seems to have changed. Qt5 installations are now located in CMake via an entry in CMAKE_PREFIX_PATH so using QT_QMAKE_EXECUTABLE doesn't seeem to be necessary for Qt5 any more.

So for MITK I thought the initial piece of information for the Qt4/5 decision is the DESIRED_QT_VERSION variable. If it is "4", find_package(Qt4) is used, which uses QT_QMAKE_EXECUTABLE if set. As far as I know, this used to work for RHEL6 too (probably depending on the version of CMake and the FindQt4.cmake script - if it includes the qmake-qt4 name or not).

If DESIRED_QT_VERSION is "5", the find_package(Qt5) call relies on the standard look-up paths and on CMAKE_PREFIX_PATH (but not on QT_QMAKE_EXECUTABLE) to find a Qt5 installation. I didn't test this with RHEL6 yet, but I assumed that the CMake scripts shipped with Qt5 know about the correctly named qmake executable internally anyway.

I know that we are still passing both QT_QMAKE_EXECUTABLE and CMAKE_PREFIX_PATH in both the Qt4 and Qt5 cases to external projects in the MITK superbuild. I would suggest to remove all QT_QMAKE_EXECUTABLE occurrences in case of Qt5 usage.

Do you think this will work for RHEL6? So re-iterating: we wouldn't be looking for a qmake executable at all in the Qt5 case, just relying on CMAKE_PREFIX_PATH and assuming the FindQt5(Core).cmake script does the right thing.

Thanks! With that explanation it all makes sense. I was coming from the perspective making the Qt build configuration work the same regardless of Qt4 or Qt5 (using QT_QMAKE_EXECUTABLE), but you're right that it doesn't make sense. And anyway, at some point (probably in the distant future?) MITK will drop Qt4 support and only the (better) Qt5 configuration will be needed.

In the meantime, I think the BuildInstructions.dox page should include an explanation of the Qt4 vs Qt5 configuration options, basically like the explanation you just gave me.

[dec978]: COMP: Merge branch 'bug-17816-Build-is-broken-when-using-Qt5'

Merged commits:

2014-06-10 15:48:45 Sascha Zelzer [0b7332]
Disable Qxt SSL support in case of Qt < 4.8.

User zelzer has pushed new remote branch:

bug-17816-add-build-documentation-for-qt-versions

[18c52c]: Merge branch 'bug-17816-add-build-documentation-for-qt-versions'

Merged commits:

2014-10-28 21:25:53 Sascha Zelzer [22a360]
Added build documentation about Qt4 and Qt5.