D18947: Bring FindUDev.cmake up to ECM standards

2019-02-16 Thread Volker Krause 
This revision was automatically updated to reflect the committed changes.
Closed by commit R245:7464b55f0638: Bring FindUDev.cmake up to ECM standards 
(authored by vkrause).

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18947?vs=51848=51853

REVISION DETAIL
  https://phabricator.kde.org/D18947

AFFECTED FILES
  CMakeLists.txt
  KF5SolidConfig.cmake.in
  cmake/FindUDev.cmake
  src/solid/devices/CMakeLists.txt

To: vkrause, #build_system, cgiboudeaux, apol
Cc: aacid, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D18947: Bring FindUDev.cmake up to ECM standards

2019-02-16 Thread Christophe Giboudeaux 
cgiboudeaux accepted this revision.

REPOSITORY
  R245 Solid

BRANCH
  arcpatch-D18947

REVISION DETAIL
  https://phabricator.kde.org/D18947

To: vkrause, #build_system, cgiboudeaux, apol
Cc: aacid, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D18943: Add Fontconfig find modudle

2019-02-16 Thread Volker Krause 
vkrause updated this revision to Diff 51849.
vkrause added a comment.


  Search for pkgconfig quietly.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18943?vs=51524=51849

BRANCH
  arcpatch-D18943

REVISION DETAIL
  https://phabricator.kde.org/D18943

AFFECTED FILES
  attic/modules/FindFontconfig.cmake
  docs/find-module/FindFontconfig.rst
  find-modules/FindFontconfig.cmake

To: vkrause
Cc: kossebau, apol, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
michaelh, ngraham, bruns

D18947: Bring FindUDev.cmake up to ECM standards

2019-02-16 Thread Volker Krause 
vkrause updated this revision to Diff 51848.
vkrause added a comment.


  Fix typos.

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18947?vs=51845=51848

BRANCH
  arcpatch-D18947

REVISION DETAIL
  https://phabricator.kde.org/D18947

AFFECTED FILES
  CMakeLists.txt
  KF5SolidConfig.cmake.in
  cmake/FindUDev.cmake
  src/solid/devices/CMakeLists.txt

To: vkrause, #build_system, cgiboudeaux, apol
Cc: aacid, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D18943: Add Fontconfig find modudle

2019-02-16 Thread Christophe Giboudeaux 
cgiboudeaux added inline comments.

INLINE COMMENTS

> FindFontconfig.cmake:56
> +# in the FIND_PATH() and FIND_LIBRARY() calls
> +find_package(PkgConfig)
> +pkg_check_modules(PC_FONTCONFIG QUIET fontconfig)

QUIET here as well

> apol wrote in FindFontconfig.cmake:57
> If you use IMPORTED_TARGET you can skip most stuff below. i.e.
> `pkg_check_modules(Flatpak IMPORTED_TARGET flatpak>=0.11.8)`
> 
> In fact, I'd argue it's just easier to have pkg_check_modules called upstream 
> rather than having an intermediary in ECM or so. 
> It's what I did in Discover at least:
> https://phabricator.kde.org/source/discover/browse/master/CMakeLists.txt$36

ECM requires CMake 2.8.12. 'IMPORTED_TARGET' is not available in this version.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D18943

To: vkrause
Cc: kossebau, apol, cgiboudeaux, kde-frameworks-devel, kde-buildsystem, 
michaelh, ngraham, bruns

D18952: new find module for Canberra

2019-02-16 Thread Christophe Giboudeaux 
cgiboudeaux added a comment.


  In D18952#413454 , @aacid wrote:
  
  > Yes, imported targets are the future/present :)
  >
  > Canberra::Canberra sounds good to me as target name
  
  
  +1
  You may also add a set_package_properties() call. see eg: D18947 
. This way, the users only have to use the 
VERSION and TYPE in their CMakeLists.txt.

INLINE COMMENTS

> FindCanberra.cmake:9-16
> +# ``CANBERRA_FOUND``
> +# True if (the requested version of) Canberra is available
> +# ``CANBERRA_VERSION``
> +# The version of Canberra
> +# ``CANBERRA_LIBRARIES``
> +# The libraries of Canberra for use with target_link_libraries()
> +# ``CANBERRA_INCLUDE_DIRS``

The variables shall be renamed "Canberra_XXX".

If needed, the uppercase ones shall be added at the bottom of the file with a 
comment to indicate they only exist for compatibility.

> FindCanberra.cmake:48-49
> +
> +find_package(PkgConfig)
> +pkg_check_modules(PC_CANBERRA libcanberra)
> +

Use the 'QUIET' keyword for both lines

> FindCanberra.cmake:53
> +NAMES canberra
> +HINTS ${PC_CANBERRA_LIBRARY_DIRS} ${PC_CANBERRA_LIBDIR}
> +)

PC_CANBERRA_LIBDIR isn't needed

> FindCanberra.cmake:58
> +NAMES canberra.h
> +HINTS ${PC_CANBERRA_INCLUDE_DIRS} ${PC_CANBERRA_INCLUDEDIR}
> +)

Same thing for PC_CANBERRA_INCLUDEDIR

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D18952

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D19075: Fix FindEGL

2019-02-16 Thread Tobias C. Berner 
tcberner updated this revision to Diff 51847.
tcberner added a comment.


  Simply set `NAMES EGL/egl.h` and fixup the header version check.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19075?vs=51842=51847

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D19075

AFFECTED FILES
  find-modules/FindEGL.cmake

To: tcberner, #automotive, hausmann, #freebsd
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18947: Bring FindUDev.cmake up to ECM standards

2019-02-16 Thread Christophe Giboudeaux 
cgiboudeaux added inline comments.

INLINE COMMENTS

> FindUDev.cmake:10
> +#
> +#  ``UDev_INCLUDE_DIR``
> +#  The libudev include directory.

UDev_INCLUDE_DIRS

> FindUDev.cmake:13
> +#
> +# ``UDev_LIBS``
> +# The libudev libraries.

UDev_LIBRARIES

REPOSITORY
  R245 Solid

BRANCH
  arcpatch-D18947

REVISION DETAIL
  https://phabricator.kde.org/D18947

To: vkrause, #build_system, cgiboudeaux, apol
Cc: aacid, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D19075: Fix FindEGL

2019-02-16 Thread Tobias C. Berner 
tcberner added a comment.


  In D19075#413462 , @hausmann wrote:
  
  > I think that it should be NAMES EGL/egl.h
  
  
  Yeah, then it might actuayll work without redefining the path -- but in both 
cases we are kind of assuming that all install egl.h into a subdir called 'EGL'.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D19075

To: tcberner, #automotive, hausmann, #freebsd
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18947: Bring FindUDev.cmake up to ECM standards

2019-02-16 Thread Volker Krause 
vkrause updated this revision to Diff 51845.
vkrause added a comment.


  Add IMPORTED_COMPILE_DEFINITIONS property.

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18947?vs=51485=51845

BRANCH
  arcpatch-D18947

REVISION DETAIL
  https://phabricator.kde.org/D18947

AFFECTED FILES
  CMakeLists.txt
  KF5SolidConfig.cmake.in
  cmake/FindUDev.cmake
  src/solid/devices/CMakeLists.txt

To: vkrause, #build_system, cgiboudeaux, apol
Cc: aacid, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D19075: Fix FindEGL

2019-02-16 Thread Simon Hausmann 
hausmann added a comment.


  I think that it should be NAMES EGL/egl.h

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D19075

To: tcberner, #automotive, hausmann, #freebsd
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18947: Bring FindUDev.cmake up to ECM standards

2019-02-16 Thread Albert Astals Cid 
aacid added inline comments.

INLINE COMMENTS

> FindUDev.cmake:75
> +IMPORTED_LOCATION "${UDev_LIBRARIES}"
> +INTERFACE_INCLUDE_DIRECTORIES "${UDev_INCLUDE_DIRS}"
> +)

Do we need a 
INTERFACE_COMPILER_DEFINITIONS "${UDev_DEFINITIONS}"
just in case that udev adds some definitions at some point?

REPOSITORY
  R245 Solid

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18947

To: vkrause, #build_system, cgiboudeaux, apol
Cc: aacid, apol, kde-frameworks-devel, michaelh, ngraham, bruns

D19075: Fix FindEGL

2019-02-16 Thread Tobias C. Berner 
tcberner updated this revision to Diff 51842.
tcberner added a comment.


  Store the path gathered via pkgconfig in `COMPLETE_EGL_INCLUDE_DIR` and 
  use its parent directory for `EGL_INCLUDE_DIR`.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19075?vs=51825=51842

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D19075

AFFECTED FILES
  find-modules/FindEGL.cmake

To: tcberner, #automotive, hausmann, #freebsd
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

2019-02-16 Thread Albert Astals Cid 
aacid added a comment.


  Yes, imported targets are the future/present :)
  
  Canberra::Canberra sounds good to me as target name

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D18952

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D19075: Fix FindEGL

2019-02-16 Thread Simon Hausmann 
hausmann added a comment.


  Yeah I think find_path should use the same style as the test program 
(EGL/egl.h). It is the style of inclusion as per the specification. I think 
that’s better than the ../ approach.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D19075

To: tcberner, #automotive, hausmann, #freebsd
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D19075: Fix FindEGL

2019-02-16 Thread Simon Hausmann 
hausmann requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D19075

To: tcberner, #automotive, hausmann, #freebsd
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns