Re: Review Request 113670: Link kde4support privately to QtX11Extras, QtSvg and QtTest

2013-11-08 Thread Andrius da Costa Ribas

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113670/#review43271
---


Please use X11 stuff conditionally (Q_WS_X11).

- Andrius da Costa Ribas


On Nov. 7, 2013, 8:33 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113670/
 ---
 
 (Updated Nov. 7, 2013, 8:33 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kdelibs
 
 
 Description
 ---
 
 Link kde4support privately to QtX11Extras, QtSvg and QtTest
 
 Otherwise every user of the target KF5::KDE4Support will also have
 Qt5::X11Extras Qt5::Svg Qt5::Test linked.
 
 This can cause linker errors like ld: cannot find -lQt5::Test
 
 REVIEW: 113670
 
 
 Diffs
 -
 
   tier4/kde4support/src/kdeui/kxerrorhandler.cpp 
 3f4765d03559c99833c573fe0e0072bc9973b973 
   tier4/kde4support/src/kdeui/kxerrorhandler.h 
 babf931e8a750d405885f02443f4910356511429 
   tier4/kde4support/src/CMakeLists.txt 
 cbfac3e9e4363cb66d0941c9048d8f501311084c 
 
 Diff: http://git.reviewboard.kde.org/r/113670/diff/
 
 
 Testing
 ---
 
 I previously got the following error compiling okteta, now it works:
 /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: 
 cannot find -lQt5::X11Extras
 /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: 
 cannot find -lQt5::Svg
 
 QtX11Info and QtSvg are not used in any installed headers, so IMO this should 
 be fine
 qtest_kde.h does actually include QtTest headers, but only uses these types 
 inside macros. And I guess any user of that header already also links to 
 QtTest
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 113670: Link kde4support privately to QtX11Extras, QtSvg and QtTest

2013-11-08 Thread Alex Merry


 On Nov. 8, 2013, 4:25 p.m., Andrius da Costa Ribas wrote:
  Please use X11 stuff conditionally (Q_WS_X11).

There's no need to put conditionals in the file (since the whole thing is 
conditionally compiled), but the linking should be done conditionally.

Specifically, you should take Qt5::X11Extras out of the target_link_libraries, 
and put it in its own call wrapped in X11_FOUND, like
if (X11_FOUND)
   target_link_libraries(KDE4Support PRIVATE Qt5::X11Extras)
endif ()

Or, even better, find the conditional where kxerrorhandler is added to 
libkde4support_SRCS, and add Qt5::X11Extras to libkd4support_OPTIONAL_LIBS in 
that same conditional.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113670/#review43271
---


On Nov. 7, 2013, 8:33 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113670/
 ---
 
 (Updated Nov. 7, 2013, 8:33 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kdelibs
 
 
 Description
 ---
 
 Link kde4support privately to QtX11Extras, QtSvg and QtTest
 
 Otherwise every user of the target KF5::KDE4Support will also have
 Qt5::X11Extras Qt5::Svg Qt5::Test linked.
 
 This can cause linker errors like ld: cannot find -lQt5::Test
 
 REVIEW: 113670
 
 
 Diffs
 -
 
   tier4/kde4support/src/kdeui/kxerrorhandler.cpp 
 3f4765d03559c99833c573fe0e0072bc9973b973 
   tier4/kde4support/src/kdeui/kxerrorhandler.h 
 babf931e8a750d405885f02443f4910356511429 
   tier4/kde4support/src/CMakeLists.txt 
 cbfac3e9e4363cb66d0941c9048d8f501311084c 
 
 Diff: http://git.reviewboard.kde.org/r/113670/diff/
 
 
 Testing
 ---
 
 I previously got the following error compiling okteta, now it works:
 /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: 
 cannot find -lQt5::X11Extras
 /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: 
 cannot find -lQt5::Svg
 
 QtX11Info and QtSvg are not used in any installed headers, so IMO this should 
 be fine
 qtest_kde.h does actually include QtTest headers, but only uses these types 
 inside macros. And I guess any user of that header already also links to 
 QtTest
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 113670: Link kde4support privately to QtX11Extras, QtSvg and QtTest

2013-11-07 Thread Alexander Richardson

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

(Updated Nov. 7, 2013, 9:33 p.m.)


Review request for KDE Frameworks.


Summary (updated)
-

Link kde4support privately to QtX11Extras, QtSvg and QtTest


Repository: kdelibs


Description (updated)
---

Link kde4support privately to QtX11Extras, QtSvg and QtTest

Otherwise every user of the target KF5::KDE4Support will also have
Qt5::X11Extras Qt5::Svg Qt5::Test linked.

This can cause linker errors like ld: cannot find -lQt5::Test

REVIEW: 113670


Diffs (updated)
-

  tier4/kde4support/src/kdeui/kxerrorhandler.cpp 
3f4765d03559c99833c573fe0e0072bc9973b973 
  tier4/kde4support/src/kdeui/kxerrorhandler.h 
babf931e8a750d405885f02443f4910356511429 
  tier4/kde4support/src/CMakeLists.txt cbfac3e9e4363cb66d0941c9048d8f501311084c 

Diff: http://git.reviewboard.kde.org/r/113670/diff/


Testing (updated)
---

I previously got the following error compiling okteta, now it works:
/usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: 
cannot find -lQt5::X11Extras
/usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: 
cannot find -lQt5::Svg

QtX11Info and QtSvg are not used in any installed headers, so IMO this should 
be fine
qtest_kde.h does actually include QtTest headers, but only uses these types 
inside macros. And I guess any user of that header already also links to QtTest


Thanks,

Alexander Richardson

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