<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111769/">http://git.reviewboard.kde.org/r/111769/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks, looks good. Two suggestions:
- Use the global CMAKE_AUTOMOC property instead of target specific ones
- Only enable CMAKE_AUTOMOC for Qt5
Using the global automoc property works around a problem with kde4_add_plugin which also fiddles around with automoc and only respects the global property. With target specific automoc plugins do not build in the KDE version, but cmake generates an error. Another approach would be to get rid of kde4_add_plugin which I have in mind as well, but let's keep that separate of this patch. A nice side effect of using CMAKE_AUTOMOC is that the marble_cmake_automoc macro is not needed.
I'd prefer using cmake builtin automoc only for Qt5 to avoid the sheer mass of warnings which occur because of us using cmake automotoc relaxed mode. Once we do not need to support older cmake versions anymore (which don't have builtin automoc), we can rename our moc inclusion scheme to the cmake builtin style and get rid of relaxed mode.
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ee8e667..e18b726 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -271,6 +271,12 @@ IF ( NOT QT_QTDECLARATIVE_FOUND )
include_directories(${QT_QTDECLARATIVE_INCLUDE_DIR})
ENDIF()
+if( NOT ${CMAKE_VERSION} STRLESS "2.8" AND NOT QT4_FOUND)
+ SET(CMAKE_AUTOMOC TRUE)
+ SET(CMAKE_AUTOMOC_RELAXED_MODE TRUE)
+endif()
+
+
if(QTONLY)
# add a flag to be able to distinguish between qt
# and kde mode in the sources
diff --git a/MarbleMacros.cmake b/MarbleMacros.cmake
index 5a0aedc..4c3c121 100644
--- a/MarbleMacros.cmake
+++ b/MarbleMacros.cmake
@@ -19,6 +19,15 @@ else()
endmacro()
endif()
+
+macro( marble_qt4_automoc )
+ if( ${CMAKE_VERSION} STRLESS "2.8" OR QT4_FOUND)
+ qt4_automoc( ${ARGN} )
+ else()
+ # Just ignore it
+ endif()
+endmacro()
+
# the place to put in common cmake macros
# this is needed to minimize the amount of errors to do
macro( marble_add_plugin _target_name )</pre>
<br />
<p>- Dennis</p>
<br />
<p>On August 4th, 2013, 2:33 p.m. UTC, Michael Zanetti wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Michael Zanetti.</div>
<p style="color: grey;"><i>Updated Aug. 4, 2013, 2:33 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This commit switches from qt4_automoc to cmake automoc.
This is one more step on the way to make Marble work with Qt5.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested on Ubuntu with Qt4 and -DQTONLY=1</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>MarbleMacros.cmake <span style="color: grey">(5a0aedc)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(b2a3e09)</span></li>
<li>src/lib/CMakeLists.txt <span style="color: grey">(16c7fab)</span></li>
<li>src/plugins/declarative/CMakeLists.txt <span style="color: grey">(300ebed)</span></li>
<li>src/plugins/positionprovider/geoclue/GeoCute/CMakeLists.txt <span style="color: grey">(abde4f2)</span></li>
<li>src/plugins/render/aprs/CMakeLists.txt <span style="color: grey">(4896a8e)</span></li>
<li>src/plugins/render/earthquake/CMakeLists.txt <span style="color: grey">(6a4704e)</span></li>
<li>src/plugins/render/elevationprofilefloatitem/CMakeLists.txt <span style="color: grey">(872e5e1)</span></li>
<li>src/plugins/render/elevationprofilemarker/CMakeLists.txt <span style="color: grey">(bbe043e)</span></li>
<li>src/plugins/render/fileview/CMakeLists.txt <span style="color: grey">(adfa961)</span></li>
<li>src/plugins/render/foursquare/CMakeLists.txt <span style="color: grey">(0b62efd)</span></li>
<li>src/plugins/render/license/CMakeLists.txt <span style="color: grey">(90c0097)</span></li>
<li>src/plugins/render/navigation/CMakeLists.txt <span style="color: grey">(b0597aa)</span></li>
<li>src/plugins/render/opendesktop/CMakeLists.txt <span style="color: grey">(50da329)</span></li>
<li>src/qt-components/marble-touch/CMakeLists.txt <span style="color: grey">(52940a6)</span></li>
<li>src/routing-instructions/CMakeLists.txt <span style="color: grey">(c1e6386)</span></li>
<li>src/tilecreator/CMakeLists.txt <span style="color: grey">(f1eed44)</span></li>
<li>tools/tilecreator-srtm2/CMakeLists.txt <span style="color: grey">(8088187)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111769/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>