Re: Review Request 114885: Remove custom build types
--- 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 (updated) --- 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
On Jan. 7, 2014, 3:36 p.m., Stephen Kelly wrote: kde-modules/KDECompilerSettings.cmake, line 34 https://git.reviewboard.kde.org/r/114885/diff/1/?file=233187#file233187line34 add_compiler_export_flags() should not be used at all. http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=0f3a66673f set CMAKE_CXX_VISIBILITY_PRESET and CMAKE_VISIBILITY_INLINES_HIDDEN instead. This isn't really part of this patch; I only added that comment because I was confused by its presence at first. I can just omit this change and put in a separate RR to use CMAKE_VISIBILITY_INLINES_HIDDEN and CMAKE_CXX_VISIBILITY_PRESET. 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 OK. I guess the rest of the file should also use that, but that's out of the scope of this RR. - Alex --- 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
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. 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. - Alex --- 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
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. Stephen Kelly wrote: 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. Yep; I'll put that in another RR. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/#review46978 --- On Jan. 7, 2014, 4:30 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, 4:30 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/ --- (Updated Jan. 7, 2014, 4:30 p.m.) Review request for Build System, KDE Frameworks, David Faure, Kevin Ottens, and Stephen Kelly. Changes --- No longer add an unrelated comment. 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 (updated) - 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/#review46996 --- Ship it! Nice, this also fixes some inconsistencies between compilers. I tested in a pure qt5+cmake test (no ECM) that: * RelWithDebInfo already sets -DQT_NO_DEBUG -O2 -g -DNDEBUG. So this was indeed unnecessary by now. * Release sets -DQT_NO_DEBUG -O3 -DNDEBUG, which is even better (-O3 vs -O2) than what ECM was doing. * Debug sets -g. No -O2, which isn't expected in debug mode. - David Faure On Jan. 7, 2014, 4:30 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, 4:30 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/ --- (Updated Jan. 7, 2014, 7:13 p.m.) Status -- This change has been marked as submitted. 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114885: Remove custom build types
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/#review46997 --- This review has been submitted with commit 4068592ad9aa3f241027f6dbd6aff5f756671fd3 by Alex Merry to branch master. - Commit Hook On Jan. 7, 2014, 4:30 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, 4:30 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
