D8998: Add FindSeccomp to find-modules

2018-01-28 Thread David Kahles 
This revision was automatically updated to reflect the committed changes.
Closed by commit R240:c30802019895: Add FindSeccomp to find-modules (authored 
by davidk).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8998?vs=26106=26107

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

AFFECTED FILES
  find-modules/FindSeccomp.cmake

To: davidk, graesslin, cgiboudeaux
Cc: cgiboudeaux, #frameworks, #build_system, michaelh

D8998: Add FindSeccomp to find-modules

2018-01-28 Thread David Kahles 
davidk added a comment.


  Thanks for your review.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin, cgiboudeaux
Cc: cgiboudeaux, #frameworks, #build_system, michaelh

D8998: Add FindSeccomp to find-modules

2018-01-28 Thread David Kahles 
davidk updated this revision to Diff 26106.
davidk added a comment.


  Fix version.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8998?vs=24751=26106

BRANCH
  master

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

AFFECTED FILES
  find-modules/FindSeccomp.cmake

To: davidk, graesslin, cgiboudeaux
Cc: cgiboudeaux, #frameworks, #build_system, michaelh

D8998: Add FindSeccomp to find-modules

2018-01-28 Thread Christophe Giboudeaux 
cgiboudeaux accepted this revision.
cgiboudeaux added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> FindSeccomp.cmake:21
> +#
> +# Since 5.42.0.
> +

5.44.0

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: davidk, graesslin, cgiboudeaux
Cc: cgiboudeaux, #frameworks, #build_system, michaelh

D8998: Add FindSeccomp to find-modules

2018-01-28 Thread David Kahles 
davidk added a comment.


  @cgiboudeaux is it ready to go now?

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system, michaelh

D8998: Add FindSeccomp to find-modules

2018-01-05 Thread David Kahles 
davidk marked 14 inline comments as done.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2018-01-04 Thread David Kahles 
davidk marked 2 inline comments as not done.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2018-01-04 Thread David Kahles 
davidk marked 8 inline comments as not done.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2018-01-04 Thread David Kahles 
davidk updated this revision to Diff 24751.
davidk marked an inline comment as done.
davidk added a comment.


  Fix remaining problems

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8998?vs=22958=24751

BRANCH
  master

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

AFFECTED FILES
  find-modules/FindSeccomp.cmake

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-27 Thread Christophe Giboudeaux 
cgiboudeaux added a comment.


  Mostly good. Last question : is the version important ? If yes, please add an 
additional way to get the version if Seccomp_VERSION is empty. (you can parse 
seccomp.h to find it, look at the other Find*.cmake modules for examples)

INLINE COMMENTS

> FindSeccomp.cmake:3
> +# FindSeccomp
> +# -
> +#

nitpick : missing 2 dashes

> FindSeccomp.cmake:51
> +
> +find_package(PkgConfig)
> +pkg_check_modules(PKG_Libseccomp QUIET libseccomp)

Also add the QUIET keyword here

> FindSeccomp.cmake:54
> +
> +set(Seccomp_DEFINITIONS ${PKG_Libseccomp_CFLAGS_OTHER})
> +set(Seccomp_VERSION ${PKG_Libseccomp_VERSION})

This doesn't look useful. Looking at my pkgconfig file, the cflags just adds 
the include dir.

If neither baloo or kscreenlocker use it, just remove this line and #85.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-26 Thread David Kahles 
davidk marked 7 inline comments as done.
davidk added a comment.


  Thank you, missed this when renaming the docs.

INLINE COMMENTS

> cgiboudeaux wrote in FindSeccomp.cmake:12
> so, what about naming your variables Seccomp_LIBRARIES and 
> Seccomp_INCLUDE_DIRS in the file ?

WellI'm for it! ;)

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-26 Thread David Kahles 
davidk updated this revision to Diff 22958.
davidk added a comment.


  Fix variable names

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8998?vs=22931=22958

BRANCH
  master

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

AFFECTED FILES
  find-modules/FindSeccomp.cmake

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-26 Thread Christophe Giboudeaux 
cgiboudeaux added inline comments.

INLINE COMMENTS

> FindSeccomp.cmake:12
> +# The seccomp include directories
> +# ``Seccomp_LIBRARIES``
> +# The seccomp libraries for linking

so, what about naming your variables Seccomp_LIBRARIES and Seccomp_INCLUDE_DIRS 
in the file ?

> FindSeccomp.cmake:24
> +#=
> +# Copyright (c) 2017 Martin Flöser , David Kahles 
> 
> +#

One author per line

> FindSeccomp.cmake:56
> +
> +find_path(Seccomp_INCLUDE_DIR
> +NAMES

Seccomp_INCLUDE_DIRS

> FindSeccomp.cmake:62
> +)
> +find_library(Seccomp_LIBRARY
> +NAMES

Seccomp_LIBRARIES

> FindSeccomp.cmake:74-75
> +REQUIRED_VARS
> +Seccomp_LIBRARY
> +Seccomp_INCLUDE_DIR
> +VERSION_VAR

same thing here

> FindSeccomp.cmake:83-85
> +IMPORTED_LOCATION "${Seccomp_LIBRARY}"
> +INTERFACE_COMPILE_OPTIONS "${Seccomp_DEFINITIONS}"
> +INTERFACE_INCLUDE_DIRECTORIES "${Seccomp_INCLUDE_DIR}"

and there

> FindSeccomp.cmake:89
> +
> +mark_as_advanced(Seccomp_LIBRARY Seccomp_INCLUDE_DIR)
> +

here as well

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: cgiboudeaux, #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread David Kahles 
davidk marked 3 inline comments as done.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread David Kahles 
davidk updated this revision to Diff 22931.
davidk added a comment.


  Remove apparently unneeded version check

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8998?vs=22922=22931

BRANCH
  master

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

AFFECTED FILES
  find-modules/FindSeccomp.cmake

To: davidk, graesslin
Cc: #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread David Kahles 
davidk added inline comments.

INLINE COMMENTS

> graesslin wrote in FindSeccomp.cmake:50-55
> No idea, that's copy pasted from some other cmake modules.

Well, based on the fact that no other find-module includes such a check, I will 
remove it.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread Martin Flöser 
graesslin added inline comments.

INLINE COMMENTS

> davidk wrote in FindSeccomp.cmake:50-55
> I'm not sure about this. @graesslin  is this nessecarry? Most find-modules 
> don't check for the CMake version, and we don't do anything special here.

No idea, that's copy pasted from some other cmake modules.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread David Kahles 
davidk added inline comments.

INLINE COMMENTS

> FindSeccomp.cmake:50-55
> +if(CMAKE_VERSION VERSION_LESS 2.8.12)
> +message(FATAL_ERROR "CMake 2.8.12 is required by FindSeccomp.cmake")
> +endif()
> +if(CMAKE_MINIMUM_REQUIRED_VERSION VERSION_LESS 2.8.12)
> +message(AUTHOR_WARNING "Your project should require at least CMake 
> 2.8.12 to use FindSeccomp.cmake")
> +endif()

I'm not sure about this. @graesslin  is this nessecarry? Most find-modules 
don't check for the CMake version, and we don't do anything special here.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread David Kahles 
davidk added a reviewer: graesslin.

REPOSITORY
  R240 Extra CMake Modules

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

To: davidk, graesslin
Cc: #frameworks, #build_system

D8998: Add FindSeccomp to find-modules

2017-11-25 Thread David Kahles 
davidk created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: Build System, Frameworks.

REVISION SUMMARY
  This is copied from KScreenlocker, but will be utilized in Baloo too.

TEST PLAN
  - Autotests are working
  - KScreenlocker and Baloo build with seccomp enabled

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  find-modules/FindSeccomp.cmake

To: davidk
Cc: #frameworks, #build_system