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 <[email protected]>
>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
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to