<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/111938/">http://git.reviewboard.kde.org/r/111938/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 1st, 2013, 5:16 p.m. UTC, <b>David Faure</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;">Looks fine to me.</pre>
</blockquote>
<p>On September 5th, 2013, 6:49 p.m. UTC, <b>Ivan Romanov</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;">I don't agree with this patch. I didn't get any notification about this. So I very ask you before do any changes in cmake rules talk with me. It is important for me. </pre>
</blockquote>
<p>On September 6th, 2013, 9:30 a.m. UTC, <b>David Faure</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;">Holy... and here I was, thinking I was doing something good by reviewing patches for QCA, for lack of reviewers/maintainers.
Fine, I will definitely stay out of QCA completely, it's not like I ever wanted to be involved with it.
It's all yours now.
When objecting to a patch, you might want to give a more detailed technical argumentation than "I don't agree", BTW.
But I don't care anymore for that argumentation, that's a discussion between you and Alexander.
About notifications: you were in the CC for the request, according to reviewboard. You might want to check which emails reviewboard has for you, or your mail setup.</pre>
</blockquote>
<p>On September 6th, 2013, 4:36 p.m. UTC, <b>Ivan Romanov</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;">Bad ides was to change default values. Now Gentoo building broken also ... yestarday we was building QCA for Qt5 on Windows and couldn't understand why it was installing to "C:\Program Files\" and had -qt5 suffix.
So Alexander please revert
1. QCA_INSTALL_IN_QT_PREFIX default value
2. QCA_LIB_SUFFIX defalut value
3. QCA_INCLUDE_INSTALL_DIR and QCA_PRIVATE_INCLUDE_INSTALL_DIR in case when QCA_INSTALL_IN_QT_PREFIX == OFF.
Also what for LIB_INSTALL_DIR has different values for Qt4 and Qt5?
When I was writing this rules I want to reach the purposes of:
1. Easy installing when building manually.
2. Good adjustability to satisfy any needs.
David. Thanks for review and thanks for your care Qca.</pre>
</blockquote>
<p>On September 7th, 2013, 9:55 a.m. UTC, <b>Ivan Romanov</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;">Alexander, I discussed question about QCA_INSTALL_IN_QT_PREFIX with David. So, I agree to use QCA_INSTALL_IN_QT_PREFIX=ON only when CMAKE_INSTALL_PREFIX is not defined. Furthemore I want to use QCA_INSTALL_IN_QT_PREFIX as non-cache entry. It's mean that by default for installing will be used Qt prefix, if user defined CMAKE_INSTALL_PREFIX this folder will be used. If user want to install in /usr/local he must do 'cmake -DCMAKE_INSTALL_PREFIX=/usr/local'.
Yes, I do. I know that /usr/local is Unix tradition. And now I consciously break this. Package maintainers always use CMAKE_INSTALL_PREFIX, default path is important only for people how build for themself. If someone want to build for himself he will prefer Qt prefix. Anyway I very doubt that he want to install in /usr/local, more likely he will use CMAKE_INSTALL_PREFIX with other path. So /usr/local is the most rarely case.</pre>
</blockquote>
<p>On September 7th, 2013, 10:01 a.m. UTC, <b>David Faure</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;">+1. This seems to be the best solution.</pre>
</blockquote>
<p>On September 7th, 2013, 12:21 p.m. UTC, <b>Christophe Giboudeaux</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;">No no no, you completely ignored what I said yesterday.
QCA_INSTALL_IN_QT_PREFIX must be OFF by default without condition, and that was fixed with this review request.
> So, I agree to use QCA_INSTALL_IN_QT_PREFIX=ON only when CMAKE_INSTALL_PREFIX is not defined.
That just won't work, CMAKE_INSTALL_PREFIX will always be defined. This can be verified very simply:
project(test)
cmake_minimum_required(VERSION 2.8.9 FATAL_ERROR)
if(DEFINED CMAKE_INSTALL_PREFIX)
message(STATUS "PREFIX: ${CMAKE_INSTALL_PREFIX}")
endif()
Running 'cmake -DCMAKE_INSTALL_PREFIX="/foo"' will return "/foo"
Just running 'cmake' will return "/usr/local"
Please just add a README.PACKAGERS file giving information about the qca options.
</pre>
</blockquote>
<p>On September 7th, 2013, 7:07 p.m. UTC, <b>Ivan Romanov</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;">if(DEFINED CMAKE_INSTALL_PREFIX)
message(STATUS "PREFIX: ${CMAKE_INSTALL_PREFIX}")
endif()
project(test)
cmake_minimum_required(VERSION 2.8.9 FATAL_ERROR)
[taurus@localhost test]$ cmake .
-- The C compiler identification is GNU 4.8.1
-- The CXX compiler identification is GNU 4.8.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Configuring done
-- Generating done
-- Build files have been written to: /home/taurus/develop/free-lance/hamster-tz/icons/test
[taurus@localhost test]$ cmake -DCMAKE_INSTALL_PREFIX=/usr/local .
-- PREFIX: /usr/local
-- Configuring done
-- Generating done
-- Build files have been written to: /home/taurus/develop/free-lance/hamster-tz/icons/test
</pre>
</blockquote>
<p>On September 7th, 2013, 7:12 p.m. UTC, <b>Ivan Romanov</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;">I didn't ignore you. I just didn't see solid reasons to use /usr/local by default. I am looking for a way to satisfy everbody. Also I want to satisfy myself.</pre>
</blockquote>
<p>On September 7th, 2013, 7:31 p.m. UTC, <b>Ivan Romanov</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;">[taurus@localhost test]$ rm -fr CMakeCache.txt CMakeFiles cmake_install.cmake Makefile
[taurus@localhost test]$ cmake -DCMAKE_INSTALL_PREFIX=/usr/local .
-- PREFIX: /usr/local
-- The C compiler identification is GNU 4.8.1
-- The CXX compiler identification is GNU 4.8.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Configuring done
-- Generating done
-- Build files have been written to: /home/taurus/develop/free-lance/hamster-tz/icons/test
</pre>
</blockquote>
<p>On September 7th, 2013, 7:56 p.m. UTC, <b>Christophe Giboudeaux</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;">Forget /usr/local, that's not the problem. It's the default value that CMake will use if no installation prefix is given.
The issue is your option: I'm repeating: "QCA_INSTALL_IN_QT_PREFIX must be OFF by default" or said differently, never install anything outside CMAKE_INSTALL_PREFIX unless this option is enabled and you will make everyone happy.
</pre>
</blockquote>
<p>On September 7th, 2013, 8:28 p.m. UTC, <b>Ivan Romanov</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;">Why CMAKE_INSTLL_PREFIX must be used by default?</pre>
</blockquote>
<p>On September 7th, 2013, 8:30 p.m. UTC, <b>Ivan Romanov</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;">What bad to use Qt prefix by default? Why are you against it?</pre>
</blockquote>
<p>On September 7th, 2013, 8:34 p.m. UTC, <b>Ivan Romanov</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;">Also have a look at this line
111: set(QCA_FEATURE_INSTALL_DIR "${QT_MKSPECS_DIR}/features" CACHE PATH "Directory where qca feature file will install")
It is not uses CMAKE_INSTALL_PREFIX. </pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> Why CMAKE_INSTLL_PREFIX must be used by default?
> What bad to use Qt prefix by default? Why are you against it?
This is the same question.
Three examples:
1/ Joe uses his distribution Qt5 packages, they're installed to /usr with root permissions
If Joe tries to build QCA, if this option is on by default, it will require root password to install.
2/ Jack builds his own Qt5 and uses it uninstalled.
When he builds QCA with the behaviour you suggest, files will be installed in his Qt5 sources dir.
3/ Aver..err Ben did set up a continuous build host called Jenkins (also known as build.kde.org) where every module is installed in a different prefix. This helps to detect when developers make wrong assumtions and believe everything will be magically found. With the QCA option enabled by default, files will be installed where they shouldn't.
If none of these behaviours looks weird to you, discussing this further will lead nowhere.
> set(QCA_FEATURE_INSTALL_DIR "${QT_MKSPECS_DIR}/features" CACHE PATH "Directory where qca feature file will install")
Alexander probably didn't notice, I'll fix it asap
</pre>
<br />
<p>- Christophe</p>
<br />
<p>On September 3rd, 2013, 12:42 p.m. UTC, Alexander Richardson 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 KDE Frameworks and Ivan Romanov.</div>
<div>By Alexander Richardson.</div>
<p style="color: grey;"><i>Updated Sept. 3, 2013, 12:42 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;">Use qt5 suffix for files by default when installing a Qt5 version of QCA
This way coinstallation of Qt4 and Qt5 based QCA is possible by default
</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;">Compiled and installed.
creating a Qt5 package for openSuSE works fine</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>CMakeLists.txt <span style="color: grey">(8cff977)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(037c9ff)</span></li>
<li>src/config-qca.h.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/qca_plugin.cpp <span style="color: grey">(ad810b9)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111938/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>