Re: Review Request 122313: Expose to world whether KPty has been built with utempter library

2015-03-19 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122313/#review77724
---

Ship it!


- David Faure


On Jan. 29, 2015, 6:54 p.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122313/
 ---
 
 (Updated Jan. 29, 2015, 6:54 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kpty
 
 
 Description
 ---
 
 Equivalent to https://svn.reviewboard.kde.org/r/2468/
 It was lost in KF5 porting, and it was directly used to determine whether 
 kwrited should be built as module, or executable (in this case, it would be a 
 SUID binary, which Qt5 no longer -by default- allows)
 
 
 Diffs
 -
 
   CMakeLists.txt 7fe77da7b0bc97c6f64db4fcc63ef7831fa065b1 
   KF5PtyConfig.cmake.in 04bde7bffd209b57e755a66278025ee8b6453770 
   cmake/FindUTEMPTER.cmake PRE-CREATION 
   src/CMakeLists.txt caf2f0ba87ad4173af9860ae369b43d50ffd219f 
   src/ConfigureChecks.cmake f52be3f5e031c73dd7d26296622c14c9d69db42c 
 
 Diff: https://git.reviewboard.kde.org/r/122313/diff/
 
 
 Testing
 ---
 
 built, cmake configuration looks ok.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120393: [kdelibs4support] Kill dead code

2015-03-19 Thread Hrvoje Senjan


 On March 19, 2015, 12:23 a.m., Vishesh Handa wrote:
  I'm all for getting rid of the Nepomuk code. However, I'm not too sure 
  about the strigi part. That should still work.

It does not ;-)
Originally, this review added back the find_package(Strigi) call which was 
removed a while back (at least before 5.0.0), so this code was/is never 
compiled.


- Hrvoje


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120393/#review77714
---


On March 18, 2015, 7:24 p.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120393/
 ---
 
 (Updated March 18, 2015, 7:24 p.m.)
 
 
 Review request for KDE Frameworks, David Faure and Vishesh Handa.
 
 
 Repository: kdelibs4support
 
 
 Description
 ---
 
 Strigi check has been removed in commit 
 c8f4c69650c71276b2a2263212addde63764e58b, and soprano wasn't even ported to 
 Qt5 (afaik), so this was never compiled.
 
 
 Diffs
 -
 
   autotests/kfilemetainfotest.cpp c751cdd 
   src/CMakeLists.txt b662893 
   src/config-kdelibs4support.h.cmake 1af3ee0 
   src/kio/kfilemetadataconfigurationwidget.cpp 259b205 
   src/kio/kfilemetadataprovider.cpp 3468546 
   src/kio/kfilemetadataprovider_p.h 31137b2 
   src/kio/kfilemetadatawidget.cpp 1edb069 
   src/kio/kfilemetainfo.cpp eae1295 
   src/kio/kfilemetainfoitem.cpp 62f760d 
   src/kio/kfilemetainfoitem_p.h 8929e46 
   src/kio/knfotranslator.cpp 8eec6a1 
 
 Diff: https://git.reviewboard.kde.org/r/120393/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122313: Expose to world whether KPty has been built with utempter library

2015-03-19 Thread Hrvoje Senjan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122313/
---

(Updated March 19, 2015, 11:18 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit acf078d84d8f1c4d5fcf28cf9a9f570760ba1501 by Hrvoje Senjan 
to branch master.


Repository: kpty


Description
---

Equivalent to https://svn.reviewboard.kde.org/r/2468/
It was lost in KF5 porting, and it was directly used to determine whether 
kwrited should be built as module, or executable (in this case, it would be a 
SUID binary, which Qt5 no longer -by default- allows)


Diffs
-

  CMakeLists.txt 7fe77da7b0bc97c6f64db4fcc63ef7831fa065b1 
  KF5PtyConfig.cmake.in 04bde7bffd209b57e755a66278025ee8b6453770 
  cmake/FindUTEMPTER.cmake PRE-CREATION 
  src/CMakeLists.txt caf2f0ba87ad4173af9860ae369b43d50ffd219f 
  src/ConfigureChecks.cmake f52be3f5e031c73dd7d26296622c14c9d69db42c 

Diff: https://git.reviewboard.kde.org/r/122313/diff/


Testing
---

built, cmake configuration looks ok.


Thanks,

Hrvoje Senjan

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 123075: do not require X11 on Mac OS X

2015-03-19 Thread Jeremy Whiting

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123075/#review3
---

Ship it!


Ship It!

- Jeremy Whiting


On March 19, 2015, 4:59 p.m., Harald Fernengel wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123075/
 ---
 
 (Updated March 19, 2015, 4:59 p.m.)
 
 
 Review request for KDE Frameworks and Michael Palimaka.
 
 
 Repository: kdesu
 
 
 Description
 ---
 
 do not require X11 on Mac OS X
 
 
 Diffs
 -
 
   CMakeLists.txt 9623483d6f11f9cdb7d7dc19decfd7cf5e86d079 
 
 Diff: https://git.reviewboard.kde.org/r/123075/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Harald Fernengel
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 123075: do not require X11 on Mac OS X

2015-03-19 Thread Harald Fernengel

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123075/
---

Review request for KDE Frameworks and Michael Palimaka.


Repository: kdesu


Description
---

do not require X11 on Mac OS X


Diffs
-

  CMakeLists.txt 9623483d6f11f9cdb7d7dc19decfd7cf5e86d079 

Diff: https://git.reviewboard.kde.org/r/123075/diff/


Testing
---


Thanks,

Harald Fernengel

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


cmake CMP0028 missing targets - what does one do about it?

2015-03-19 Thread Harald Sitter
alohas

While investigating a wall of build failures in the kubuntu ci this
morning I stumbled upon a very interesting problem.

Problem tldr: target A links library B, A doesn't get the includes of
link libraries of B by default. e.g. A links B, B links karchive, A
doesn't have access to the karchive headers. also if karchive wasn't
found in a super-scope of A and B, A will not know about karchive.

Ultimately the build failures are fallout from the public dependency
tightening, they do however hightlight a bit of a problem with how we
handle find_packages outside the main cmakelists as well as linking to
targets from within the same cmake project. I feel like I had seen a
solution for that somewhere so excuse my stupidity in case we already
have a well understood solution to this apparently not so uncommon
problem.

The two repos I noticed the problem with is muon [1] and kio [2].

The general problem is that they build a library that links against
some framework (in case of muon it's KF5::KDELibs4Support, in case of
KIO it's KIOFileWidgets linking against KF5::XmlGui). Both repos have
somewhat split find_package calls that happen as close to the import
target use as possible, giving the import targets a lower scope than
main cmakelists.
In both repos another target (in muon it's another lib, in KIO a test
case) then links against the respectively library that linked the
framework.
This is where things go terribly wrong.

Since the find_package calls are in a lower scope, the target linking
the lib target does not know about them. This ideally results in a
cmake warning about CMP0028 and a colon target that cannot be resolved
(which KIO ignores by forcing old policy behavior, weeh).

Now the obvious way to resolve this is to find_package in both scopes,
such that the targets become available.

BUT...

that would duplicate logic and is altogether not going to scale very well.

SO...

Question 1: can we somehow pass along imported target information to
linkees within the same cmake project? Much like we do with the
generated cmake configs that will import the interface dependencies.

Question 2: If there is such a way, is that preferred over duplicated
find_packages?

Question 3: Should we really force the old policy behavior regarding
CMP0028 considering it might very well cause a build failure which
then doesn't have an obvious cause anymore? (KIO autotests dir does
that right now)

I pushed a minimal example of the problem as I understand it to [3].
Problem is in a/CMakeLists.txt

[1] 
https://launchpadlibrarian.net/200634395/buildlog_ubuntu-vivid-i386.muon_4%3A5.2.1%2Bgit20150319.1139%2B15.04-0ubuntu0_BUILDING.txt.gz
[2] 
https://launchpadlibrarian.net/200604917/buildlog_ubuntu-vivid-amd64.kio_5.8.0%2Bgit20150319.0312%2B15.04-0ubuntu0_BUILDING.txt.gz
[3] kde:scratch/sitter/CMP0028
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: cmake CMP0028 missing targets - what does one do about it?

2015-03-19 Thread Aleix Pol
On Thu, Mar 19, 2015 at 2:16 PM, Harald Sitter sit...@kde.org wrote:
 alohas

 While investigating a wall of build failures in the kubuntu ci this
 morning I stumbled upon a very interesting problem.

 Problem tldr: target A links library B, A doesn't get the includes of
 link libraries of B by default. e.g. A links B, B links karchive, A
 doesn't have access to the karchive headers. also if karchive wasn't
 found in a super-scope of A and B, A will not know about karchive.

 Ultimately the build failures are fallout from the public dependency
 tightening, they do however hightlight a bit of a problem with how we
 handle find_packages outside the main cmakelists as well as linking to
 targets from within the same cmake project. I feel like I had seen a
 solution for that somewhere so excuse my stupidity in case we already
 have a well understood solution to this apparently not so uncommon
 problem.

 The two repos I noticed the problem with is muon [1] and kio [2].

 The general problem is that they build a library that links against
 some framework (in case of muon it's KF5::KDELibs4Support, in case of
 KIO it's KIOFileWidgets linking against KF5::XmlGui). Both repos have
 somewhat split find_package calls that happen as close to the import
 target use as possible, giving the import targets a lower scope than
 main cmakelists.
 In both repos another target (in muon it's another lib, in KIO a test
 case) then links against the respectively library that linked the
 framework.
 This is where things go terribly wrong.

 Since the find_package calls are in a lower scope, the target linking
 the lib target does not know about them. This ideally results in a
 cmake warning about CMP0028 and a colon target that cannot be resolved
 (which KIO ignores by forcing old policy behavior, weeh).

 Now the obvious way to resolve this is to find_package in both scopes,
 such that the targets become available.

 BUT...

 that would duplicate logic and is altogether not going to scale very well.

 SO...

 Question 1: can we somehow pass along imported target information to
 linkees within the same cmake project? Much like we do with the
 generated cmake configs that will import the interface dependencies.
I'd say the only thing that makes sense is to have globally defined
find_packages(). Simplifies the whole issue.


 Question 2: If there is such a way, is that preferred over duplicated
 find_packages?
As soon as it's the dependencies of the dependencies, I'd say it
doesn't make sense to duplicate.


 Question 3: Should we really force the old policy behavior regarding
 CMP0028 considering it might very well cause a build failure which
 then doesn't have an obvious cause anymore? (KIO autotests dir does
 that right now)
We want the new behavior, IMHO.


 I pushed a minimal example of the problem as I understand it to [3].
 Problem is in a/CMakeLists.txt

 [1] 
 https://launchpadlibrarian.net/200634395/buildlog_ubuntu-vivid-i386.muon_4%3A5.2.1%2Bgit20150319.1139%2B15.04-0ubuntu0_BUILDING.txt.gz
 [2] 
 https://launchpadlibrarian.net/200604917/buildlog_ubuntu-vivid-amd64.kio_5.8.0%2Bgit20150319.0312%2B15.04-0ubuntu0_BUILDING.txt.gz
 [3] kde:scratch/sitter/CMP0028

Aleix
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Change in kio[master]: Consolidate find_package calls

2015-03-19 Thread Hrvoje Senjan (Code Review)
Hello Aleix Pol Gonzalez, David Faure,

I'd like you to do a code review.  Please visit

https://gerrit.vesnicky.cesnet.cz/r/432

to review the following change.

Change subject: Consolidate find_package calls
..

Consolidate find_package calls

Remove all duplicated find_package calls from 2nd and below level
CMakeLists. One exception is KF5XmlGui, which needs to be found for
autotests; previously it was found through KF5Bookmarks' find_dependency call.
This will fix build against KF5 master with BUILD_TESTING=ON (=default).

Change-Id: I9025505d57fe82438dea8c0270f962bf9fed36cf
---
M CMakeLists.txt
M autotests/CMakeLists.txt
M autotests/http/CMakeLists.txt
M src/filewidgets/CMakeLists.txt
M src/ioslaves/help/CMakeLists.txt
M src/ioslaves/http/CMakeLists.txt
6 files changed, 1 insertion(+), 17 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/kio refs/changes/32/432/1

diff --git a/CMakeLists.txt b/CMakeLists.txt
index b1cd0e1..577f8c5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -46,6 +46,7 @@
 find_package(KF5IconThemes ${KF5_DEP_VERSION} REQUIRED)
 find_package(KF5ItemViews ${KF5_DEP_VERSION} REQUIRED)
 find_package(KF5JobWidgets ${KF5_DEP_VERSION} REQUIRED)
+find_package(KF5XmlGui ${KF5_DEP_VERSION} REQUIRED)
 find_package(KF5WidgetsAddons ${KF5_DEP_VERSION} REQUIRED)
 find_package(KF5WindowSystem ${KF5_DEP_VERSION} REQUIRED)
 endif()
diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
index 69c8957..1bbcb35 100644
--- a/autotests/CMakeLists.txt
+++ b/autotests/CMakeLists.txt
@@ -8,11 +8,7 @@
 add_subdirectory(http)
 add_subdirectory(kcookiejar)
 
-find_package(Qt5Widgets REQUIRED)
-
 ### unittests ###
-
-find_package(Qt5Concurrent 5.2.0 REQUIRED NO_MODULE)
 
 ecm_add_tests(
  kacltest.cpp
diff --git a/autotests/http/CMakeLists.txt b/autotests/http/CMakeLists.txt
index 069d7ae..a55c2cc 100644
--- a/autotests/http/CMakeLists.txt
+++ b/autotests/http/CMakeLists.txt
@@ -1,7 +1,3 @@
-find_package(Qt5Test REQUIRED)
-find_package(Qt5Widgets REQUIRED)
-find_package(KF5Archive ${KF5_DEP_VERSION} REQUIRED)
-
 find_package(ZLIB)
 set_package_properties(ZLIB PROPERTIES DESCRIPTION Support for gzip 
compressed files and data streams
URL http://www.zlib.net;
diff --git a/src/filewidgets/CMakeLists.txt b/src/filewidgets/CMakeLists.txt
index 37c3f26..903baad 100644
--- a/src/filewidgets/CMakeLists.txt
+++ b/src/filewidgets/CMakeLists.txt
@@ -1,8 +1,5 @@
 project(KIOFileWidgets)
 
-find_package(KF5Bookmarks ${KF5_DEP_VERSION} REQUIRED)
-find_package(KF5XmlGui ${KF5_DEP_VERSION} REQUIRED)
-
 configure_file(config-kiofilewidgets.h.cmake 
${CMAKE_CURRENT_BINARY_DIR}/config-kiofilewidgets.h)
 
 set(kiofilewidgets_SRCS
diff --git a/src/ioslaves/help/CMakeLists.txt b/src/ioslaves/help/CMakeLists.txt
index 8b7b21e..1895669 100644
--- a/src/ioslaves/help/CMakeLists.txt
+++ b/src/ioslaves/help/CMakeLists.txt
@@ -2,7 +2,6 @@
 
 remove_definitions(-DQT_NO_CAST_FROM_ASCII)
 
-find_package(KF5Archive ${KF5_DEP_VERSION} REQUIRED)
 find_package(LibXslt)
 set_package_properties(LibXslt PROPERTIES
URL http://xmlsoft.org/XSLT;
@@ -27,8 +26,6 @@
 configure_file(config-help.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-help.h )
 
 #macro_additional_clean_files( ${CMAKE_CURRENT_BINARY_DIR}/checkXML )
-
-find_package(Qt5Core 5.2.0 REQUIRED NO_MODULE)
 
 ### next target ###
 
diff --git a/src/ioslaves/http/CMakeLists.txt b/src/ioslaves/http/CMakeLists.txt
index 76a8e28..0066bd1 100644
--- a/src/ioslaves/http/CMakeLists.txt
+++ b/src/ioslaves/http/CMakeLists.txt
@@ -3,9 +3,6 @@
 include(ConfigureChecks.cmake)
 configure_file(config-kioslave-http.h.cmake 
${CMAKE_CURRENT_BINARY_DIR}/config-kioslave-http.h )
 
-find_package(X11)
-set(HAVE_X11 ${X11_FOUND})
-
 if(GSSAPI_FOUND)
 set(HAVE_LIBGSSAPI 1)
 if(GSSAPI_FLAVOR STREQUAL MIT)

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/432
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9025505d57fe82438dea8c0270f962bf9fed36cf
Gerrit-PatchSet: 1
Gerrit-Project: kio
Gerrit-Branch: master
Gerrit-Owner: Hrvoje Senjan hrvoje.sen...@gmail.com
Gerrit-Reviewer: Aleix Pol Gonzalez aleix...@kde.org
Gerrit-Reviewer: David Faure fa...@kde.org
Gerrit-Reviewer: Michael Palimaka kensing...@gentoo.org
Gerrit-Reviewer: Patrick Spendrin ps...@gmx.de
Gerrit-Reviewer: Sysadmin Testing Account n...@kde.org
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel