Review Request 117658: Add a stub for ECMFindModuleHelpers to the find-modules dir

2014-04-20 Thread Alex Merry

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

Review request for Build System, Extra Cmake Modules and KDE Frameworks.


Repository: extra-cmake-modules


Description
---

Split up module execution tests

Find module tests now use find_package(), and there is a version for
when CMAKE_MODULE_PATH is set and a version for when
ecm_use_find_modules() is used.

KDE modules are also now tested.

Add a stub for ECMFindModuleHelpers to the find-modules dir

ECMUseFindModules allows find modules to be copied to a local directory.
These find modules may use ECMFindModuleHelpers, but they will not be in
the same relative location to ECMFindModuleHelpers.cmake and there is no
guarantee that ECMFindModulesHelpers.cmake will be in the CMake module
path.

To solve this, we make sure there is always a stub file in the same
directory as the find modules that includes the real
ECMFindModuleHelpers.cmake. The one installed with ECM just includes
../modules/ECMFindModuleHelpers.cmake, while ecm_use_find_modules
generates a stub that uses an absolute path.


Diffs
-

  find-modules/FindWayland.cmake b7790e750e8ae9b1c6d5be81b7683b489d60a5a7 
  find-modules/ECMFindModuleHelpersStub.cmake PRE-CREATION 
  find-modules/FindXCB.cmake dd876b214edd35993b8e8d3582536a24776a2e64 
  modules/ECMUseFindModules.cmake 25f42666ceaecdac4034caf43c31f3f219f9070b 
  tests/CMakeLists.txt e464a0305bd71364463c3132103ffe02dcb94eb6 
  tests/ExecuteAllModules/CMakeLists.txt 
12e86c15d209ff38340af0dd0a5900091ce4cadb 
  tests/ExecuteAllModules/main.c  
  tests/ExecuteCoreModules/CMakeLists.txt PRE-CREATION 
  tests/ExecuteKDEModules/CMakeLists.txt PRE-CREATION 
  tests/ExecuteKDEModules/main.c PRE-CREATION 
  tests/FindModules/CMakeLists.txt PRE-CREATION 
  tests/FindModules/main.c PRE-CREATION 
  tests/UseFindModules/CMakeLists.txt PRE-CREATION 
  tests/UseFindModules/main.c PRE-CREATION 

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


Testing
---

Tests pass. Also made a little test that called find_package(Wayland), both 
using set(CMAKE_MODULE_PATH ${ECM_FIND_MODULE_DIR}) and using 
ecm_use_find_modules(), and Wayland was found both times.


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 115210: Always set DATA_INSTALL_DIR to %ALLUSERSPROFILE% on Windows

2014-04-20 Thread Andrius da Costa Ribas


 On Jan. 22, 2014, 8:22 a.m., Patrick von Reth wrote:
  Until now we had no problems with the data installed to bin/../share  and 
  this setup would make it impossible to have multiple independent  kde 
  setups on one system.
 
 Alexander Richardson wrote:
 I know. The problem is QStandardPaths with 
 QStandardPaths::GenericDataLocation only looks in %ALLUSERSPROFILE% and I 
 think %APPDATA%. KF5 based KDE software won't work otherwise since it can't 
 find the data. I think the better way of fixing this is patching Qt, but for 
 now this works.
 
 Patrick Spendrin wrote:
 Can you keep that patch locally for now and we try and come up with a 
 patch for Qt instead? We cannot restrict ourselves at that point I think.
 
 Alexander Richardson wrote:
 Sure no problem. I'll drop this request
 
 David Faure wrote:
 So what do you recommend instead, for QStandardPaths? Checking some 
 non-standard environment variable? or?
 
 Alexander Richardson wrote:
 I would go for the environment variable. Something like 
 QSTANDARDPATHS_EXTRA_DATA_DIRS that is checked in addition to the default 
 dirs.
 
 Would also be useful for other cases: e.g. in the okteta unit tests I set 
 XDG_DATA_DIRS so that my test data gets found by QStandardPaths (I know there 
 is QFINDTESTDATA, but that won't work in that case).
 
 It would also be nice if there were some cross-platform solution like 
 QStandardpaths::addDirectory(QStandardPaths::StandardLocation, const QString 
 path) to inject (like KStandardDirs::addResourceDir).
 
 Patrick von Reth wrote:
 I don't like the idea of using the env var as this would require the user 
 to setup the variables or a kde process to set them up.
 We also would get an undefined behaviour if the env var is not set.
 I think kde is not the only qt project ported to windows wich uses the 
 bin/../share location on windows, so why not only add this path with a low 
 priority to QStrandardpathes?

 
 David Faure wrote:
 I agree that the env var would be quite inconvenient, which is why I was 
 dubious about that approach.
 
 A method to add paths wouldn't help either (how would all apps do it?)
 
 bin/../share means go up one level from the location of the executable 
 and enter share? I thought Windows apps didn't use a bin/ dir actually, but 
 were rather in the toplevel?
 Anyhow I'd be fine with that, especially if you can find any 
 documentation of this outside of kde (to explain the reasoning in the Qt 
 change request).

As far as I can see:

1 - Most GNU apps and libs ported from *nix to Windows use $PREFIX/bin, 
$PREFIX/share etc... and PREFIX is not standard on windows (those apps normally 
don't use %PROGRAMFILES%)
 * Other autoconf-based apps and libs also follow this structure
 * even those using %PROGRAMFILES% also follow this structure (e.g. for GIMP 2, 
$PREFIX is %PROGRAMFILES%\GIMP 2\, having bin, lib, share... inside it)
2 - Most CMake-based apps also follow a similar pattern, relative to 
$CMAKE_INSTALL_PREFIX, having no specific if(WIN32) to install to a different 
directory structure
 * Cmake itself is distributed in this kind of structure 
(http://www.cmake.org/files/v2.8/cmake-2.8.12.2.zip)

I think those can explain the reasoning needed for a Qt request.


- Andrius da Costa


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


On Jan. 22, 2014, 2:53 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115210/
 ---
 
 (Updated Jan. 22, 2014, 2:53 p.m.)
 
 
 Review request for Build System, Extra Cmake Modules, KDE Frameworks, and 
 kdewin.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Always set DATA_INSTALL_DIR to %ALLUSERSPROFILE% on Windows
 
 Otherwise QStandardPaths will always fail with e.g. GenericDataLocation
 
 
 Diffs
 -
 
   kde-modules/KDEInstallDirs.cmake 46e15c17d488d56f146aba0c2d420f74a22b9152 
 
 Diff: https://git.reviewboard.kde.org/r/115210/diff/
 
 
 Testing
 ---
 
 
 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 117652: Rewrite FindLibGcrypt.cmake to not use gcrypt-config

2014-04-20 Thread Nicolás Alvarez

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

(Updated April 20, 2014, 9:06 p.m.)


Review request for Build System and KDE Frameworks.


Changes
---

Fixed issues:
- Reformatted docs
- Added full license text
- Removed LibGcrypt_FIND_VERSION
- Run libgcrypt-config to get path hints


Repository: kwallet


Description
---

Using the gcrypt-config shell script won't work on Windows.

I wrote a new find module from scratch. It now searches for the library and 
include directory the usual CMake way (find_library and find_path). It supports 
version checking by extracting the version number from the gcrypt.h header.

By the way, lxr shows there are four different copies of FindLibGcrypt.cmake in 
different KDE projects and they all have one flaw or another. I think none uses 
FindPackageHandleStandardArgs, for example. Maybe we should consider putting 
this one into ECM for kwallet and the others to share.


Diffs (updated)
-

  CMakeLists.txt 9aad9c0 
  cmake/FindLibGcrypt.cmake 45c0d5d 
  src/runtime/kwalletd/backend/CMakeLists.txt 4c36767 

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


Testing
---

Tested on Windows with MSVC2010, cmake 2.8.12, finding emerge-built libgcrypt. 
Also tested on Debian Linux, cmake 2.8.12.1, finding the distro's libgcrypt.


Thanks,

Nicolás Alvarez

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


Re: Review Request 117012: Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed camelcase header under $

2014-04-20 Thread Andrius da Costa Ribas

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

(Updated April 21, 2014, 1:59 a.m.)


Review request for KDE Frameworks and kdewin.


Changes
---

place all includes under include/KJsEmbed (instead of having another kjsembed 
subdir) and remove 
$INSTALL_INTERFACE:${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed from 
target_include_directories.


Summary (updated)
-

Place KJsEmbed headers directly under ${INCLUDE_INSTALL_DIR}/KJsEmbed instead 
of under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed [was: Place KJsEmbed 
camelcase header under ${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed]


Repository: kjsembed


Description
---

Currently kjsembed CMake file tries to install both 
${INCLUDE_INSTALL_DIR}/KJsEmbed/kjsembed (directory) and 
${INCLUDE_INSTALL_DIR}/KJsEmbed/KJsEmbed (camel case header). This is not 
allowed in a case-insensitive filesystem, causing the install step to fail on 
Windows.


Diffs (updated)
-

  src/kjsembed/CMakeLists.txt e0ab74c 

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


Testing
---

Tested using MSVC 2013


Thanks,

Andrius da Costa Ribas

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