[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 114885: Remove custom build types
From: "Stephen Kelly" <steveire () gmail ! com>
Date: 2014-01-07 16:30:23
Message-ID: 20140107163023.27045.67509 () probe ! kde ! org
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Jan. 7, 2014, 3:36 p.m., Stephen Kelly wrote:
> > kde-modules/KDECompilerSettings.cmake, line 25
> > <https://git.reviewboard.kde.org/r/114885/diff/1/?file=233187#file233187line25>
> >
> > Setting CMAKE_CXX_FLAGS is not 'modern cmake'. Prefer to use add_compile_options \
> > instead.
> > http://www.cmake.org/cmake/help/git-next/command/add_compile_options.html
>
> Alex Merry wrote:
> OK. I guess the rest of the file should also use that, but that's out of the scope \
> of this RR.
> Alex Merry wrote:
> Ah, it turns out that with KDE_ENABLE_EXCEPTIONS defined as it is, you cannot pass \
> it to add_compile_options (the Clang and GNU versions put quotes around two \
> distinct arguments, making them appear as a single argument, which the compiler \
> then complains about).
> So this is also something that should go in a separate review.
I don't know what particularly is in KDE_ENABLE_EXCEPTIONS, but to work with \
add_compile_options, it should be a list, not a string.
- Stephen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114885/#review46978
-----------------------------------------------------------
On Jan. 7, 2014, 3:22 p.m., Alex Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114885/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2014, 3:22 p.m.)
>
>
> Review request for Build System, KDE Frameworks, David Faure, Kevin Ottens, and \
> Stephen Kelly.
>
> Repository: extra-cmake-modules
>
>
> Description
> -------
>
> This is a cleaned-up version of https://git.reviewboard.kde.org/r/113805/ , with \
> documentation fixes. The discussion there appeared to end up being largely in \
> favour of this move.
> Obviously, this can only go in once TP1 is done.
>
>
> Remove custom build types
>
> KDECompilerSettings.cmake no longer alters CMake's built-in build types
> or adds its own. The "debug" build type therefore simply sets -g with
> no additional flags (rather than -O2 and, depending on the compiler,
> some no-inline/no-reorder flags as previously), the "release" build
> types no longer set -DQT_NO_DEBUG and the "debugfull", "profile" and
> "coverage" build types no longer exist.
>
> QT_NO_DEBUG is set by Qt's CMake scripts depending on the build type of
> Qt itself. "debugfull" mostly set -g3, allowing macro expansion when
> debugging; users can set this flag using environment variables if they
> wish. "RelWithDebugInfo" should be used instead of "profile" (according
> to dfaure); -fprofile-arcs and -ftest-coverage are easy enough to add to
> $CXX_FLAGS if they are required (formerly set by "profile" and
> "coverage").
>
>
> Diffs
> -----
>
> kde-modules/KDECompilerSettings.cmake 72824e166d03dcc2d089814dc121f08ba998974a
>
> Diff: https://git.reviewboard.kde.org/r/114885/diff/
>
>
> Testing
> -------
>
> Built kcoreaddons on linux with gcc. -DCMAKE_BUILD_TYPE=debugfull works, but does \
> not set -g. -DCMAKE_BUILD_TYPE=debug does set -g.
>
> Thanks,
>
> Alex Merry
>
>
[Attachment #5 (text/html)]
<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="https://git.reviewboard.kde.org/r/114885/">https://git.reviewboard.kde.org/r/114885/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On January 7th, 2014, 3:36 p.m. UTC, <b>Stephen \
Kelly</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/114885/diff/1/?file=233187#file233187line25" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kde-modules/KDECompilerSettings.cmake</a> <span style="font-weight: \
normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">9</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="c"># set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} \
${KDE_ENABLE_EXCEPTIONS}")</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Setting CMAKE_CXX_FLAGS \
is not 'modern cmake'. Prefer to use add_compile_options instead.
http://www.cmake.org/cmake/help/git-next/command/add_compile_options.html</pre>
</blockquote>
<p>On January 7th, 2014, 3:55 p.m. UTC, <b>Alex Merry</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">OK. I guess the rest of \
the file should also use that, but that's out of the scope of this RR.</pre> \
</blockquote>
<p>On January 7th, 2014, 4:28 p.m. UTC, <b>Alex Merry</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ah, it turns out that \
with KDE_ENABLE_EXCEPTIONS defined as it is, you cannot pass it to \
add_compile_options (the Clang and GNU versions put quotes around two distinct \
arguments, making them appear as a single argument, which the compiler then complains \
about).
So this is also something that should go in a separate review.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't \
know what particularly is in KDE_ENABLE_EXCEPTIONS, but to work with \
add_compile_options, it should be a list, not a string.</pre> <br />
<p>- Stephen</p>
<br />
<p>On January 7th, 2014, 3:22 p.m. UTC, Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://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 Build System, KDE Frameworks, David Faure, Kevin Ottens, and \
Stephen Kelly.</div> <div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated Jan. 7, 2014, 3:22 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
extra-cmake-modules
</div>
<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 is a cleaned-up version of \
https://git.reviewboard.kde.org/r/113805/ , with documentation fixes. The discussion \
there appeared to end up being largely in favour of this move.
Obviously, this can only go in once TP1 is done.
Remove custom build types
KDECompilerSettings.cmake no longer alters CMake's built-in build types
or adds its own. The "debug" build type therefore simply sets -g with
no additional flags (rather than -O2 and, depending on the compiler,
some no-inline/no-reorder flags as previously), the "release" build
types no longer set -DQT_NO_DEBUG and the "debugfull", "profile" \
and "coverage" build types no longer exist.
QT_NO_DEBUG is set by Qt's CMake scripts depending on the build type of
Qt itself. "debugfull" mostly set -g3, allowing macro expansion when
debugging; users can set this flag using environment variables if they
wish. "RelWithDebugInfo" should be used instead of "profile" \
(according to dfaure); -fprofile-arcs and -ftest-coverage are easy enough to add to
$CXX_FLAGS if they are required (formerly set by "profile" and
"coverage").</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;">Built kcoreaddons on linux with gcc. -DCMAKE_BUILD_TYPE=debugfull \
works, but does not set -g. -DCMAKE_BUILD_TYPE=debug does set -g.</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>kde-modules/KDECompilerSettings.cmake <span style="color: \
grey">(72824e166d03dcc2d089814dc121f08ba998974a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/114885/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic