On Fri, Nov 20, 2015 at 08:50:50PM +0100, Lukas Slebodnik wrote:
On (20/11/15 18:58), Jakub Hrozek wrote:
>The attached patch tries to detect if the build platform has the rules.d
>polkit directory and only installs the file from contrib if so. As an
>additional improvement we might add a config option that specifies the
>directory directly, but I don't think it's needed at the moment.
>
>I ran a local mock build of epel-6 and fedora and those passed. Now the
>patch is in CI to check Debian.
>
>Sorry again for the careless review of the first patch.
>From 060b224356acf387eafa6f7daeca87ce71fabaa5 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Fri, 20 Nov 2015 18:54:13 +0100
>Subject: [PATCH] BUILD: Install polkit rules file only on platforms that
> support it
>
>---
> Makefile.am | 2 ++
> configure.ac | 8 ++++++++
> contrib/ci/deps.sh | 1 +
> contrib/sssd.spec.in | 1 +
> 4 files changed, 12 insertions(+)
>
>diff --git a/Makefile.am b/Makefile.am
>index
212440c9b174de10ca4b8481af350f06b090cc34..d6ca1cf1b79859cf4de6788ecdc306b614630a42 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -151,10 +151,12 @@ endif
> if HAVE_NSS
> sssdlibexec_PROGRAMS += p11_child
> if SSSD_USER
>+if HAVE_POLKIT_RULES_D
> polkit_rulesdir = $(datadir)/polkit-1/rules.d
> dist_polkit_rules_DATA = contrib/sssd-pcsc.rules
> endif
> endif
>+endif
>
> if BUILD_PAC_RESPONDER
> sssdlibexec_PROGRAMS += sssd_pac
>diff --git a/configure.ac b/configure.ac
>index
f7254c096659e7d8e395b4d0b0c71b5aa28d38f1..f68aef59b4ab48cc5e8fcac06925706f744cf4be 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -424,6 +424,14 @@ SSS_ENABLE_INTGCHECK_REQS
>
> AM_CONDITIONAL([HAVE_DEVSHM], [test -d /dev/shm])
>
>+# Check if we're building on a platform with new polkit scheme
>+AC_PATH_PROG([PKCHECK],[pkcheck])
a) there is a missing space after ','
OK, fixed locally.
b) binary pkcheck is unused. It would make sense to use it
if pkcheck was used for detecting version of polkit and version of
polkit to distinguish if /usr/share/polkit-1/rules.d/ is used in such version.
I would prefer to check for features (directory in this case) than version
numbers..
c) It should be sufficient to test if directory exists.
But you need to make sure the package that provides the directory is
there. And since pkcheck is the binary that decides if process is
authorized for action, it's integral part of polkit.
>+if test ! -x "$PKCHECK"; then
>+ AC_MSG_WARN([pkcheck not found, will not install polkit rules])
>+else
>+ AM_CONDITIONAL([HAVE_POLKIT_RULES_D], [test -d ${datadir}/polkit-1/rules.d ])
>+fi
> abs_build_dir=`pwd`
> AC_DEFINE_UNQUOTED([ABS_BUILD_DIR], ["$abs_build_dir"], [Absolute path to
the build directory])
> AC_SUBST([abs_builddir], $abs_build_dir)
>diff --git a/contrib/ci/deps.sh b/contrib/ci/deps.sh
>index
c9a8a6384a31c796a7b79e4f94d89f3e94ce6542..508c512b75d2d5a4e333fe657de5430f8e72c0c4 100644
>--- a/contrib/ci/deps.sh
>+++ b/contrib/ci/deps.sh
>@@ -114,6 +114,7 @@ if [[ "$DISTRO_BRANCH" == -debian-* ]]; then
> python-ldap
> ldap-utils
> slapd
>+ policykit-1
Debian (stable, testing, unstable) has old version of policykit which does not
support policy kit rules.
$ pkcheck --version
pkcheck version 0.105
OK, then it doesn't have the rulesdir. When debian upgrades to something
recent, we will detect the rulesdir, but we already pull in the package
that will provide the rulesdir.
> )
> DEPS_INTGCHECK_SATISFIED=true
> fi
>diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>index
cff77b29ec55788fa45c2fa948f15d3ed7a06897..698694686945a25ba551dcfa22eb1753bbc780ae 100644
>--- a/contrib/sssd.spec.in
>+++ b/contrib/sssd.spec.in
>@@ -160,6 +160,7 @@ BuildRequires: nfs-utils-lib-devel
>
> BuildRequires: samba4-devel
> BuildRequires: libsmbclient-devel
>+BuildRequires: polkit
>
> %description
> Provides a set of daemons to manage access to remote directories and
I have a slightly off topic question.
Why did we used /usr/share/polkit-1/rules.d/ and not /etc/polkit-1/rules.d/
for storing polkit rules?
According to
http://www.freedesktop.org/software/polkit/docs/latest/polkit.8.html
/etc is searched before /usr.
Maybe it would make sense to install to /etc if there was another
low-level software that might provide the rules, like systemd.