On (23/11/15 15:12), Jakub Hrozek wrote:
On Mon, Nov 23, 2015 at 09:18:47AM +0100, Jakub Hrozek wrote:
> On Mon, Nov 23, 2015 at 09:10:59AM +0100, Lukas Slebodnik wrote:
> > On (23/11/15 09:07), Jakub Hrozek wrote:
> > >On Mon, Nov 23, 2015 at 08:09:29AM +0100, Lukas Slebodnik wrote:
> > >> 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.
> > >
> > >So how would you propose to check if the directory exists?
> > If binary pkcheck does not exist then the directory should not exist as well.
> > Write warning with explanation that pk11_child might not work continue.
>
> OK, I understand the proposal now, thanks.
Or at least I hope I understand, see the attached patch..pkexec is still
checkef for in the buildroot, but if it's not there, the directory check
is silently skipped.
CI passed with this patch:
http://sssd-ci.duckdns.org/logs/job/33/80/summary.html
From 68dcc8d06cffeb551c4291b3b8ac07ae72535081 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 | 11 +++++++++++
contrib/ci/deps.sh | 1 +
contrib/sssd.spec.in | 1 +
4 files changed, 15 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..ba326ffba4f95fb1914037455ee95f7f3de3a6a9
100644
--- a/configure.ac
+++ b/configure.ac
@@ -424,6 +424,17 @@ 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], [false])
^
^^^^^^^
missing space :-) we do not need any default value.
+if test -x "$PKCHECK"; then
+ if test -d ${datadir}/polkit-1/rules.d; then
previous two conditions can be
combined
if test -x "$PKCHECK" -a -d "${datadir}/polkit-1/rules.d"; then
+ HAVE_POLKIT_RULES_D=1
+ fi
+else
+ AC_MSG_WARN([pkcheck not found, will not install polkit rules])
This branch
will be executed after combining the conditions.
So it will be good to change text.
e.g.
"Cannot install polkit rules to ..."
+fi
+AM_CONDITIONAL([HAVE_POLKIT_RULES_D], [test x$HAVE_POLKIT_RULES_D != x])
+
but there is a big problem with this part.
It does not work for following case:
./configure --prefix=/usr
becuase "${datadir}" expands to ${datarootdir}"
which expands to "${prefix}/share" which expads to "/usr/prefix"
It works in mock build because configure script was called with
--datadir=/usr/share
I'm not sure what is the wight solution of this problem.
A) We can just print a warning that *string* "${datarootdir}//polkit-1/rules.d"
if not a directory and configure script shoudl be called with correct argument
--datadir.
B) we can try to eval "${datadir}" in loop. Which might be problematic as well.
sss_datadir=${datadir};
while test ! -d "${sss_datadir}" ; do
eval sss_datadir=${sss_datadir}
done
C) or directly test for "/usr/share/polkit-1/rules.d"
but it could be overrriden with configure argument as other paths
e.g. --with-init-dir (default /etc/rc.d/init.d)
--with-environment-file (default /etc/sysconfig/sssd)
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
)
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
We needn't install polkit on centos6 and we need at least
polkit >= 106.
It was the first version where rules in javascript was added.
%if (0%{?fedora} || 0%{?rhel} >= 7)
BuildRequires: polkit >= 106
%endif
LS