On (21/11/15 15:43), Jakub Hrozek wrote:
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.
But it needn't be there. We do not have a strict requirement for polkit.
and Sumit mention that it's not required on rhel6. So it should not be in
buildroot.
You might check for feature but we should not fail if the pkcheck file
does not exist. Warning should be enough and configure script should continue.
>
> >+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.
But on the other hand
if we ship file in /usr then user might override
it in /etc/. So it might be better to not change anything.
LS