[PATCH] Build cifs plugin depending on the existence of cifsidmap.h

[Sumit Bose]

Hi,

with this patch it is not required anymore to have cifs-utils-devel
installed for the default build.

Fixes https://fedorahosted.org/sssd/ticket/2125

bye,
Sumit

[Jakub Hrozek]

On Fri, Nov 01, 2013 at 06:41:20PM +0100, Sumit Bose wrote:
>  Hi,
>  
>  with this patch it is not required anymore to have cifs-utils-devel
>  installed for the default build.
>  
>  Fixes https://fedorahosted.org/sssd/ticket/2125
>  
>  bye,
>  Sumit 
Hi,

I think it would also be nice to add explicit
"with_cifs_utils_plugin_option --enable-cifs-idmap-plugin" to
contrib/sssd.spec.in where available. That way, even if cifs-utils-devel
was renamed configure would error out (assuming we want the CIFS support
enabled in upstream RPMs by defaul, which I think we do and Simo's point
was mostly about building from source).

The configure option itself works nicely, I tested with and without
cifs-utils-devel, explicitly disabling or enabling the support.

[Lukas Slebodnik]

On (01/11/13 18:41), Sumit Bose wrote:
> Hi,
>
> with this patch it is not required anymore to have cifs-utils-devel
> installed for the default build.
>
> Fixes https://fedorahosted.org/sssd/ticket/2125
>
> bye,
> Sumit 
> From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00
> 2001
> From: Sumit Bose <sbose(a)redhat.com&gt;
> Date: Fri, 1 Nov 2013 18:33:32 +0100
> Subject: [PATCH] Build cifs plugin depending on the existence of cifsidmap.h
>
> To allow the "configure; make; make install" scheme on systems where
> cifsidmap.h is not available the cifs-utils plugin is only build if the
> include file is available and not build if it is not available.
>
> If the configure option --enable-cifs-idmap-plugin is used, configure
> will fail if cifsidmap.h is not available.
>
> Fixes https://fedorahosted.org/sssd/ticket/2125
> ---
>  src/external/cifsidmap.m4 | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
> index 53cb8b7..fbcbe65 100644
> --- a/src/external/cifsidmap.m4
> +++ b/src/external/cifsidmap.m4
> @@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>                [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>                                [do not build CIFS idmap plugin])],
>                [build_cifs_idmap_plugin=$enableval],
> -              [build_cifs_idmap_plugin=yes])
> +              [build_cifs_idmap_plugin=auto])
>  
> -AS_IF([test x$build_cifs_idmap_plugin = xyes],
> -      [AC_CHECK_HEADER([cifsidmap.h], [],
> -                       [AC_MSG_ERROR([you must have the cifsidmap header installed to
> build the idmap plugin])])
> -      ]) 
I have an idea. We can leave default value "yes" and extend AC_MSG_ERROR
message. For example:
"You can disable cifs plugin with parameter --disable-cifs-idmap-plugin"

Is it a bad idea?

LS

> +AC_CHECK_HEADER([cifsidmap.h],
> +                [],
> +                [AS_IF([test x$build_cifs_idmap_plugin = xyes],
> +                       [AC_MSG_ERROR([you must have the cifsidmap header installed to
> build the idmap plugin])],
> +                       [AC_MSG_NOTICE([cifsidmap.h not available CIFS idmap plugin will
> not be build])])])
>  
>  AM_CONDITIONAL([BUILD_CIFS_IDMAP_PLUGIN],
> -               [test x$build_cifs_idmap_plugin = xyes])
> +               [test x$ac_cv_header_cifsidmap_h = xyes -a \( x$build_cifs_idmap_plugin =
> xyes -o x$build_cifs_idmap_plugin = xauto \) ])
>  
>  AM_COND_IF([BUILD_CIFS_IDMAP_PLUGIN],
> -           [AC_DEFINE_UNQUOTED(HAVE_CIFS_IDMAP_PLUGIN, 1, [Build with cifs idmap
> plugin])])
> +           [AC_DEFINE_UNQUOTED(HAVE_CIFS_IDMAP_PLUGIN, 1, [Build with cifs idmap
> plugin])
> +            AC_MSG_NOTICE([Build with cifs idmap plugin])],
> +           [AC_MSG_NOTICE([Build without cifs idmap plugin])])
> -- 
> 1.8.3.1
>  
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Sumit Bose]

On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  On (01/11/13 18:41), Sumit Bose wrote:
>  >Hi,
>  >
>  >with this patch it is not required anymore to have cifs-utils-devel
>  >installed for the default build.
>  >
>  >Fixes https://fedorahosted.org/sssd/ticket/2125
>  >
>  >bye,
>  >Sumit
>  
>  >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00 2001
>  >From: Sumit Bose <sbose(a)redhat.com&gt;
>  >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  >Subject: [PATCH] Build cifs plugin depending on the existence of cifsidmap.h
>  >
>  >To allow the "configure; make; make install" scheme on systems where
>  >cifsidmap.h is not available the cifs-utils plugin is only build if the
>  >include file is available and not build if it is not available.
>  >
>  >If the configure option --enable-cifs-idmap-plugin is used, configure
>  >will fail if cifsidmap.h is not available.
>  >
>  >Fixes https://fedorahosted.org/sssd/ticket/2125
>  >---
>  > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  > 1 file changed, 10 insertions(+), 7 deletions(-)
>  >
>  >diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
>  >index 53cb8b7..fbcbe65 100644
>  >--- a/src/external/cifsidmap.m4
>  >+++ b/src/external/cifsidmap.m4
>  >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  >                               [do not build CIFS idmap plugin])],
>  >               [build_cifs_idmap_plugin=$enableval],
>  >-              [build_cifs_idmap_plugin=yes])
>  >+              [build_cifs_idmap_plugin=auto])
>  > 
>  >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  >-                       [AC_MSG_ERROR([you must have the cifsidmap header installed
> to build the idmap plugin])])
>  >-      ])
>  
>  I have an idea. We can leave default value "yes" and extend AC_MSG_ERROR
>  message. For example:
>  "You can disable cifs plugin with parameter --disable-cifs-idmap-plugin"
>  
>  Is it a bad idea? 
I would like to hear Simo's opinion on both suggestions, before doing
any changes.

bye,
Sumit

>  
>  LS
>  
>  >+AC_CHECK_HEADER([cifsidmap.h],
>  >+                [],
>  >+                [AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  >+                       [AC_MSG_ERROR([you must have the cifsidmap header installed
> to build the idmap plugin])],
>  >+                       [AC_MSG_NOTICE([cifsidmap.h not available CIFS idmap plugin
> will not be build])])])
>  > 
>  > AM_CONDITIONAL([BUILD_CIFS_IDMAP_PLUGIN],
>  >-               [test x$build_cifs_idmap_plugin = xyes])
>  >+               [test x$ac_cv_header_cifsidmap_h = xyes -a \(
> x$build_cifs_idmap_plugin = xyes -o x$build_cifs_idmap_plugin = xauto \) ])
>  > 
>  > AM_COND_IF([BUILD_CIFS_IDMAP_PLUGIN],
>  >-           [AC_DEFINE_UNQUOTED(HAVE_CIFS_IDMAP_PLUGIN, 1, [Build with cifs idmap
> plugin])])
>  >+           [AC_DEFINE_UNQUOTED(HAVE_CIFS_IDMAP_PLUGIN, 1, [Build with cifs idmap
> plugin])
>  >+            AC_MSG_NOTICE([Build with cifs idmap plugin])],
>  >+           [AC_MSG_NOTICE([Build without cifs idmap plugin])])
>  >-- 
>  >1.8.3.1
>  >
>  
>  >_______________________________________________
>  >sssd-devel mailing list
>  &gt;sssd-devel(a)lists.fedorahosted.org
>  >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>  
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Simo Sorce]

On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
>  On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  > On (01/11/13 18:41), Sumit Bose wrote:
>  > >Hi,
>  > >
>  > >with this patch it is not required anymore to have cifs-utils-devel
>  > >installed for the default build.
>  > >
>  > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > >
>  > >bye,
>  > >Sumit
>  > 
>  > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00 2001
>  > >From: Sumit Bose <sbose(a)redhat.com&gt;
>  > >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  > >Subject: [PATCH] Build cifs plugin depending on the existence of cifsidmap.h
>  > >
>  > >To allow the "configure; make; make install" scheme on systems where
>  > >cifsidmap.h is not available the cifs-utils plugin is only build if the
>  > >include file is available and not build if it is not available.
>  > >
>  > >If the configure option --enable-cifs-idmap-plugin is used, configure
>  > >will fail if cifsidmap.h is not available.
>  > >
>  > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > >---
>  > > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  > > 1 file changed, 10 insertions(+), 7 deletions(-)
>  > >
>  > >diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
>  > >index 53cb8b7..fbcbe65 100644
>  > >--- a/src/external/cifsidmap.m4
>  > >+++ b/src/external/cifsidmap.m4
>  > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  > >                               [do not build CIFS idmap plugin])],
>  > >               [build_cifs_idmap_plugin=$enableval],
>  > >-              [build_cifs_idmap_plugin=yes])
>  > >+              [build_cifs_idmap_plugin=auto])
>  > > 
>  > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  > >-                       [AC_MSG_ERROR([you must have the cifsidmap header
> installed to build the idmap plugin])])
>  > >-      ])
>  > 
>  > I have an idea. We can leave default value "yes" and extend AC_MSG_ERROR
>  > message. For example:
>  > "You can disable cifs plugin with parameter --disable-cifs-idmap-plugin"
>  > 
>  > Is it a bad idea?
>  
>  I would like to hear Simo's opinion on both suggestions, before doing
>  any changes. 
As long as it is possible to build without the idmap plugin I do not
really care what is the default, use your best judgment.
The problem is in that not being an option, not in what we use as
default.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

[Sumit Bose]

On Mon, Nov 04, 2013 at 10:36:56AM -0500, Simo Sorce wrote:
>  On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
>  > On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  > > On (01/11/13 18:41), Sumit Bose wrote:
>  > > >Hi,
>  > > >
>  > > >with this patch it is not required anymore to have cifs-utils-devel
>  > > >installed for the default build.
>  > > >
>  > > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > > >
>  > > >bye,
>  > > >Sumit
>  > > 
>  > > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00 2001
>  > > >From: Sumit Bose <sbose(a)redhat.com&gt;
>  > > >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  > > >Subject: [PATCH] Build cifs plugin depending on the existence of
> cifsidmap.h
>  > > >
>  > > >To allow the "configure; make; make install" scheme on systems
> where
>  > > >cifsidmap.h is not available the cifs-utils plugin is only build if the
>  > > >include file is available and not build if it is not available.
>  > > >
>  > > >If the configure option --enable-cifs-idmap-plugin is used, configure
>  > > >will fail if cifsidmap.h is not available.
>  > > >
>  > > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > > >---
>  > > > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  > > > 1 file changed, 10 insertions(+), 7 deletions(-)
>  > > >
>  > > >diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
>  > > >index 53cb8b7..fbcbe65 100644
>  > > >--- a/src/external/cifsidmap.m4
>  > > >+++ b/src/external/cifsidmap.m4
>  > > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  > > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  > > >                               [do not build CIFS idmap plugin])],
>  > > >               [build_cifs_idmap_plugin=$enableval],
>  > > >-              [build_cifs_idmap_plugin=yes])
>  > > >+              [build_cifs_idmap_plugin=auto])
>  > > > 
>  > > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  > > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  > > >-                       [AC_MSG_ERROR([you must have the cifsidmap header
> installed to build the idmap plugin])])
>  > > >-      ])
>  > > 
>  > > I have an idea. We can leave default value "yes" and extend
> AC_MSG_ERROR
>  > > message. For example:
>  > > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
>  > > 
>  > > Is it a bad idea?
>  > 
>  > I would like to hear Simo's opinion on both suggestions, before doing
>  > any changes.
>  
>  As long as it is possible to build without the idmap plugin I do not
>  really care what is the default, use your best judgment.
>  The problem is in that not being an option, not in what we use as
>  default. 
Configure options --disable-cifs-idmap-plugin and
--enable-cifs-idmap-plugin are already available, so no change is
needed?

bye,
Sumit

>  
>  Simo.
>  
>  -- 
>  Simo Sorce * Red Hat, Inc * New York
>

[Simo Sorce]

On Mon, 2013-11-04 at 16:44 +0100, Sumit Bose wrote:
>  On Mon, Nov 04, 2013 at 10:36:56AM -0500, Simo Sorce wrote:
>  > On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
>  > > On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  > > > On (01/11/13 18:41), Sumit Bose wrote:
>  > > > >Hi,
>  > > > >
>  > > > >with this patch it is not required anymore to have cifs-utils-devel
>  > > > >installed for the default build.
>  > > > >
>  > > > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > > > >
>  > > > >bye,
>  > > > >Sumit
>  > > > 
>  > > > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00
> 2001
>  > > > >From: Sumit Bose <sbose(a)redhat.com&gt;
>  > > > >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  > > > >Subject: [PATCH] Build cifs plugin depending on the existence of
> cifsidmap.h
>  > > > >
>  > > > >To allow the "configure; make; make install" scheme on
> systems where
>  > > > >cifsidmap.h is not available the cifs-utils plugin is only build if
> the
>  > > > >include file is available and not build if it is not available.
>  > > > >
>  > > > >If the configure option --enable-cifs-idmap-plugin is used, configure
>  > > > >will fail if cifsidmap.h is not available.
>  > > > >
>  > > > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > > > >---
>  > > > > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  > > > > 1 file changed, 10 insertions(+), 7 deletions(-)
>  > > > >
>  > > > >diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
>  > > > >index 53cb8b7..fbcbe65 100644
>  > > > >--- a/src/external/cifsidmap.m4
>  > > > >+++ b/src/external/cifsidmap.m4
>  > > > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  > > > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  > > > >                               [do not build CIFS idmap plugin])],
>  > > > >               [build_cifs_idmap_plugin=$enableval],
>  > > > >-              [build_cifs_idmap_plugin=yes])
>  > > > >+              [build_cifs_idmap_plugin=auto])
>  > > > > 
>  > > > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  > > > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  > > > >-                       [AC_MSG_ERROR([you must have the cifsidmap
> header installed to build the idmap plugin])])
>  > > > >-      ])
>  > > > 
>  > > > I have an idea. We can leave default value "yes" and extend
> AC_MSG_ERROR
>  > > > message. For example:
>  > > > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
>  > > > 
>  > > > Is it a bad idea?
>  > > 
>  > > I would like to hear Simo's opinion on both suggestions, before doing
>  > > any changes.
>  > 
>  > As long as it is possible to build without the idmap plugin I do not
>  > really care what is the default, use your best judgment.
>  > The problem is in that not being an option, not in what we use as
>  > default.
>  
>  Configure options --disable-cifs-idmap-plugin and
>  --enable-cifs-idmap-plugin are already available, so no change is
>  needed? 
When I opened the bug they were not ...

Does --disable-cifs-idmap-plugin allow you to build if you do not have
the required headers ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

[Lukas Slebodnik]

On (04/11/13 11:51), Simo Sorce wrote:
> On Mon, 2013-11-04 at 16:44 +0100, Sumit Bose wrote:
> > On Mon, Nov 04, 2013 at 10:36:56AM -0500, Simo Sorce wrote:
> > > On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
> > > > On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
> > > > > On (01/11/13 18:41), Sumit Bose wrote:
> > > > > >Hi,
> > > > > >
> > > > > >with this patch it is not required anymore to have
> cifs-utils-devel
> > > > > >installed for the default build.
> > > > > >
> > > > > >Fixes https://fedorahosted.org/sssd/ticket/2125
> > > > > >
> > > > > >bye,
> > > > > >Sumit
> > > > > 
> > > > > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00
> 2001
> > > > > >From: Sumit Bose <sbose(a)redhat.com&gt;
> > > > > >Date: Fri, 1 Nov 2013 18:33:32 +0100
> > > > > >Subject: [PATCH] Build cifs plugin depending on the existence of
> cifsidmap.h
> > > > > >
> > > > > >To allow the "configure; make; make install" scheme on
> systems where
> > > > > >cifsidmap.h is not available the cifs-utils plugin is only build
> if the
> > > > > >include file is available and not build if it is not available.
> > > > > >
> > > > > >If the configure option --enable-cifs-idmap-plugin is used,
> configure
> > > > > >will fail if cifsidmap.h is not available.
> > > > > >
> > > > > >Fixes https://fedorahosted.org/sssd/ticket/2125
> > > > > >---
> > > > > > src/external/cifsidmap.m4 | 17 ++++++++++-------
> > > > > > 1 file changed, 10 insertions(+), 7 deletions(-)
> > > > > >
> > > > > >diff --git a/src/external/cifsidmap.m4
> b/src/external/cifsidmap.m4
> > > > > >index 53cb8b7..fbcbe65 100644
> > > > > >--- a/src/external/cifsidmap.m4
> > > > > >+++ b/src/external/cifsidmap.m4
> > > > > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
> > > > > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
> > > > > >                               [do not build CIFS idmap
> plugin])],
> > > > > >               [build_cifs_idmap_plugin=$enableval],
> > > > > >-              [build_cifs_idmap_plugin=yes])
> > > > > >+              [build_cifs_idmap_plugin=auto])
> > > > > > 
> > > > > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
> > > > > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
> > > > > >-                       [AC_MSG_ERROR([you must have the cifsidmap
> header installed to build the idmap plugin])])
> > > > > >-      ])
> > > > > 
> > > > > I have an idea. We can leave default value "yes" and extend
> AC_MSG_ERROR
> > > > > message. For example:
> > > > > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
> > > > > 
> > > > > Is it a bad idea?
> > > > 
> > > > I would like to hear Simo's opinion on both suggestions, before doing
> > > > any changes.
> > > 
> > > As long as it is possible to build without the idmap plugin I do not
> > > really care what is the default, use your best judgment.
> > > The problem is in that not being an option, not in what we use as
> > > default.
> > 
> > Configure options --disable-cifs-idmap-plugin and
> > --enable-cifs-idmap-plugin are already available, so no change is
> > needed?
>
> When I opened the bug they were not ... 
They *were*.

Ticket was opened "2013-10-22T00:21:26 02:00" and patches were commited
few days ago.

commit c3b1e99007d5f0069fbbd32a3df799d71ea4882e
Author:     Sumit Bose <sbose(a)redhat.com&gt;
AuthorDate: Mon Oct 14 12:47:59 2013 +0200
Commit:     Sumit Bose <sbose(a)redhat.com&gt;
CommitDate: Tue Oct 15 11:36:37 2013 +0200

    Spec file changes for cifs-utils plugin

commit af4ffe1001adcc0a96897e426d26444f07af9aa1
Author:     Benjamin Franzke <benjaminfranzke(a)googlemail.com&gt;
AuthorDate: Thu Sep 26 10:27:33 2013 +0200
Commit:     Sumit Bose <sbose(a)redhat.com&gt;
CommitDate: Tue Oct 15 11:36:32 2013 +0200

    Add CIFS idmap plugin

    https://fedorahosted.org/sssd/ticket/1534

> Does --disable-cifs-idmap-plugin allow you to build if you do not have
> the required headers ? 
Yes, because RHEL6 does not have header file cifsidmap.h

--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -41,6 +41,12 @@
 %define _hardened_build 1
 %endif

+%if (0%{?fedora} >= 17 || 0%{?rhel} >= 7)
+    %global with_cifs_utils_plugin 1
+%else
+    %global with_cifs_utils_plugin_option --disable-cifs-idmap-plugin
+%endif
+

LS

[Simo Sorce]

On Mon, 2013-11-04 at 18:07 +0100, Lukas Slebodnik wrote:
>  On (04/11/13 11:51), Simo Sorce wrote:
>  >On Mon, 2013-11-04 at 16:44 +0100, Sumit Bose wrote:
>  >> On Mon, Nov 04, 2013 at 10:36:56AM -0500, Simo Sorce wrote:
>  >> > On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
>  >> > > On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  >> > > > On (01/11/13 18:41), Sumit Bose wrote:
>  >> > > > >Hi,
>  >> > > > >
>  >> > > > >with this patch it is not required anymore to have
> cifs-utils-devel
>  >> > > > >installed for the default build.
>  >> > > > >
>  >> > > > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  >> > > > >
>  >> > > > >bye,
>  >> > > > >Sumit
>  >> > > > 
>  >> > > > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17
> 00:00:00 2001
>  >> > > > >From: Sumit Bose <sbose(a)redhat.com&gt;
>  >> > > > >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  >> > > > >Subject: [PATCH] Build cifs plugin depending on the existence
> of cifsidmap.h
>  >> > > > >
>  >> > > > >To allow the "configure; make; make install" scheme
> on systems where
>  >> > > > >cifsidmap.h is not available the cifs-utils plugin is only
> build if the
>  >> > > > >include file is available and not build if it is not
> available.
>  >> > > > >
>  >> > > > >If the configure option --enable-cifs-idmap-plugin is used,
> configure
>  >> > > > >will fail if cifsidmap.h is not available.
>  >> > > > >
>  >> > > > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  >> > > > >---
>  >> > > > > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  >> > > > > 1 file changed, 10 insertions(+), 7 deletions(-)
>  >> > > > >
>  >> > > > >diff --git a/src/external/cifsidmap.m4
> b/src/external/cifsidmap.m4
>  >> > > > >index 53cb8b7..fbcbe65 100644
>  >> > > > >--- a/src/external/cifsidmap.m4
>  >> > > > >+++ b/src/external/cifsidmap.m4
>  >> > > > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  >> > > > >              
> [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  >> > > > >                               [do not build CIFS idmap
> plugin])],
>  >> > > > >               [build_cifs_idmap_plugin=$enableval],
>  >> > > > >-              [build_cifs_idmap_plugin=yes])
>  >> > > > >+              [build_cifs_idmap_plugin=auto])
>  >> > > > > 
>  >> > > > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  >> > > > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  >> > > > >-                       [AC_MSG_ERROR([you must have the
> cifsidmap header installed to build the idmap plugin])])
>  >> > > > >-      ])
>  >> > > > 
>  >> > > > I have an idea. We can leave default value "yes" and
> extend AC_MSG_ERROR
>  >> > > > message. For example:
>  >> > > > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
>  >> > > > 
>  >> > > > Is it a bad idea?
>  >> > > 
>  >> > > I would like to hear Simo's opinion on both suggestions, before
> doing
>  >> > > any changes.
>  >> > 
>  >> > As long as it is possible to build without the idmap plugin I do not
>  >> > really care what is the default, use your best judgment.
>  >> > The problem is in that not being an option, not in what we use as
>  >> > default.
>  >> 
>  >> Configure options --disable-cifs-idmap-plugin and
>  >> --enable-cifs-idmap-plugin are already available, so no change is
>  >> needed?
>  >
>  >When I opened the bug they were not ...
>  They *were*. 
Uhmm odd, I remember I had updated master and didn't see a way out, my
bad, I must have missed to something.

>  Ticket was opened "2013-10-22T00:21:26 02:00" and patches
> were commited
>  few days ago.
>  
>  commit c3b1e99007d5f0069fbbd32a3df799d71ea4882e
>  Author:     Sumit Bose <sbose(a)redhat.com&gt;
>  AuthorDate: Mon Oct 14 12:47:59 2013 +0200
>  Commit:     Sumit Bose <sbose(a)redhat.com&gt;
>  CommitDate: Tue Oct 15 11:36:37 2013 +0200
>  
>      Spec file changes for cifs-utils plugin
>  
>  commit af4ffe1001adcc0a96897e426d26444f07af9aa1
>  Author:     Benjamin Franzke <benjaminfranzke(a)googlemail.com&gt;
>  AuthorDate: Thu Sep 26 10:27:33 2013 +0200
>  Commit:     Sumit Bose <sbose(a)redhat.com&gt;
>  CommitDate: Tue Oct 15 11:36:32 2013 +0200
>  
>      Add CIFS idmap plugin
>  
>      https://fedorahosted.org/sssd/ticket/1534
>  
>  >
>  >Does --disable-cifs-idmap-plugin allow you to build if you do not have
>  >the required headers ?
>  Yes, because RHEL6 does not have header file cifsidmap.h 
I think we are all set then ?

Simo.

>  --- a/contrib/sssd.spec.in
>  +++ b/contrib/sssd.spec.in
>  @@ -41,6 +41,12 @@
>   %define _hardened_build 1
>   %endif
>  
>  +%if (0%{?fedora} >= 17 || 0%{?rhel} >= 7)
>  +    %global with_cifs_utils_plugin 1
>  +%else
>  +    %global with_cifs_utils_plugin_option --disable-cifs-idmap-plugin
>  +%endif
>  +
>  
>  LS
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/sssd-devel 

-- 
Simo Sorce * Red Hat, Inc * New York

[Lukas Slebodnik]

On (04/11/13 10:36), Simo Sorce wrote:
> On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
> > On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
> > > On (01/11/13 18:41), Sumit Bose wrote:
> > > >Hi,
> > > >
> > > >with this patch it is not required anymore to have cifs-utils-devel
> > > >installed for the default build.
> > > >
> > > >Fixes https://fedorahosted.org/sssd/ticket/2125
> > > >
> > > >bye,
> > > >Sumit
> > > 
> > > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00 2001
> > > >From: Sumit Bose <sbose(a)redhat.com&gt;
> > > >Date: Fri, 1 Nov 2013 18:33:32 +0100
> > > >Subject: [PATCH] Build cifs plugin depending on the existence of
> cifsidmap.h
> > > >
> > > >To allow the "configure; make; make install" scheme on systems
> where
> > > >cifsidmap.h is not available the cifs-utils plugin is only build if the
> > > >include file is available and not build if it is not available.
> > > >
> > > >If the configure option --enable-cifs-idmap-plugin is used, configure
> > > >will fail if cifsidmap.h is not available.
> > > >
> > > >Fixes https://fedorahosted.org/sssd/ticket/2125
> > > >---
> > > > src/external/cifsidmap.m4 | 17 ++++++++++-------
> > > > 1 file changed, 10 insertions(+), 7 deletions(-)
> > > >
> > > >diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
> > > >index 53cb8b7..fbcbe65 100644
> > > >--- a/src/external/cifsidmap.m4
> > > >+++ b/src/external/cifsidmap.m4
> > > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
> > > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
> > > >                               [do not build CIFS idmap plugin])],
> > > >               [build_cifs_idmap_plugin=$enableval],
> > > >-              [build_cifs_idmap_plugin=yes])
> > > >+              [build_cifs_idmap_plugin=auto])
> > > > 
> > > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
> > > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
> > > >-                       [AC_MSG_ERROR([you must have the cifsidmap header
> installed to build the idmap plugin])])
> > > >-      ])
> > > 
> > > I have an idea. We can leave default value "yes" and extend
> AC_MSG_ERROR
> > > message. For example:
> > > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
> > > 
> > > Is it a bad idea?
> > 
> > I would like to hear Simo's opinion on both suggestions, before doing
> > any changes.
>
> As long as it is possible to build without the idmap plugin I do not
> really care what is the default, use your best judgment.
> The problem is in that not being an option, not in what we use as
> default.
>
> Simo.
>
> -- 
> Simo Sorce * Red Hat, Inc * New York
>  
It was not clear that cifs-idmap plugin is optional feature of sssd.
At least, simo and user from sssd-users were confused.
https://lists.fedorahosted.org/pipermail/sssd-users/2013-October/001137.html

Attached patch changes only AC_MSG_ERROR.

LS

[Sumit Bose]

On Mon, Nov 11, 2013 at 05:29:17PM +0100, Lukas Slebodnik wrote:
>  On (04/11/13 10:36), Simo Sorce wrote:
>  >On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
>  >> On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  >> > On (01/11/13 18:41), Sumit Bose wrote:
>  >> > >Hi,
>  >> > >
>  >> > >with this patch it is not required anymore to have cifs-utils-devel
>  >> > >installed for the default build.
>  >> > >
>  >> > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  >> > >
>  >> > >bye,
>  >> > >Sumit
>  >> > 
>  >> > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00 2001
>  >> > >From: Sumit Bose <sbose(a)redhat.com&gt;
>  >> > >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  >> > >Subject: [PATCH] Build cifs plugin depending on the existence of
> cifsidmap.h
>  >> > >
>  >> > >To allow the "configure; make; make install" scheme on
> systems where
>  >> > >cifsidmap.h is not available the cifs-utils plugin is only build if
> the
>  >> > >include file is available and not build if it is not available.
>  >> > >
>  >> > >If the configure option --enable-cifs-idmap-plugin is used, configure
>  >> > >will fail if cifsidmap.h is not available.
>  >> > >
>  >> > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  >> > >---
>  >> > > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  >> > > 1 file changed, 10 insertions(+), 7 deletions(-)
>  >> > >
>  >> > >diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
>  >> > >index 53cb8b7..fbcbe65 100644
>  >> > >--- a/src/external/cifsidmap.m4
>  >> > >+++ b/src/external/cifsidmap.m4
>  >> > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  >> > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  >> > >                               [do not build CIFS idmap plugin])],
>  >> > >               [build_cifs_idmap_plugin=$enableval],
>  >> > >-              [build_cifs_idmap_plugin=yes])
>  >> > >+              [build_cifs_idmap_plugin=auto])
>  >> > > 
>  >> > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  >> > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  >> > >-                       [AC_MSG_ERROR([you must have the cifsidmap
> header installed to build the idmap plugin])])
>  >> > >-      ])
>  >> > 
>  >> > I have an idea. We can leave default value "yes" and extend
> AC_MSG_ERROR
>  >> > message. For example:
>  >> > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
>  >> > 
>  >> > Is it a bad idea?
>  >> 
>  >> I would like to hear Simo's opinion on both suggestions, before doing
>  >> any changes.
>  >
>  >As long as it is possible to build without the idmap plugin I do not
>  >really care what is the default, use your best judgment.
>  >The problem is in that not being an option, not in what we use as
>  >default.
>  >
>  >Simo.
>  >
>  >-- 
>  >Simo Sorce * Red Hat, Inc * New York
>  >
>  
>  It was not clear that cifs-idmap plugin is optional feature of sssd.
>  At least, simo and user from sssd-users were confused.
>  https://lists.fedorahosted.org/pipermail/sssd-users/2013-October/001137.html
>  
>  Attached patch changes only AC_MSG_ERROR. 
Works for me, ACK.

bye,
Sumit

>  
>  LS

[Jakub Hrozek]

On Tue, Nov 12, 2013 at 11:12:36AM +0100, Sumit Bose wrote:
>  On Mon, Nov 11, 2013 at 05:29:17PM +0100, Lukas Slebodnik wrote:
>  > On (04/11/13 10:36), Simo Sorce wrote:
>  > >On Mon, 2013-11-04 at 16:21 +0100, Sumit Bose wrote:
>  > >> On Mon, Nov 04, 2013 at 12:05:15PM +0100, Lukas Slebodnik wrote:
>  > >> > On (01/11/13 18:41), Sumit Bose wrote:
>  > >> > >Hi,
>  > >> > >
>  > >> > >with this patch it is not required anymore to have
> cifs-utils-devel
>  > >> > >installed for the default build.
>  > >> > >
>  > >> > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > >> > >
>  > >> > >bye,
>  > >> > >Sumit
>  > >> > 
>  > >> > >From 35f5932946d67a943146411a336afac34a78fb90 Mon Sep 17 00:00:00
> 2001
>  > >> > >From: Sumit Bose <sbose(a)redhat.com&gt;
>  > >> > >Date: Fri, 1 Nov 2013 18:33:32 +0100
>  > >> > >Subject: [PATCH] Build cifs plugin depending on the existence of
> cifsidmap.h
>  > >> > >
>  > >> > >To allow the "configure; make; make install" scheme on
> systems where
>  > >> > >cifsidmap.h is not available the cifs-utils plugin is only build
> if the
>  > >> > >include file is available and not build if it is not available.
>  > >> > >
>  > >> > >If the configure option --enable-cifs-idmap-plugin is used,
> configure
>  > >> > >will fail if cifsidmap.h is not available.
>  > >> > >
>  > >> > >Fixes https://fedorahosted.org/sssd/ticket/2125
>  > >> > >---
>  > >> > > src/external/cifsidmap.m4 | 17 ++++++++++-------
>  > >> > > 1 file changed, 10 insertions(+), 7 deletions(-)
>  > >> > >
>  > >> > >diff --git a/src/external/cifsidmap.m4
> b/src/external/cifsidmap.m4
>  > >> > >index 53cb8b7..fbcbe65 100644
>  > >> > >--- a/src/external/cifsidmap.m4
>  > >> > >+++ b/src/external/cifsidmap.m4
>  > >> > >@@ -2,15 +2,18 @@ AC_ARG_ENABLE([cifs-idmap-plugin],
>  > >> > >               [AS_HELP_STRING([--disable-cifs-idmap-plugin],
>  > >> > >                               [do not build CIFS idmap
> plugin])],
>  > >> > >               [build_cifs_idmap_plugin=$enableval],
>  > >> > >-              [build_cifs_idmap_plugin=yes])
>  > >> > >+              [build_cifs_idmap_plugin=auto])
>  > >> > > 
>  > >> > >-AS_IF([test x$build_cifs_idmap_plugin = xyes],
>  > >> > >-      [AC_CHECK_HEADER([cifsidmap.h], [],
>  > >> > >-                       [AC_MSG_ERROR([you must have the cifsidmap
> header installed to build the idmap plugin])])
>  > >> > >-      ])
>  > >> > 
>  > >> > I have an idea. We can leave default value "yes" and extend
> AC_MSG_ERROR
>  > >> > message. For example:
>  > >> > "You can disable cifs plugin with parameter
> --disable-cifs-idmap-plugin"
>  > >> > 
>  > >> > Is it a bad idea?
>  > >> 
>  > >> I would like to hear Simo's opinion on both suggestions, before doing
>  > >> any changes.
>  > >
>  > >As long as it is possible to build without the idmap plugin I do not
>  > >really care what is the default, use your best judgment.
>  > >The problem is in that not being an option, not in what we use as
>  > >default.
>  > >
>  > >Simo.
>  > >
>  > >-- 
>  > >Simo Sorce * Red Hat, Inc * New York
>  > >
>  > 
>  > It was not clear that cifs-idmap plugin is optional feature of sssd.
>  > At least, simo and user from sssd-users were confused.
>  > https://lists.fedorahosted.org/pipermail/sssd-users/2013-October/001137.html
>  > 
>  > Attached patch changes only AC_MSG_ERROR.
>  
>  Works for me, ACK.
>  
>  bye,
>  Sumit 
Pushed to master.