[PATCH] Split the providers into separate subpackages

[Jakub Hrozek]

Hi,

the attached patch splits the previously monolithic sssd package into
sssd-common that contains the deamon and the responders and per-provider
packages such as sssd-ldap or sssd-ipa.

This split would benefit two parties:
    1) security auditors who are often trying to find the smallest package
    set including dependencies needed for the package to function. They
    would be able to i.e. install sssd-ldap and not bother about
    sssd-ipa or sssd-ad pulling in more dependencies.
    2) 3rd party programs such as realmd or authconfig that would only
    be able to require or install on demand the needed packages.

The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must b
applied on the two specfile patches I sent earlier (the thread subject
included libsss_sudo).

[Ondrej Kos]

On 10/11/2012 01:14 PM, Jakub Hrozek wrote:
>  Hi,
>
>  the attached patch splits the previously monolithic sssd package into
>  sssd-common that contains the deamon and the responders and per-provider
>  packages such as sssd-ldap or sssd-ipa.
>
>  This split would benefit two parties:
>       1) security auditors who are often trying to find the smallest package
>       set including dependencies needed for the package to function. They
>       would be able to i.e. install sssd-ldap and not bother about
>       sssd-ipa or sssd-ad pulling in more dependencies.
>       2) 3rd party programs such as realmd or authconfig that would only
>       be able to require or install on demand the needed packages.
>
>  The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must b
>  applied on the two specfile patches I sent earlier (the thread subject
>  included libsss_sudo).
>
>
>
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>  
Ack

O.

-- 
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech

phone: [phone]
cell:  [phone]
ext:   82-62558
loc:   1/5C Brno 1 office
irc:   okos @ #brno

[Simo Sorce]

On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
>  
>  Hi,
>  
>  the attached patch splits the previously monolithic sssd package into
>  sssd-common that contains the deamon and the responders and
>  per-provider
>  packages such as sssd-ldap or sssd-ipa.
>  
>  This split would benefit two parties:
>      1) security auditors who are often trying to find the smallest
>  package
>      set including dependencies needed for the package to function.
>  They
>      would be able to i.e. install sssd-ldap and not bother about
>      sssd-ipa or sssd-ad pulling in more dependencies.
>      2) 3rd party programs such as realmd or authconfig that would only
>      be able to require or install on demand the needed packages.
>  
>  The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
>  b
>  applied on the two specfile patches I sent earlier (the thread subject
>  included libsss_sudo). 
Questions inline.

>  
>  
>  
>  
>  
>  
>  plain text
>  document
>  attachment
>  (0001-Split-the-providers-into-separate-subpackages.patch)
>  
>  From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
>  From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
>  Date: Fri, 28 Sep 2012 09:21:18 +0200
>  Subject: [PATCH] Split the providers into separate subpackages
>  
>  ---
>   contrib/sssd.spec.in | 145
>  ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 115 insertions(+), 30 deletions(-)
>  
>  diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  index
>  e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437
> 100644
>  --- a/contrib/sssd.spec.in
>  +++ b/contrib/sssd.spec.in
>  @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
>  %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>   Patch0001:  sssd-1.9-man-change-default-ccache.patch
>   
>   ### Dependencies ###
>  -
>  -Requires: libldb >= 0.9.3
>  -Requires: libtdb >= 1.1.3
>  +Conflicts: sssd < %{version}-%{release}
>   Requires: sssd-client%{?_isa} = %{version}-%{release}
>  -Requires: libipa_hbac = %{version}-%{release}
>  -Requires: libsss_idmap = %{version}-%{release}
>  -Requires: cyrus-sasl-gssapi
>  -Requires: keyutils-libs
>  -Requires(post): initscripts chkconfig
>  -Requires(preun):  initscripts chkconfig
>  -Requires(postun): initscripts chkconfig
>  +Requires: sssd-common = %{version}-%{release}
>  +Requires: sssd-ldap = %{version}-%{release}
>  +Requires: sssd-krb5 = %{version}-%{release}
>  +Requires: sssd-ipa = %{version}-%{release}
>  +Requires: sssd-ad = %{version}-%{release} 

Doesn't this set of requires makes the split useless ?
If I read it correctly it means sssd will require all subpackages anyway
so you cannot pick and choose to install only one as you say the purpose
is in the mail message.

>   %global servicename sssd
>   %global sssdstatedir %{_localstatedir}/lib/sss
>  @@ -126,6 +122,21 @@ the system and a pluggable backend system to
>  connect to multiple different
>   account sources. It is also the basis to provide client auditing and
>  policy
>   services for projects like FreeIPA.
>   
>  +%package common
>  +Summary: Common files for the SSSD
>  +Group: Applications/System
>  +License: GPLv3+
>  +Requires: libldb >= 0.9.3
>  +Requires: libtdb >= 1.1.3
>  +Requires: sssd-client%{?_isa} = %{version}-%{release}
>  +Requires(post): initscripts chkconfig
>  +Requires(preun):  initscripts chkconfig
>  +Requires(postun): initscripts chkconfig
>  +Conflicts: sssd < %{version}-%{release}
>  +
>  +%description common
>  +Common files for the SSSD.
>  +
>   %package client
>   Summary: SSSD Client libraries for NSS and PAM
>   Group: Applications/System
>  @@ -141,7 +152,7 @@ service.
>   Summary: Userspace tools for use with the SSSD
>   Group: Applications/System
>   License: GPLv3+
>  -Requires: sssd = %{version}-%{release}
>  +Requires: sssd-common = %{version}-%{release}
>   
>   %description tools
>   Provides userspace tools for manipulating users, groups, and nested
>  groups in
>  @@ -153,6 +164,61 @@ Also provides several other administrative tools:
>       * sss_seed which pre-creates a user entry for use in kickstarts
>       * sss_obfuscate for generating an obfuscated LDAP password
>   
>  +%package ldap
>  +Summary: The LDAP back end of the SSSD
>  +Group: Applications/System
>  +License: GPLv3+
>  +Conflicts: sssd < %{version}-%{release}
>  +Requires: cyrus-sasl-gssapi
>  +Requires: sssd-common = %{version}-%{release}
>  +Requires: libsss_idmap = %{version}-%{release}
>  +
>  +%description ldap
>  +Provides the LDAP back end that the SSSD can utilize to fetch
>  identity data
>  +from and authenticate against an LDAP server.
>  +
>  +%package krb5
>  +Summary: The Kerberos authentication back end for the SSSD
>  +Group: Applications/System
>  +License: GPLv3+
>  +Conflicts: sssd < %{version}-%{release}
>  +Requires: cyrus-sasl-gssapi
>  +Requires: sssd-common = %{version}-%{release}
>  +
>  +%description krb5
>  +Provides the Kerberos back end that the SSSD can utilize authenticate
>  +against a Kerberos server.
>  +
>  +%package ipa
>  +Summary: The IPA back end of the SSSD
>  +Group: Applications/System
>  +License: GPLv3+
>  +Conflicts: sssd < %{version}-%{release}
>  +Requires: sssd-common = %{version}-%{release}
>  +Requires: sssd-ldap = %{version}-%{release}
>  +Requires: sssd-krb5 = %{version}-%{release}
>  +Requires: libipa_hbac = %{version}-%{release}
>  +Requires: libsss_idmap = %{version}-%{release}
>  +Requires: bind-utils 
Does the ipa provider really need the sssd-ldap and sssd-krb5
subpackages ?
IIRC we statically compile the ldap and krb5 packages bits we need in
the ipa provider.
If you change this you probably want a require on cyrus-sasl-gssapi
here.

(if it is just for the ldap and krb child processes shouldn't we simply
keep those binaries in the sssd or sssd-common package ?)


>  +%description ipa
>  +Provides the IPA back end that the SSSD can utilize to fetch identity
>  data
>  +from and authenticate against an IPA server.
>  +
>  +%package ad
>  +Summary: The AD back end of the SSSD
>  +Group: Applications/System
>  +License: GPLv3+
>  +Conflicts: sssd < %{version}-%{release}
>  +Requires: sssd-common = %{version}-%{release}
>  +Requires: sssd-ldap = %{version}-%{release}
>  +Requires: sssd-krb5 = %{version}-%{release}
>  +Requires: libsss_idmap = %{version}-%{release} 
SAme questions as for the ipa subpackage

>  +%description ad
>  +Provides the Active Directory back end that the SSSD can utilize to
>  fetch
>  +identity data from and authenticate against an Active Directory
>  server.
>  +
>   %package -n libsss_idmap
>   Summary: FreeIPA Idmap library
>   Group: Development/Libraries
>  @@ -205,7 +271,7 @@ used by Python applications.
>   Summary: A library to allow communication between SUDO and SSSD
>   Group: Development/Libraries
>   License: LGPLv3+
>  -Requires: sssd = %{version}-%{release}
>  +Requires: sssd-ldap = %{version}-%{release}
>   Requires(post): /sbin/ldconfig
>   Requires(postun): /sbin/ldconfig 
why libsss_idmap would require the sssd-ldap subpakage ?

Simo.

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

[Jakub Hrozek]

On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
>  On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
>  > 
>  > Hi,
>  > 
>  > the attached patch splits the previously monolithic sssd package into
>  > sssd-common that contains the deamon and the responders and
>  > per-provider
>  > packages such as sssd-ldap or sssd-ipa.
>  > 
>  > This split would benefit two parties:
>  >     1) security auditors who are often trying to find the smallest
>  > package
>  >     set including dependencies needed for the package to function.
>  > They
>  >     would be able to i.e. install sssd-ldap and not bother about
>  >     sssd-ipa or sssd-ad pulling in more dependencies.
>  >     2) 3rd party programs such as realmd or authconfig that would only
>  >     be able to require or install on demand the needed packages.
>  > 
>  > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
>  > b
>  > applied on the two specfile patches I sent earlier (the thread subject
>  > included libsss_sudo).
>  
>  Questions inline.
>   
Not even nine months after the initial submission, here comes a revised
patch. I remember we had a discussion on IRC with Simo about this
problem, but I'll reply to the thread.

With the Radius provider patches on the list and requiring Samba bits in
the last couple of releases, I think that splitting the providers is
something we really should do.

>  > 
>  > 
>  > 
>  > 
>  > 
>  > 
>  > plain text
>  > document
>  > attachment
>  > (0001-Split-the-providers-into-separate-subpackages.patch)
>  > 
>  > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
>  > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
>  > Date: Fri, 28 Sep 2012 09:21:18 +0200
>  > Subject: [PATCH] Split the providers into separate subpackages
>  > 
>  > ---
>  >  contrib/sssd.spec.in | 145
>  > ++++++++++++++++++++++++++++++++++++++++-----------
>  >  1 file changed, 115 insertions(+), 30 deletions(-)
>  > 
>  > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  > index
>  > e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437
> 100644
>  > --- a/contrib/sssd.spec.in
>  > +++ b/contrib/sssd.spec.in
>  > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
>  > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>  >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
>  >  
>  >  ### Dependencies ###
>  > -
>  > -Requires: libldb >= 0.9.3
>  > -Requires: libtdb >= 1.1.3
>  > +Conflicts: sssd < %{version}-%{release}
>  >  Requires: sssd-client%{?_isa} = %{version}-%{release}
>  > -Requires: libipa_hbac = %{version}-%{release}
>  > -Requires: libsss_idmap = %{version}-%{release}
>  > -Requires: cyrus-sasl-gssapi
>  > -Requires: keyutils-libs
>  > -Requires(post): initscripts chkconfig
>  > -Requires(preun):  initscripts chkconfig
>  > -Requires(postun): initscripts chkconfig
>  > +Requires: sssd-common = %{version}-%{release}
>  > +Requires: sssd-ldap = %{version}-%{release}
>  > +Requires: sssd-krb5 = %{version}-%{release}
>  > +Requires: sssd-ipa = %{version}-%{release}
>  > +Requires: sssd-ad = %{version}-%{release}
>  
>  
>  Doesn't this set of requires makes the split useless ?
>  If I read it correctly it means sssd will require all subpackages anyway
>  so you cannot pick and choose to install only one as you say the purpose
>  is in the mail message.
>   
The intent of the sssd package requiring all the dependencies is to make sure
that any kickstart that specified "sssd" would get the whole set, because
we can't currently know what functionality and which provider was used.

To pick the minimal set for LDAP, you can run:
# yum install sssd-ldap
for instance.

>  >  %global servicename sssd
>  >  %global sssdstatedir %{_localstatedir}/lib/sss
>  > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
>  > connect to multiple different
>  >  account sources. It is also the basis to provide client auditing and
>  > policy
>  >  services for projects like FreeIPA.
>  >  
>  > +%package common
>  > +Summary: Common files for the SSSD
>  > +Group: Applications/System
>  > +License: GPLv3+
>  > +Requires: libldb >= 0.9.3
>  > +Requires: libtdb >= 1.1.3
>  > +Requires: sssd-client%{?_isa} = %{version}-%{release}
>  > +Requires(post): initscripts chkconfig
>  > +Requires(preun):  initscripts chkconfig
>  > +Requires(postun): initscripts chkconfig
>  > +Conflicts: sssd < %{version}-%{release}
>  > +
>  > +%description common
>  > +Common files for the SSSD.
>  > +
>  >  %package client
>  >  Summary: SSSD Client libraries for NSS and PAM
>  >  Group: Applications/System
>  > @@ -141,7 +152,7 @@ service.
>  >  Summary: Userspace tools for use with the SSSD
>  >  Group: Applications/System
>  >  License: GPLv3+
>  > -Requires: sssd = %{version}-%{release}
>  > +Requires: sssd-common = %{version}-%{release}
>  >  
>  >  %description tools
>  >  Provides userspace tools for manipulating users, groups, and nested
>  > groups in
>  > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
>  >      * sss_seed which pre-creates a user entry for use in kickstarts
>  >      * sss_obfuscate for generating an obfuscated LDAP password
>  >  
>  > +%package ldap
>  > +Summary: The LDAP back end of the SSSD
>  > +Group: Applications/System
>  > +License: GPLv3+
>  > +Conflicts: sssd < %{version}-%{release}
>  > +Requires: cyrus-sasl-gssapi
>  > +Requires: sssd-common = %{version}-%{release}
>  > +Requires: libsss_idmap = %{version}-%{release}
>  > +
>  > +%description ldap
>  > +Provides the LDAP back end that the SSSD can utilize to fetch
>  > identity data
>  > +from and authenticate against an LDAP server.
>  > +
>  > +%package krb5
>  > +Summary: The Kerberos authentication back end for the SSSD
>  > +Group: Applications/System
>  > +License: GPLv3+
>  > +Conflicts: sssd < %{version}-%{release}
>  > +Requires: cyrus-sasl-gssapi
>  > +Requires: sssd-common = %{version}-%{release}
>  > +
>  > +%description krb5
>  > +Provides the Kerberos back end that the SSSD can utilize authenticate
>  > +against a Kerberos server.
>  > +
>  > +%package ipa
>  > +Summary: The IPA back end of the SSSD
>  > +Group: Applications/System
>  > +License: GPLv3+
>  > +Conflicts: sssd < %{version}-%{release}
>  > +Requires: sssd-common = %{version}-%{release}
>  > +Requires: sssd-ldap = %{version}-%{release}
>  > +Requires: sssd-krb5 = %{version}-%{release}
>  > +Requires: libipa_hbac = %{version}-%{release}
>  > +Requires: libsss_idmap = %{version}-%{release}
>  > +Requires: bind-utils
>  
>  Does the ipa provider really need the sssd-ldap and sssd-krb5
>  subpackages ?
>  IIRC we statically compile the ldap and krb5 packages bits we need in
>  the ipa provider.
>  If you change this you probably want a require on cyrus-sasl-gssapi
>  here.
>  
>  (if it is just for the ldap and krb child processes shouldn't we simply
>  keep those binaries in the sssd or sssd-common package ?) 
Yes, the intent was to make sure the ldap child and krb5 child processes
are pulled in. But now that we switched to internal shared libraries, I
think a better solution is to have the krb5_common internal shared
library along with the ldap and krb5 child in a subpackage of its own
and let the Kerberos-aware providers pull these in.

>  
>  
>  > +%description ipa
>  > +Provides the IPA back end that the SSSD can utilize to fetch identity
>  > data
>  > +from and authenticate against an IPA server.
>  > +
>  > +%package ad
>  > +Summary: The AD back end of the SSSD
>  > +Group: Applications/System
>  > +License: GPLv3+
>  > +Conflicts: sssd < %{version}-%{release}
>  > +Requires: sssd-common = %{version}-%{release}
>  > +Requires: sssd-ldap = %{version}-%{release}
>  > +Requires: sssd-krb5 = %{version}-%{release}
>  > +Requires: libsss_idmap = %{version}-%{release}
>  
>  SAme questions as for the ipa subpackage
>  
>  > +%description ad
>  > +Provides the Active Directory back end that the SSSD can utilize to
>  > fetch
>  > +identity data from and authenticate against an Active Directory
>  > server.
>  > +
>  >  %package -n libsss_idmap
>  >  Summary: FreeIPA Idmap library
>  >  Group: Development/Libraries
>  > @@ -205,7 +271,7 @@ used by Python applications.
>  >  Summary: A library to allow communication between SUDO and SSSD
>  >  Group: Development/Libraries
>  >  License: LGPLv3+
>  > -Requires: sssd = %{version}-%{release}
>  > +Requires: sssd-ldap = %{version}-%{release}
>  >  Requires(post): /sbin/ldconfig
>  >  Requires(postun): /sbin/ldconfig
>  
>  why libsss_idmap would require the sssd-ldap subpakage ? 
I think this was a mass-replace bug, fixed.

>  
>  Simo. 
There are also two patches preceding the one that splits the providers:

[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD package
https://fedorahosted.org/sssd/ticket/1845

libsss_sudo and libsss_autofs are separate packages that contain just a
single client library with no additional dependencies. This separation
comes from the F-17 timeframe where the feature was really just a tech
preview so we didn't want it to be packaged in sssd proper. On the other
hand users are getting regularly confused about "sudo not working" when
all they really miss is the single library.

This patch moves the files owned by the libsss_autofs and libsss_sudo
packages back to the main sssd package. We also no longer build the
libsss_sudo documentation by default and do not ship the header file as
it was just a private one.



[PATCH 2/3] rpm: Use hardened flags for RPM build
https://fedorahosted.org/sssd/ticket/1797

This patch adds relro and bind_now linker flags to produce hardened
binaries. The change amounts to adding "-Wl,-z,now".

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/05/2013 08:50 AM, Jakub Hrozek wrote:
>  On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
> > On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
> >> 
> >> Hi,
> >> 
> >> the attached patch splits the previously monolithic sssd
> >> package into sssd-common that contains the deamon and the
> >> responders and per-provider packages such as sssd-ldap or
> >> sssd-ipa.
> >> 
> >> This split would benefit two parties: 1) security auditors who
> >> are often trying to find the smallest package set including
> >> dependencies needed for the package to function. They would be
> >> able to i.e. install sssd-ldap and not bother about sssd-ipa or
> >> sssd-ad pulling in more dependencies. 2) 3rd party programs
> >> such as realmd or authconfig that would only be able to require
> >> or install on demand the needed packages.
> >> 
> >> The patch addresses https://fedorahosted.org/sssd/ticket/1510
> >> and must b applied on the two specfile patches I sent earlier
> >> (the thread subject included libsss_sudo).
> > 
> > Questions inline.
> > 
>  
>  Not even nine months after the initial submission, here comes a
>  revised patch. I remember we had a discussion on IRC with Simo
>  about this problem, but I'll reply to the thread.
>  
>  With the Radius provider patches on the list and requiring Samba
>  bits in the last couple of releases, I think that splitting the
>  providers is something we really should do.
>  
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> plain text document attachment 
> >> (0001-Split-the-providers-into-separate-subpackages.patch)
> >> 
> >> From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17
> >> 00:00:00 2001 From: Jakub Hrozek <jhrozek(a)redhat.com&gt; Date:
> >> Fri, 28 Sep 2012 09:21:18 +0200 Subject: [PATCH] Split the
> >> providers into separate subpackages
> >> 
> >> --- contrib/sssd.spec.in | 145 
> >> ++++++++++++++++++++++++++++++++++++++++----------- 1 file
> >> changed, 115 insertions(+), 30 deletions(-)
> >> 
> >> diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 
> >>
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437
> >> 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@
> >> -45,17 +45,13 @@ BuildRoot: %(mktemp -ud 
> >> %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) Patch0001:
> >> sssd-1.9-man-change-default-ccache.patch
> >> 
> >> ### Dependencies ### - -Requires: libldb >= 0.9.3 -Requires:
> >> libtdb >= 1.1.3 +Conflicts: sssd < %{version}-%{release} 
> >> Requires: sssd-client%{?_isa} = %{version}-%{release} 
> >> -Requires: libipa_hbac = %{version}-%{release} -Requires:
> >> libsss_idmap = %{version}-%{release} -Requires:
> >> cyrus-sasl-gssapi -Requires: keyutils-libs -Requires(post):
> >> initscripts chkconfig -Requires(preun):  initscripts chkconfig 
> >> -Requires(postun): initscripts chkconfig +Requires: sssd-common
> >> = %{version}-%{release} +Requires: sssd-ldap =
> >> %{version}-%{release} +Requires: sssd-krb5 =
> >> %{version}-%{release} +Requires: sssd-ipa =
> >> %{version}-%{release} +Requires: sssd-ad =
> >> %{version}-%{release}
> > 
> > 
> > Doesn't this set of requires makes the split useless ? If I read
> > it correctly it means sssd will require all subpackages anyway so
> > you cannot pick and choose to install only one as you say the
> > purpose is in the mail message.
> > 
>  
>  The intent of the sssd package requiring all the dependencies is to
>  make sure that any kickstart that specified "sssd" would get the
>  whole set, because we can't currently know what functionality and
>  which provider was used.
>  
>  To pick the minimal set for LDAP, you can run: # yum install
>  sssd-ldap for instance.
>  
> >> %global servicename sssd %global sssdstatedir
> >> %{_localstatedir}/lib/sss @@ -126,6 +122,21 @@ the system and a
> >> pluggable backend system to connect to multiple different 
> >> account sources. It is also the basis to provide client
> >> auditing and policy services for projects like FreeIPA.
> >> 
> >> +%package common +Summary: Common files for the SSSD +Group:
> >> Applications/System +License: GPLv3+ +Requires: libldb >=
> >> 0.9.3 +Requires: libtdb >= 1.1.3 +Requires: sssd-client%{?_isa}
> >> = %{version}-%{release} +Requires(post): initscripts chkconfig 
> >> +Requires(preun):  initscripts chkconfig +Requires(postun):
> >> initscripts chkconfig +Conflicts: sssd < %{version}-%{release} 
> >> + +%description common +Common files for the SSSD. + %package
> >> client Summary: SSSD Client libraries for NSS and PAM Group:
> >> Applications/System @@ -141,7 +152,7 @@ service. Summary:
> >> Userspace tools for use with the SSSD Group:
> >> Applications/System License: GPLv3+ -Requires: sssd =
> >> %{version}-%{release} +Requires: sssd-common =
> >> %{version}-%{release}
> >> 
> >> %description tools Provides userspace tools for manipulating
> >> users, groups, and nested groups in @@ -153,6 +164,61 @@ Also
> >> provides several other administrative tools: * sss_seed which
> >> pre-creates a user entry for use in kickstarts * sss_obfuscate
> >> for generating an obfuscated LDAP password
> >> 
> >> +%package ldap +Summary: The LDAP back end of the SSSD +Group:
> >> Applications/System +License: GPLv3+ +Conflicts: sssd <
> >> %{version}-%{release} +Requires: cyrus-sasl-gssapi +Requires:
> >> sssd-common = %{version}-%{release} +Requires: libsss_idmap =
> >> %{version}-%{release} + +%description ldap +Provides the LDAP
> >> back end that the SSSD can utilize to fetch identity data +from
> >> and authenticate against an LDAP server. + +%package krb5 
> >> +Summary: The Kerberos authentication back end for the SSSD 
> >> +Group: Applications/System +License: GPLv3+ +Conflicts: sssd <
> >> %{version}-%{release} +Requires: cyrus-sasl-gssapi +Requires:
> >> sssd-common = %{version}-%{release} + +%description krb5 
> >> +Provides the Kerberos back end that the SSSD can utilize
> >> authenticate +against a Kerberos server. + +%package ipa 
> >> +Summary: The IPA back end of the SSSD +Group:
> >> Applications/System +License: GPLv3+ +Conflicts: sssd <
> >> %{version}-%{release} +Requires: sssd-common =
> >> %{version}-%{release} +Requires: sssd-ldap =
> >> %{version}-%{release} +Requires: sssd-krb5 =
> >> %{version}-%{release} +Requires: libipa_hbac =
> >> %{version}-%{release} +Requires: libsss_idmap =
> >> %{version}-%{release} +Requires: bind-utils
> > 
> > Does the ipa provider really need the sssd-ldap and sssd-krb5 
> > subpackages ? IIRC we statically compile the ldap and krb5
> > packages bits we need in the ipa provider. If you change this you
> > probably want a require on cyrus-sasl-gssapi here.
> > 
> > (if it is just for the ldap and krb child processes shouldn't we
> > simply keep those binaries in the sssd or sssd-common package ?)
>  
>  Yes, the intent was to make sure the ldap child and krb5 child
>  processes are pulled in. But now that we switched to internal
>  shared libraries, I think a better solution is to have the
>  krb5_common internal shared library along with the ldap and krb5
>  child in a subpackage of its own and let the Kerberos-aware
>  providers pull these in.
>  
> > 
> > 
> >> +%description ipa +Provides the IPA back end that the SSSD can
> >> utilize to fetch identity data +from and authenticate against
> >> an IPA server. + +%package ad +Summary: The AD back end of the
> >> SSSD +Group: Applications/System +License: GPLv3+ +Conflicts:
> >> sssd < %{version}-%{release} +Requires: sssd-common =
> >> %{version}-%{release} +Requires: sssd-ldap =
> >> %{version}-%{release} +Requires: sssd-krb5 =
> >> %{version}-%{release} +Requires: libsss_idmap =
> >> %{version}-%{release}
> > 
> > SAme questions as for the ipa subpackage
> > 
> >> +%description ad +Provides the Active Directory back end that
> >> the SSSD can utilize to fetch +identity data from and
> >> authenticate against an Active Directory server. + %package -n
> >> libsss_idmap Summary: FreeIPA Idmap library Group:
> >> Development/Libraries @@ -205,7 +271,7 @@ used by Python
> >> applications. Summary: A library to allow communication between
> >> SUDO and SSSD Group: Development/Libraries License: LGPLv3+ 
> >> -Requires: sssd = %{version}-%{release} +Requires: sssd-ldap =
> >> %{version}-%{release} Requires(post): /sbin/ldconfig 
> >> Requires(postun): /sbin/ldconfig
> > 
> > why libsss_idmap would require the sssd-ldap subpakage ?
>  
>  I think this was a mass-replace bug, fixed.
>  
> > 
> > Simo.
>  
>  There are also two patches preceding the one that splits the
>  providers:
>  
>  [PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the
>  main SSSD package https://fedorahosted.org/sssd/ticket/1845
>   
>  libsss_sudo and libsss_autofs are separate packages that contain
>  just a single client library with no additional dependencies. This
>  separation comes from the F-17 timeframe where the feature was
>  really just a tech preview so we didn't want it to be packaged in
>  sssd proper. On the other hand users are getting regularly confused
>  about "sudo not working" when all they really miss is the single
>  library.
>  
>  This patch moves the files owned by the libsss_autofs and
>  libsss_sudo packages back to the main sssd package. We also no
>  longer build the libsss_sudo documentation by default and do not
>  ship the header file as it was just a private one.
>  
>  
>  
>  [PATCH 2/3] rpm: Use hardened flags for RPM build 
>  https://fedorahosted.org/sssd/ticket/1797
>  
>  This patch adds relro and bind_now linker flags to produce
>  hardened binaries. The change amounts to adding "-Wl,-z,now".
>   
FWIW, the official Fedora packages have been building with
_hardened_build in RPM for a long time, which adds both -fPIC (if
- -fPIE isn't already specified) as well as -Wl,-z,now. So this is
already well-tested, and this is just changing this default upstream
as well.

I don't have the time right now to do a full review, but I wanted to
clarify this.

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlGvS5oACgkQeiVVYja6o6MJ+wCePtutlPOkV34gYEFCvV6n62fp
> 3x0AniaE/oLi4Nrw4OVQh6UNhzETy37Z
> =bVLn
> -----END PGP SIGNATURE-----

[Lukas Slebodnik]

On (05/06/13 14:50), Jakub Hrozek wrote:
> On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
> > On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
> > > 
> > > Hi,
> > > 
> > > the attached patch splits the previously monolithic sssd package into
> > > sssd-common that contains the deamon and the responders and
> > > per-provider
> > > packages such as sssd-ldap or sssd-ipa.
> > > 
> > > This split would benefit two parties:
> > >     1) security auditors who are often trying to find the smallest
> > > package
> > >     set including dependencies needed for the package to function.
> > > They
> > >     would be able to i.e. install sssd-ldap and not bother about
> > >     sssd-ipa or sssd-ad pulling in more dependencies.
> > >     2) 3rd party programs such as realmd or authconfig that would only
> > >     be able to require or install on demand the needed packages.
> > > 
> > > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
> > > b
> > > applied on the two specfile patches I sent earlier (the thread subject
> > > included libsss_sudo).
> > 
> > Questions inline.
> > 
>
> Not even nine months after the initial submission, here comes a revised
> patch. I remember we had a discussion on IRC with Simo about this
> problem, but I'll reply to the thread.
>
> With the Radius provider patches on the list and requiring Samba bits in
> the last couple of releases, I think that splitting the providers is
> something we really should do.
>
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > plain text
> > > document
> > > attachment
> > > (0001-Split-the-providers-into-separate-subpackages.patch)
> > > 
> > > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
> > > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
> > > Date: Fri, 28 Sep 2012 09:21:18 +0200
> > > Subject: [PATCH] Split the providers into separate subpackages
> > > 
> > > ---
> > >  contrib/sssd.spec.in | 145
> > > ++++++++++++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 115 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> > > index
> > >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
> > > --- a/contrib/sssd.spec.in
> > > +++ b/contrib/sssd.spec.in
> > > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
> > > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> > >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
> > >  
> > >  ### Dependencies ###
> > > -
> > > -Requires: libldb >= 0.9.3
> > > -Requires: libtdb >= 1.1.3
> > > +Conflicts: sssd < %{version}-%{release}
> > >  Requires: sssd-client%{?_isa} = %{version}-%{release}
> > > -Requires: libipa_hbac = %{version}-%{release}
> > > -Requires: libsss_idmap = %{version}-%{release}
> > > -Requires: cyrus-sasl-gssapi
> > > -Requires: keyutils-libs
> > > -Requires(post): initscripts chkconfig
> > > -Requires(preun):  initscripts chkconfig
> > > -Requires(postun): initscripts chkconfig
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > > +Requires: sssd-krb5 = %{version}-%{release}
> > > +Requires: sssd-ipa = %{version}-%{release}
> > > +Requires: sssd-ad = %{version}-%{release}
> > 
> > 
> > Doesn't this set of requires makes the split useless ?
> > If I read it correctly it means sssd will require all subpackages anyway
> > so you cannot pick and choose to install only one as you say the purpose
> > is in the mail message.
> > 
>
> The intent of the sssd package requiring all the dependencies is to make sure
> that any kickstart that specified "sssd" would get the whole set, because
> we can't currently know what functionality and which provider was used.
>
> To pick the minimal set for LDAP, you can run:
> # yum install sssd-ldap
> for instance.
>
> > >  %global servicename sssd
> > >  %global sssdstatedir %{_localstatedir}/lib/sss
> > > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
> > > connect to multiple different
> > >  account sources. It is also the basis to provide client auditing and
> > > policy
> > >  services for projects like FreeIPA.
> > >  
> > > +%package common
> > > +Summary: Common files for the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Requires: libldb >= 0.9.3
> > > +Requires: libtdb >= 1.1.3
> > > +Requires: sssd-client%{?_isa} = %{version}-%{release}
> > > +Requires(post): initscripts chkconfig
> > > +Requires(preun):  initscripts chkconfig
> > > +Requires(postun): initscripts chkconfig
> > > +Conflicts: sssd < %{version}-%{release}
> > > +
> > > +%description common
> > > +Common files for the SSSD.
> > > +
> > >  %package client
> > >  Summary: SSSD Client libraries for NSS and PAM
> > >  Group: Applications/System
> > > @@ -141,7 +152,7 @@ service.
> > >  Summary: Userspace tools for use with the SSSD
> > >  Group: Applications/System
> > >  License: GPLv3+
> > > -Requires: sssd = %{version}-%{release}
> > > +Requires: sssd-common = %{version}-%{release}
> > >  
> > >  %description tools
> > >  Provides userspace tools for manipulating users, groups, and nested
> > > groups in
> > > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
> > >      * sss_seed which pre-creates a user entry for use in kickstarts
> > >      * sss_obfuscate for generating an obfuscated LDAP password
> > >  
> > > +%package ldap
> > > +Summary: The LDAP back end of the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: cyrus-sasl-gssapi
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: libsss_idmap = %{version}-%{release}
> > > +
> > > +%description ldap
> > > +Provides the LDAP back end that the SSSD can utilize to fetch
> > > identity data
> > > +from and authenticate against an LDAP server.
> > > +
> > > +%package krb5
> > > +Summary: The Kerberos authentication back end for the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: cyrus-sasl-gssapi
> > > +Requires: sssd-common = %{version}-%{release}
> > > +
> > > +%description krb5
> > > +Provides the Kerberos back end that the SSSD can utilize authenticate
> > > +against a Kerberos server.
> > > +
> > > +%package ipa
> > > +Summary: The IPA back end of the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > > +Requires: sssd-krb5 = %{version}-%{release}
> > > +Requires: libipa_hbac = %{version}-%{release}
> > > +Requires: libsss_idmap = %{version}-%{release}
> > > +Requires: bind-utils
> > 
> > Does the ipa provider really need the sssd-ldap and sssd-krb5
> > subpackages ?
> > IIRC we statically compile the ldap and krb5 packages bits we need in
> > the ipa provider.
> > If you change this you probably want a require on cyrus-sasl-gssapi
> > here.
> > 
> > (if it is just for the ldap and krb child processes shouldn't we simply
> > keep those binaries in the sssd or sssd-common package ?)
>
> Yes, the intent was to make sure the ldap child and krb5 child processes
> are pulled in. But now that we switched to internal shared libraries, I
> think a better solution is to have the krb5_common internal shared
> library along with the ldap and krb5 child in a subpackage of its own
> and let the Kerberos-aware providers pull these in.
>
> > 
> > 
> > > +%description ipa
> > > +Provides the IPA back end that the SSSD can utilize to fetch identity
> > > data
> > > +from and authenticate against an IPA server.
> > > +
> > > +%package ad
> > > +Summary: The AD back end of the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > > +Requires: sssd-krb5 = %{version}-%{release}
> > > +Requires: libsss_idmap = %{version}-%{release}
> > 
> > SAme questions as for the ipa subpackage
> > 
> > > +%description ad
> > > +Provides the Active Directory back end that the SSSD can utilize to
> > > fetch
> > > +identity data from and authenticate against an Active Directory
> > > server.
> > > +
> > >  %package -n libsss_idmap
> > >  Summary: FreeIPA Idmap library
> > >  Group: Development/Libraries
> > > @@ -205,7 +271,7 @@ used by Python applications.
> > >  Summary: A library to allow communication between SUDO and SSSD
> > >  Group: Development/Libraries
> > >  License: LGPLv3+
> > > -Requires: sssd = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > >  Requires(post): /sbin/ldconfig
> > >  Requires(postun): /sbin/ldconfig
> > 
> > why libsss_idmap would require the sssd-ldap subpakage ?
>
> I think this was a mass-replace bug, fixed.
>
> > 
> > Simo.
>
> There are also two patches preceding the one that splits the providers:
>
> [PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD package
> https://fedorahosted.org/sssd/ticket/1845
>
> libsss_sudo and libsss_autofs are separate packages that contain just a
> single client library with no additional dependencies. This separation
> comes from the F-17 timeframe where the feature was really just a tech
> preview so we didn't want it to be packaged in sssd proper. On the other
> hand users are getting regularly confused about "sudo not working" when
> all they really miss is the single library.
>
> This patch moves the files owned by the libsss_autofs and libsss_sudo
> packages back to the main sssd package. We also no longer build the
> libsss_sudo documentation by default and do not ship the header file as
> it was just a private one.
>  
I tested upgrade with installed freeipa-client and sssd
and upgrade worked as expected.

ACK

> [PATCH 2/3] rpm: Use hardened flags for RPM build
> https://fedorahosted.org/sssd/ticket/1797
>
> This patch adds relro and bind_now linker flags to produce hardened
> binaries. The change amounts to adding "-Wl,-z,now". 
Rpm packages are built without problems and Stephen blessed this patch.
ACK

I need more time to review last patch. I spent too much time with
troubleshooting ipa today. I will review last patch tomorrow.

LS

[Lukas Slebodnik]

On (06/06/13 20:11), Lukas Slebodnik wrote:
> On (05/06/13 14:50), Jakub Hrozek wrote:
> >On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
> >> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
> >> > 
> >> > Hi,
> >> > 
> >> > the attached patch splits the previously monolithic sssd package into
> >> > sssd-common that contains the deamon and the responders and
> >> > per-provider
> >> > packages such as sssd-ldap or sssd-ipa.
> >> > 
> >> > This split would benefit two parties:
> >> >     1) security auditors who are often trying to find the smallest
> >> > package
> >> >     set including dependencies needed for the package to function.
> >> > They
> >> >     would be able to i.e. install sssd-ldap and not bother about
> >> >     sssd-ipa or sssd-ad pulling in more dependencies.
> >> >     2) 3rd party programs such as realmd or authconfig that would only
> >> >     be able to require or install on demand the needed packages.
> >> > 
> >> > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
> >> > b
> >> > applied on the two specfile patches I sent earlier (the thread subject
> >> > included libsss_sudo).
> >> 
> >> Questions inline.
> >> 
> >
> >Not even nine months after the initial submission, here comes a revised
> >patch. I remember we had a discussion on IRC with Simo about this
> >problem, but I'll reply to the thread.
> >
> >With the Radius provider patches on the list and requiring Samba bits in
> >the last couple of releases, I think that splitting the providers is
> >something we really should do.
> >
> >> > 
> >> > 
> >> > 
> >> > 
> >> > 
> >> > 
> >> > plain text
> >> > document
> >> > attachment
> >> > (0001-Split-the-providers-into-separate-subpackages.patch)
> >> > 
> >> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
> >> > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
> >> > Date: Fri, 28 Sep 2012 09:21:18 +0200
> >> > Subject: [PATCH] Split the providers into separate subpackages
> >> > 
> >> > ---
> >> >  contrib/sssd.spec.in | 145
> >> > ++++++++++++++++++++++++++++++++++++++++-----------
> >> >  1 file changed, 115 insertions(+), 30 deletions(-)
> >> > 
> >> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> >> > index
> >> >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
> >> > --- a/contrib/sssd.spec.in
> >> > +++ b/contrib/sssd.spec.in
> >> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
> >> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> >> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
> >> >  
> >> >  ### Dependencies ###
> >> > -
> >> > -Requires: libldb >= 0.9.3
> >> > -Requires: libtdb >= 1.1.3
> >> > +Conflicts: sssd < %{version}-%{release}
> >> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
> >> > -Requires: libipa_hbac = %{version}-%{release}
> >> > -Requires: libsss_idmap = %{version}-%{release}
> >> > -Requires: cyrus-sasl-gssapi
> >> > -Requires: keyutils-libs
> >> > -Requires(post): initscripts chkconfig
> >> > -Requires(preun):  initscripts chkconfig
> >> > -Requires(postun): initscripts chkconfig
> >> > +Requires: sssd-common = %{version}-%{release}
> >> > +Requires: sssd-ldap = %{version}-%{release}
> >> > +Requires: sssd-krb5 = %{version}-%{release}
> >> > +Requires: sssd-ipa = %{version}-%{release}
> >> > +Requires: sssd-ad = %{version}-%{release}
> >> 
> >> 
> >> Doesn't this set of requires makes the split useless ?
> >> If I read it correctly it means sssd will require all subpackages anyway
> >> so you cannot pick and choose to install only one as you say the purpose
> >> is in the mail message.
> >> 
> >
> >The intent of the sssd package requiring all the dependencies is to make sure
> >that any kickstart that specified "sssd" would get the whole set, because
> >we can't currently know what functionality and which provider was used.
> >
> >To pick the minimal set for LDAP, you can run:
> ># yum install sssd-ldap
> >for instance.
> >
> >> >  %global servicename sssd
> >> >  %global sssdstatedir %{_localstatedir}/lib/sss
> >> > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
> >> > connect to multiple different
> >> >  account sources. It is also the basis to provide client auditing and
> >> > policy
> >> >  services for projects like FreeIPA.
> >> >  
> >> > +%package common
> >> > +Summary: Common files for the SSSD
> >> > +Group: Applications/System
> >> > +License: GPLv3+
> >> > +Requires: libldb >= 0.9.3
> >> > +Requires: libtdb >= 1.1.3
> >> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
> >> > +Requires(post): initscripts chkconfig
> >> > +Requires(preun):  initscripts chkconfig
> >> > +Requires(postun): initscripts chkconfig
> >> > +Conflicts: sssd < %{version}-%{release}
> >> > +
> >> > +%description common
> >> > +Common files for the SSSD.
> >> > +
> >> >  %package client
> >> >  Summary: SSSD Client libraries for NSS and PAM
> >> >  Group: Applications/System
> >> > @@ -141,7 +152,7 @@ service.
> >> >  Summary: Userspace tools for use with the SSSD
> >> >  Group: Applications/System
> >> >  License: GPLv3+
> >> > -Requires: sssd = %{version}-%{release}
> >> > +Requires: sssd-common = %{version}-%{release}
> >> >  
> >> >  %description tools
> >> >  Provides userspace tools for manipulating users, groups, and nested
> >> > groups in
> >> > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
> >> >      * sss_seed which pre-creates a user entry for use in kickstarts
> >> >      * sss_obfuscate for generating an obfuscated LDAP password
> >> >  
> >> > +%package ldap
> >> > +Summary: The LDAP back end of the SSSD
> >> > +Group: Applications/System
> >> > +License: GPLv3+
> >> > +Conflicts: sssd < %{version}-%{release}
> >> > +Requires: cyrus-sasl-gssapi
> >> > +Requires: sssd-common = %{version}-%{release}
> >> > +Requires: libsss_idmap = %{version}-%{release}
> >> > +
> >> > +%description ldap
> >> > +Provides the LDAP back end that the SSSD can utilize to fetch
> >> > identity data
> >> > +from and authenticate against an LDAP server.
> >> > +
> >> > +%package krb5
> >> > +Summary: The Kerberos authentication back end for the SSSD
> >> > +Group: Applications/System
> >> > +License: GPLv3+
> >> > +Conflicts: sssd < %{version}-%{release}
> >> > +Requires: cyrus-sasl-gssapi
> >> > +Requires: sssd-common = %{version}-%{release}
> >> > +
> >> > +%description krb5
> >> > +Provides the Kerberos back end that the SSSD can utilize authenticate
> >> > +against a Kerberos server.
> >> > +
> >> > +%package ipa
> >> > +Summary: The IPA back end of the SSSD
> >> > +Group: Applications/System
> >> > +License: GPLv3+
> >> > +Conflicts: sssd < %{version}-%{release}
> >> > +Requires: sssd-common = %{version}-%{release}
> >> > +Requires: sssd-ldap = %{version}-%{release}
> >> > +Requires: sssd-krb5 = %{version}-%{release}
> >> > +Requires: libipa_hbac = %{version}-%{release}
> >> > +Requires: libsss_idmap = %{version}-%{release}
> >> > +Requires: bind-utils
> >> 
> >> Does the ipa provider really need the sssd-ldap and sssd-krb5
> >> subpackages ?
> >> IIRC we statically compile the ldap and krb5 packages bits we need in
> >> the ipa provider.
> >> If you change this you probably want a require on cyrus-sasl-gssapi
> >> here.
> >> 
> >> (if it is just for the ldap and krb child processes shouldn't we simply
> >> keep those binaries in the sssd or sssd-common package ?)
> >
> >Yes, the intent was to make sure the ldap child and krb5 child processes
> >are pulled in. But now that we switched to internal shared libraries, I
> >think a better solution is to have the krb5_common internal shared
> >library along with the ldap and krb5 child in a subpackage of its own
> >and let the Kerberos-aware providers pull these in.
> >
> >> 
> >> 
> >> > +%description ipa
> >> > +Provides the IPA back end that the SSSD can utilize to fetch identity
> >> > data
> >> > +from and authenticate against an IPA server.
> >> > +
> >> > +%package ad
> >> > +Summary: The AD back end of the SSSD
> >> > +Group: Applications/System
> >> > +License: GPLv3+
> >> > +Conflicts: sssd < %{version}-%{release}
> >> > +Requires: sssd-common = %{version}-%{release}
> >> > +Requires: sssd-ldap = %{version}-%{release}
> >> > +Requires: sssd-krb5 = %{version}-%{release}
> >> > +Requires: libsss_idmap = %{version}-%{release}
> >> 
> >> SAme questions as for the ipa subpackage
> >> 
> >> > +%description ad
> >> > +Provides the Active Directory back end that the SSSD can utilize to
> >> > fetch
> >> > +identity data from and authenticate against an Active Directory
> >> > server.
> >> > +
> >> >  %package -n libsss_idmap
> >> >  Summary: FreeIPA Idmap library
> >> >  Group: Development/Libraries
> >> > @@ -205,7 +271,7 @@ used by Python applications.
> >> >  Summary: A library to allow communication between SUDO and SSSD
> >> >  Group: Development/Libraries
> >> >  License: LGPLv3+
> >> > -Requires: sssd = %{version}-%{release}
> >> > +Requires: sssd-ldap = %{version}-%{release}
> >> >  Requires(post): /sbin/ldconfig
> >> >  Requires(postun): /sbin/ldconfig
> >> 
> >> why libsss_idmap would require the sssd-ldap subpakage ?
> >
> >I think this was a mass-replace bug, fixed.
> >
> >> 
> >> Simo.
> >
> >There are also two patches preceding the one that splits the providers:
> >
> >[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD package
> >https://fedorahosted.org/sssd/ticket/1845
> >
> >libsss_sudo and libsss_autofs are separate packages that contain just a
> >single client library with no additional dependencies. This separation
> >comes from the F-17 timeframe where the feature was really just a tech
> >preview so we didn't want it to be packaged in sssd proper. On the other
> >hand users are getting regularly confused about "sudo not working" when
> >all they really miss is the single library.
> >
> >This patch moves the files owned by the libsss_autofs and libsss_sudo
> >packages back to the main sssd package. We also no longer build the
> >libsss_sudo documentation by default and do not ship the header file as
> >it was just a private one.
> >
>
> I tested upgrade with installed freeipa-client and sssd
> and upgrade worked as expected.
>
> ACK
>
> >
> >
> >[PATCH 2/3] rpm: Use hardened flags for RPM build
> >https://fedorahosted.org/sssd/ticket/1797
> >
> >This patch adds relro and bind_now linker flags to produce hardened
> >binaries. The change amounts to adding "-Wl,-z,now".
>
> Rpm packages are built without problems and Stephen blessed this patch.
> ACK 
Second patch NACK.

+export CFLAGS="%{optflags} -Wl,-z,now"
                            ^^^^^^^^^^
                        This is linker argument and should not be among CFLAGS

> I need more time to review last patch. I spent too much time with
> troubleshooting ipa today. I will review last patch tomorrow.
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Thu, Jun 06, 2013 at 09:56:20PM +0200, Lukas Slebodnik wrote:
>  On (06/06/13 20:11), Lukas Slebodnik wrote:
>  >On (05/06/13 14:50), Jakub Hrozek wrote:
>  >>On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
>  >>> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
>  >>> > 
>  >>> > Hi,
>  >>> > 
>  >>> > the attached patch splits the previously monolithic sssd package into
>  >>> > sssd-common that contains the deamon and the responders and
>  >>> > per-provider
>  >>> > packages such as sssd-ldap or sssd-ipa.
>  >>> > 
>  >>> > This split would benefit two parties:
>  >>> >     1) security auditors who are often trying to find the smallest
>  >>> > package
>  >>> >     set including dependencies needed for the package to function.
>  >>> > They
>  >>> >     would be able to i.e. install sssd-ldap and not bother about
>  >>> >     sssd-ipa or sssd-ad pulling in more dependencies.
>  >>> >     2) 3rd party programs such as realmd or authconfig that would only
>  >>> >     be able to require or install on demand the needed packages.
>  >>> > 
>  >>> > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
>  >>> > b
>  >>> > applied on the two specfile patches I sent earlier (the thread subject
>  >>> > included libsss_sudo).
>  >>> 
>  >>> Questions inline.
>  >>> 
>  >>
>  >>Not even nine months after the initial submission, here comes a revised
>  >>patch. I remember we had a discussion on IRC with Simo about this
>  >>problem, but I'll reply to the thread.
>  >>
>  >>With the Radius provider patches on the list and requiring Samba bits in
>  >>the last couple of releases, I think that splitting the providers is
>  >>something we really should do.
>  >>
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > plain text
>  >>> > document
>  >>> > attachment
>  >>> > (0001-Split-the-providers-into-separate-subpackages.patch)
>  >>> > 
>  >>> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
>  >>> > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
>  >>> > Date: Fri, 28 Sep 2012 09:21:18 +0200
>  >>> > Subject: [PATCH] Split the providers into separate subpackages
>  >>> > 
>  >>> > ---
>  >>> >  contrib/sssd.spec.in | 145
>  >>> > ++++++++++++++++++++++++++++++++++++++++-----------
>  >>> >  1 file changed, 115 insertions(+), 30 deletions(-)
>  >>> > 
>  >>> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  >>> > index
>  >>> >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
>  >>> > --- a/contrib/sssd.spec.in
>  >>> > +++ b/contrib/sssd.spec.in
>  >>> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
>  >>> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>  >>> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
>  >>> >  
>  >>> >  ### Dependencies ###
>  >>> > -
>  >>> > -Requires: libldb >= 0.9.3
>  >>> > -Requires: libtdb >= 1.1.3
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
>  >>> > -Requires: libipa_hbac = %{version}-%{release}
>  >>> > -Requires: libsss_idmap = %{version}-%{release}
>  >>> > -Requires: cyrus-sasl-gssapi
>  >>> > -Requires: keyutils-libs
>  >>> > -Requires(post): initscripts chkconfig
>  >>> > -Requires(preun):  initscripts chkconfig
>  >>> > -Requires(postun): initscripts chkconfig
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >>> > +Requires: sssd-ipa = %{version}-%{release}
>  >>> > +Requires: sssd-ad = %{version}-%{release}
>  >>> 
>  >>> 
>  >>> Doesn't this set of requires makes the split useless ?
>  >>> If I read it correctly it means sssd will require all subpackages anyway
>  >>> so you cannot pick and choose to install only one as you say the purpose
>  >>> is in the mail message.
>  >>> 
>  >>
>  >>The intent of the sssd package requiring all the dependencies is to make sure
>  >>that any kickstart that specified "sssd" would get the whole set,
> because
>  >>we can't currently know what functionality and which provider was used.
>  >>
>  >>To pick the minimal set for LDAP, you can run:
>  >># yum install sssd-ldap
>  >>for instance.
>  >>
>  >>> >  %global servicename sssd
>  >>> >  %global sssdstatedir %{_localstatedir}/lib/sss
>  >>> > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
>  >>> > connect to multiple different
>  >>> >  account sources. It is also the basis to provide client auditing and
>  >>> > policy
>  >>> >  services for projects like FreeIPA.
>  >>> >  
>  >>> > +%package common
>  >>> > +Summary: Common files for the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Requires: libldb >= 0.9.3
>  >>> > +Requires: libtdb >= 1.1.3
>  >>> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
>  >>> > +Requires(post): initscripts chkconfig
>  >>> > +Requires(preun):  initscripts chkconfig
>  >>> > +Requires(postun): initscripts chkconfig
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +
>  >>> > +%description common
>  >>> > +Common files for the SSSD.
>  >>> > +
>  >>> >  %package client
>  >>> >  Summary: SSSD Client libraries for NSS and PAM
>  >>> >  Group: Applications/System
>  >>> > @@ -141,7 +152,7 @@ service.
>  >>> >  Summary: Userspace tools for use with the SSSD
>  >>> >  Group: Applications/System
>  >>> >  License: GPLv3+
>  >>> > -Requires: sssd = %{version}-%{release}
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> >  
>  >>> >  %description tools
>  >>> >  Provides userspace tools for manipulating users, groups, and nested
>  >>> > groups in
>  >>> > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
>  >>> >      * sss_seed which pre-creates a user entry for use in kickstarts
>  >>> >      * sss_obfuscate for generating an obfuscated LDAP password
>  >>> >  
>  >>> > +%package ldap
>  >>> > +Summary: The LDAP back end of the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: cyrus-sasl-gssapi
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >>> > +
>  >>> > +%description ldap
>  >>> > +Provides the LDAP back end that the SSSD can utilize to fetch
>  >>> > identity data
>  >>> > +from and authenticate against an LDAP server.
>  >>> > +
>  >>> > +%package krb5
>  >>> > +Summary: The Kerberos authentication back end for the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: cyrus-sasl-gssapi
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +
>  >>> > +%description krb5
>  >>> > +Provides the Kerberos back end that the SSSD can utilize authenticate
>  >>> > +against a Kerberos server.
>  >>> > +
>  >>> > +%package ipa
>  >>> > +Summary: The IPA back end of the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >>> > +Requires: libipa_hbac = %{version}-%{release}
>  >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >>> > +Requires: bind-utils
>  >>> 
>  >>> Does the ipa provider really need the sssd-ldap and sssd-krb5
>  >>> subpackages ?
>  >>> IIRC we statically compile the ldap and krb5 packages bits we need in
>  >>> the ipa provider.
>  >>> If you change this you probably want a require on cyrus-sasl-gssapi
>  >>> here.
>  >>> 
>  >>> (if it is just for the ldap and krb child processes shouldn't we simply
>  >>> keep those binaries in the sssd or sssd-common package ?)
>  >>
>  >>Yes, the intent was to make sure the ldap child and krb5 child processes
>  >>are pulled in. But now that we switched to internal shared libraries, I
>  >>think a better solution is to have the krb5_common internal shared
>  >>library along with the ldap and krb5 child in a subpackage of its own
>  >>and let the Kerberos-aware providers pull these in.
>  >>
>  >>> 
>  >>> 
>  >>> > +%description ipa
>  >>> > +Provides the IPA back end that the SSSD can utilize to fetch identity
>  >>> > data
>  >>> > +from and authenticate against an IPA server.
>  >>> > +
>  >>> > +%package ad
>  >>> > +Summary: The AD back end of the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >>> 
>  >>> SAme questions as for the ipa subpackage
>  >>> 
>  >>> > +%description ad
>  >>> > +Provides the Active Directory back end that the SSSD can utilize to
>  >>> > fetch
>  >>> > +identity data from and authenticate against an Active Directory
>  >>> > server.
>  >>> > +
>  >>> >  %package -n libsss_idmap
>  >>> >  Summary: FreeIPA Idmap library
>  >>> >  Group: Development/Libraries
>  >>> > @@ -205,7 +271,7 @@ used by Python applications.
>  >>> >  Summary: A library to allow communication between SUDO and SSSD
>  >>> >  Group: Development/Libraries
>  >>> >  License: LGPLv3+
>  >>> > -Requires: sssd = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> >  Requires(post): /sbin/ldconfig
>  >>> >  Requires(postun): /sbin/ldconfig
>  >>> 
>  >>> why libsss_idmap would require the sssd-ldap subpakage ?
>  >>
>  >>I think this was a mass-replace bug, fixed.
>  >>
>  >>> 
>  >>> Simo.
>  >>
>  >>There are also two patches preceding the one that splits the providers:
>  >>
>  >>[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD
> package
>  >>https://fedorahosted.org/sssd/ticket/1845
>  >>
>  >>libsss_sudo and libsss_autofs are separate packages that contain just a
>  >>single client library with no additional dependencies. This separation
>  >>comes from the F-17 timeframe where the feature was really just a tech
>  >>preview so we didn't want it to be packaged in sssd proper. On the other
>  >>hand users are getting regularly confused about "sudo not working"
> when
>  >>all they really miss is the single library.
>  >>
>  >>This patch moves the files owned by the libsss_autofs and libsss_sudo
>  >>packages back to the main sssd package. We also no longer build the
>  >>libsss_sudo documentation by default and do not ship the header file as
>  >>it was just a private one.
>  >>
>  >
>  >I tested upgrade with installed freeipa-client and sssd
>  >and upgrade worked as expected.
>  >
>  >ACK
>  >
>  >>
>  >>
>  >>[PATCH 2/3] rpm: Use hardened flags for RPM build
>  >>https://fedorahosted.org/sssd/ticket/1797
>  >>
>  >>This patch adds relro and bind_now linker flags to produce hardened
>  >>binaries. The change amounts to adding "-Wl,-z,now".
>  >
>  >Rpm packages are built without problems and Stephen blessed this patch.
>  >ACK
>  Second patch NACK.
>  
>  +export CFLAGS="%{optflags} -Wl,-z,now"
>                              ^^^^^^^^^^
>                          This is linker argument and should not be among CFLAGS 
Does it matter if it's preceded by -Wl ?
Would LDFLAGS be any better?

[Lukas Slebodnik]

On (06/06/13 23:46), Jakub Hrozek wrote:
> On Thu, Jun 06, 2013 at 09:56:20PM +0200, Lukas Slebodnik wrote:
> > On (06/06/13 20:11), Lukas Slebodnik wrote:
> > >On (05/06/13 14:50), Jakub Hrozek wrote:
> > >>On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
> > >>> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
> > >>> > 
> > >>> > Hi,
> > >>> > 
> > >>> > the attached patch splits the previously monolithic sssd package
> into
> > >>> > sssd-common that contains the deamon and the responders and
> > >>> > per-provider
> > >>> > packages such as sssd-ldap or sssd-ipa.
> > >>> > 
> > >>> > This split would benefit two parties:
> > >>> >     1) security auditors who are often trying to find the smallest
> > >>> > package
> > >>> >     set including dependencies needed for the package to function.
> > >>> > They
> > >>> >     would be able to i.e. install sssd-ldap and not bother about
> > >>> >     sssd-ipa or sssd-ad pulling in more dependencies.
> > >>> >     2) 3rd party programs such as realmd or authconfig that would
> only
> > >>> >     be able to require or install on demand the needed packages.
> > >>> > 
> > >>> > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and
> must
> > >>> > b
> > >>> > applied on the two specfile patches I sent earlier (the thread
> subject
> > >>> > included libsss_sudo).
> > >>> 
> > >>> Questions inline.
> > >>> 
> > >>
> > >>Not even nine months after the initial submission, here comes a revised
> > >>patch. I remember we had a discussion on IRC with Simo about this
> > >>problem, but I'll reply to the thread.
> > >>
> > >>With the Radius provider patches on the list and requiring Samba bits in
> > >>the last couple of releases, I think that splitting the providers is
> > >>something we really should do.
> > >>
> > >>> > 
> > >>> > 
> > >>> > 
> > >>> > 
> > >>> > 
> > >>> > 
> > >>> > plain text
> > >>> > document
> > >>> > attachment
> > >>> > (0001-Split-the-providers-into-separate-subpackages.patch)
> > >>> > 
> > >>> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00
> 2001
> > >>> > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
> > >>> > Date: Fri, 28 Sep 2012 09:21:18 +0200
> > >>> > Subject: [PATCH] Split the providers into separate subpackages
> > >>> > 
> > >>> > ---
> > >>> >  contrib/sssd.spec.in | 145
> > >>> > ++++++++++++++++++++++++++++++++++++++++-----------
> > >>> >  1 file changed, 115 insertions(+), 30 deletions(-)
> > >>> > 
> > >>> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> > >>> > index
> > >>> >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
> > >>> > --- a/contrib/sssd.spec.in
> > >>> > +++ b/contrib/sssd.spec.in
> > >>> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
> > >>> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> > >>> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
> > >>> >  
> > >>> >  ### Dependencies ###
> > >>> > -
> > >>> > -Requires: libldb >= 0.9.3
> > >>> > -Requires: libtdb >= 1.1.3
> > >>> > +Conflicts: sssd < %{version}-%{release}
> > >>> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
> > >>> > -Requires: libipa_hbac = %{version}-%{release}
> > >>> > -Requires: libsss_idmap = %{version}-%{release}
> > >>> > -Requires: cyrus-sasl-gssapi
> > >>> > -Requires: keyutils-libs
> > >>> > -Requires(post): initscripts chkconfig
> > >>> > -Requires(preun):  initscripts chkconfig
> > >>> > -Requires(postun): initscripts chkconfig
> > >>> > +Requires: sssd-common = %{version}-%{release}
> > >>> > +Requires: sssd-ldap = %{version}-%{release}
> > >>> > +Requires: sssd-krb5 = %{version}-%{release}
> > >>> > +Requires: sssd-ipa = %{version}-%{release}
> > >>> > +Requires: sssd-ad = %{version}-%{release}
> > >>> 
> > >>> 
> > >>> Doesn't this set of requires makes the split useless ?
> > >>> If I read it correctly it means sssd will require all subpackages
> anyway
> > >>> so you cannot pick and choose to install only one as you say the
> purpose
> > >>> is in the mail message.
> > >>> 
> > >>
> > >>The intent of the sssd package requiring all the dependencies is to make
> sure
> > >>that any kickstart that specified "sssd" would get the whole set,
> because
> > >>we can't currently know what functionality and which provider was used.
> > >>
> > >>To pick the minimal set for LDAP, you can run:
> > >># yum install sssd-ldap
> > >>for instance.
> > >>
> > >>> >  %global servicename sssd
> > >>> >  %global sssdstatedir %{_localstatedir}/lib/sss
> > >>> > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
> > >>> > connect to multiple different
> > >>> >  account sources. It is also the basis to provide client auditing
> and
> > >>> > policy
> > >>> >  services for projects like FreeIPA.
> > >>> >  
> > >>> > +%package common
> > >>> > +Summary: Common files for the SSSD
> > >>> > +Group: Applications/System
> > >>> > +License: GPLv3+
> > >>> > +Requires: libldb >= 0.9.3
> > >>> > +Requires: libtdb >= 1.1.3
> > >>> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
> > >>> > +Requires(post): initscripts chkconfig
> > >>> > +Requires(preun):  initscripts chkconfig
> > >>> > +Requires(postun): initscripts chkconfig
> > >>> > +Conflicts: sssd < %{version}-%{release}
> > >>> > +
> > >>> > +%description common
> > >>> > +Common files for the SSSD.
> > >>> > +
> > >>> >  %package client
> > >>> >  Summary: SSSD Client libraries for NSS and PAM
> > >>> >  Group: Applications/System
> > >>> > @@ -141,7 +152,7 @@ service.
> > >>> >  Summary: Userspace tools for use with the SSSD
> > >>> >  Group: Applications/System
> > >>> >  License: GPLv3+
> > >>> > -Requires: sssd = %{version}-%{release}
> > >>> > +Requires: sssd-common = %{version}-%{release}
> > >>> >  
> > >>> >  %description tools
> > >>> >  Provides userspace tools for manipulating users, groups, and
> nested
> > >>> > groups in
> > >>> > @@ -153,6 +164,61 @@ Also provides several other administrative
> tools:
> > >>> >      * sss_seed which pre-creates a user entry for use in
> kickstarts
> > >>> >      * sss_obfuscate for generating an obfuscated LDAP password
> > >>> >  
> > >>> > +%package ldap
> > >>> > +Summary: The LDAP back end of the SSSD
> > >>> > +Group: Applications/System
> > >>> > +License: GPLv3+
> > >>> > +Conflicts: sssd < %{version}-%{release}
> > >>> > +Requires: cyrus-sasl-gssapi
> > >>> > +Requires: sssd-common = %{version}-%{release}
> > >>> > +Requires: libsss_idmap = %{version}-%{release}
> > >>> > +
> > >>> > +%description ldap
> > >>> > +Provides the LDAP back end that the SSSD can utilize to fetch
> > >>> > identity data
> > >>> > +from and authenticate against an LDAP server.
> > >>> > +
> > >>> > +%package krb5
> > >>> > +Summary: The Kerberos authentication back end for the SSSD
> > >>> > +Group: Applications/System
> > >>> > +License: GPLv3+
> > >>> > +Conflicts: sssd < %{version}-%{release}
> > >>> > +Requires: cyrus-sasl-gssapi
> > >>> > +Requires: sssd-common = %{version}-%{release}
> > >>> > +
> > >>> > +%description krb5
> > >>> > +Provides the Kerberos back end that the SSSD can utilize
> authenticate
> > >>> > +against a Kerberos server.
> > >>> > +
> > >>> > +%package ipa
> > >>> > +Summary: The IPA back end of the SSSD
> > >>> > +Group: Applications/System
> > >>> > +License: GPLv3+
> > >>> > +Conflicts: sssd < %{version}-%{release}
> > >>> > +Requires: sssd-common = %{version}-%{release}
> > >>> > +Requires: sssd-ldap = %{version}-%{release}
> > >>> > +Requires: sssd-krb5 = %{version}-%{release}
> > >>> > +Requires: libipa_hbac = %{version}-%{release}
> > >>> > +Requires: libsss_idmap = %{version}-%{release}
> > >>> > +Requires: bind-utils
> > >>> 
> > >>> Does the ipa provider really need the sssd-ldap and sssd-krb5
> > >>> subpackages ?
> > >>> IIRC we statically compile the ldap and krb5 packages bits we need in
> > >>> the ipa provider.
> > >>> If you change this you probably want a require on cyrus-sasl-gssapi
> > >>> here.
> > >>> 
> > >>> (if it is just for the ldap and krb child processes shouldn't we
> simply
> > >>> keep those binaries in the sssd or sssd-common package ?)
> > >>
> > >>Yes, the intent was to make sure the ldap child and krb5 child processes
> > >>are pulled in. But now that we switched to internal shared libraries, I
> > >>think a better solution is to have the krb5_common internal shared
> > >>library along with the ldap and krb5 child in a subpackage of its own
> > >>and let the Kerberos-aware providers pull these in.
> > >>
> > >>> 
> > >>> 
> > >>> > +%description ipa
> > >>> > +Provides the IPA back end that the SSSD can utilize to fetch
> identity
> > >>> > data
> > >>> > +from and authenticate against an IPA server.
> > >>> > +
> > >>> > +%package ad
> > >>> > +Summary: The AD back end of the SSSD
> > >>> > +Group: Applications/System
> > >>> > +License: GPLv3+
> > >>> > +Conflicts: sssd < %{version}-%{release}
> > >>> > +Requires: sssd-common = %{version}-%{release}
> > >>> > +Requires: sssd-ldap = %{version}-%{release}
> > >>> > +Requires: sssd-krb5 = %{version}-%{release}
> > >>> > +Requires: libsss_idmap = %{version}-%{release}
> > >>> 
> > >>> SAme questions as for the ipa subpackage
> > >>> 
> > >>> > +%description ad
> > >>> > +Provides the Active Directory back end that the SSSD can utilize
> to
> > >>> > fetch
> > >>> > +identity data from and authenticate against an Active Directory
> > >>> > server.
> > >>> > +
> > >>> >  %package -n libsss_idmap
> > >>> >  Summary: FreeIPA Idmap library
> > >>> >  Group: Development/Libraries
> > >>> > @@ -205,7 +271,7 @@ used by Python applications.
> > >>> >  Summary: A library to allow communication between SUDO and SSSD
> > >>> >  Group: Development/Libraries
> > >>> >  License: LGPLv3+
> > >>> > -Requires: sssd = %{version}-%{release}
> > >>> > +Requires: sssd-ldap = %{version}-%{release}
> > >>> >  Requires(post): /sbin/ldconfig
> > >>> >  Requires(postun): /sbin/ldconfig
> > >>> 
> > >>> why libsss_idmap would require the sssd-ldap subpakage ?
> > >>
> > >>I think this was a mass-replace bug, fixed.
> > >>
> > >>> 
> > >>> Simo.
> > >>
> > >>There are also two patches preceding the one that splits the providers:
> > >>
> > >>[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD
> package
> > >>https://fedorahosted.org/sssd/ticket/1845
> > >>
> > >>libsss_sudo and libsss_autofs are separate packages that contain just a
> > >>single client library with no additional dependencies. This separation
> > >>comes from the F-17 timeframe where the feature was really just a tech
> > >>preview so we didn't want it to be packaged in sssd proper. On the other
> > >>hand users are getting regularly confused about "sudo not working"
> when
> > >>all they really miss is the single library.
> > >>
> > >>This patch moves the files owned by the libsss_autofs and libsss_sudo
> > >>packages back to the main sssd package. We also no longer build the
> > >>libsss_sudo documentation by default and do not ship the header file as
> > >>it was just a private one.
> > >>
> > >
> > >I tested upgrade with installed freeipa-client and sssd
> > >and upgrade worked as expected.
> > >
> > >ACK
> > >
> > >>
> > >>
> > >>[PATCH 2/3] rpm: Use hardened flags for RPM build
> > >>https://fedorahosted.org/sssd/ticket/1797
> > >>
> > >>This patch adds relro and bind_now linker flags to produce hardened
> > >>binaries. The change amounts to adding "-Wl,-z,now".
> > >
> > >Rpm packages are built without problems and Stephen blessed this patch.
> > >ACK
> > Second patch NACK.
> > 
> > +export CFLAGS="%{optflags} -Wl,-z,now"
> >                             ^^^^^^^^^^
> >                         This is linker argument and should not be among CFLAGS
>
> Does it matter if it's preceded by -Wl ? 
I am not sure, but I inspired with this
idea in koji build.

For example:
http://kojipkgs.fedoraproject.org//packages/sssd/1.10.0/6.fc19.beta1/data...
Only linker process used -Wl flags.
"-Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld"

> Would LDFLAGS be any better? 
Yes, this is the right variable,
but I am not sure about default values for it.
(like "%{optflags}" for CFLAGS)

LS

[Jakub Hrozek]

On Fri, Jun 07, 2013 at 10:14:36AM +0200, Lukas Slebodnik wrote:
>  On (06/06/13 23:46), Jakub Hrozek wrote:
>  >On Thu, Jun 06, 2013 at 09:56:20PM +0200, Lukas Slebodnik wrote:
>  >> On (06/06/13 20:11), Lukas Slebodnik wrote:
>  >> >On (05/06/13 14:50), Jakub Hrozek wrote:
>  >> >>On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
>  >> >>> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
>  >> >>> > 
>  >> >>> > Hi,
>  >> >>> > 
>  >> >>> > the attached patch splits the previously monolithic sssd
> package into
>  >> >>> > sssd-common that contains the deamon and the responders and
>  >> >>> > per-provider
>  >> >>> > packages such as sssd-ldap or sssd-ipa.
>  >> >>> > 
>  >> >>> > This split would benefit two parties:
>  >> >>> >     1) security auditors who are often trying to find the
> smallest
>  >> >>> > package
>  >> >>> >     set including dependencies needed for the package to
> function.
>  >> >>> > They
>  >> >>> >     would be able to i.e. install sssd-ldap and not bother
> about
>  >> >>> >     sssd-ipa or sssd-ad pulling in more dependencies.
>  >> >>> >     2) 3rd party programs such as realmd or authconfig that
> would only
>  >> >>> >     be able to require or install on demand the needed
> packages.
>  >> >>> > 
>  >> >>> > The patch addresses https://fedorahosted.org/sssd/ticket/1510
> and must
>  >> >>> > b
>  >> >>> > applied on the two specfile patches I sent earlier (the thread
> subject
>  >> >>> > included libsss_sudo).
>  >> >>> 
>  >> >>> Questions inline.
>  >> >>> 
>  >> >>
>  >> >>Not even nine months after the initial submission, here comes a revised
>  >> >>patch. I remember we had a discussion on IRC with Simo about this
>  >> >>problem, but I'll reply to the thread.
>  >> >>
>  >> >>With the Radius provider patches on the list and requiring Samba bits
> in
>  >> >>the last couple of releases, I think that splitting the providers is
>  >> >>something we really should do.
>  >> >>
>  >> >>> > 
>  >> >>> > 
>  >> >>> > 
>  >> >>> > 
>  >> >>> > 
>  >> >>> > 
>  >> >>> > plain text
>  >> >>> > document
>  >> >>> > attachment
>  >> >>> > (0001-Split-the-providers-into-separate-subpackages.patch)
>  >> >>> > 
>  >> >>> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17
> 00:00:00 2001
>  >> >>> > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
>  >> >>> > Date: Fri, 28 Sep 2012 09:21:18 +0200
>  >> >>> > Subject: [PATCH] Split the providers into separate
> subpackages
>  >> >>> > 
>  >> >>> > ---
>  >> >>> >  contrib/sssd.spec.in | 145
>  >> >>> > ++++++++++++++++++++++++++++++++++++++++-----------
>  >> >>> >  1 file changed, 115 insertions(+), 30 deletions(-)
>  >> >>> > 
>  >> >>> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  >> >>> > index
>  >> >>> >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
>  >> >>> > --- a/contrib/sssd.spec.in
>  >> >>> > +++ b/contrib/sssd.spec.in
>  >> >>> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
>  >> >>> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>  >> >>> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
>  >> >>> >  
>  >> >>> >  ### Dependencies ###
>  >> >>> > -
>  >> >>> > -Requires: libldb >= 0.9.3
>  >> >>> > -Requires: libtdb >= 1.1.3
>  >> >>> > +Conflicts: sssd < %{version}-%{release}
>  >> >>> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
>  >> >>> > -Requires: libipa_hbac = %{version}-%{release}
>  >> >>> > -Requires: libsss_idmap = %{version}-%{release}
>  >> >>> > -Requires: cyrus-sasl-gssapi
>  >> >>> > -Requires: keyutils-libs
>  >> >>> > -Requires(post): initscripts chkconfig
>  >> >>> > -Requires(preun):  initscripts chkconfig
>  >> >>> > -Requires(postun): initscripts chkconfig
>  >> >>> > +Requires: sssd-common = %{version}-%{release}
>  >> >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >> >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >> >>> > +Requires: sssd-ipa = %{version}-%{release}
>  >> >>> > +Requires: sssd-ad = %{version}-%{release}
>  >> >>> 
>  >> >>> 
>  >> >>> Doesn't this set of requires makes the split useless ?
>  >> >>> If I read it correctly it means sssd will require all subpackages
> anyway
>  >> >>> so you cannot pick and choose to install only one as you say the
> purpose
>  >> >>> is in the mail message.
>  >> >>> 
>  >> >>
>  >> >>The intent of the sssd package requiring all the dependencies is to make
> sure
>  >> >>that any kickstart that specified "sssd" would get the whole
> set, because
>  >> >>we can't currently know what functionality and which provider was
> used.
>  >> >>
>  >> >>To pick the minimal set for LDAP, you can run:
>  >> >># yum install sssd-ldap
>  >> >>for instance.
>  >> >>
>  >> >>> >  %global servicename sssd
>  >> >>> >  %global sssdstatedir %{_localstatedir}/lib/sss
>  >> >>> > @@ -126,6 +122,21 @@ the system and a pluggable backend system
> to
>  >> >>> > connect to multiple different
>  >> >>> >  account sources. It is also the basis to provide client
> auditing and
>  >> >>> > policy
>  >> >>> >  services for projects like FreeIPA.
>  >> >>> >  
>  >> >>> > +%package common
>  >> >>> > +Summary: Common files for the SSSD
>  >> >>> > +Group: Applications/System
>  >> >>> > +License: GPLv3+
>  >> >>> > +Requires: libldb >= 0.9.3
>  >> >>> > +Requires: libtdb >= 1.1.3
>  >> >>> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
>  >> >>> > +Requires(post): initscripts chkconfig
>  >> >>> > +Requires(preun):  initscripts chkconfig
>  >> >>> > +Requires(postun): initscripts chkconfig
>  >> >>> > +Conflicts: sssd < %{version}-%{release}
>  >> >>> > +
>  >> >>> > +%description common
>  >> >>> > +Common files for the SSSD.
>  >> >>> > +
>  >> >>> >  %package client
>  >> >>> >  Summary: SSSD Client libraries for NSS and PAM
>  >> >>> >  Group: Applications/System
>  >> >>> > @@ -141,7 +152,7 @@ service.
>  >> >>> >  Summary: Userspace tools for use with the SSSD
>  >> >>> >  Group: Applications/System
>  >> >>> >  License: GPLv3+
>  >> >>> > -Requires: sssd = %{version}-%{release}
>  >> >>> > +Requires: sssd-common = %{version}-%{release}
>  >> >>> >  
>  >> >>> >  %description tools
>  >> >>> >  Provides userspace tools for manipulating users, groups, and
> nested
>  >> >>> > groups in
>  >> >>> > @@ -153,6 +164,61 @@ Also provides several other
> administrative tools:
>  >> >>> >      * sss_seed which pre-creates a user entry for use in
> kickstarts
>  >> >>> >      * sss_obfuscate for generating an obfuscated LDAP
> password
>  >> >>> >  
>  >> >>> > +%package ldap
>  >> >>> > +Summary: The LDAP back end of the SSSD
>  >> >>> > +Group: Applications/System
>  >> >>> > +License: GPLv3+
>  >> >>> > +Conflicts: sssd < %{version}-%{release}
>  >> >>> > +Requires: cyrus-sasl-gssapi
>  >> >>> > +Requires: sssd-common = %{version}-%{release}
>  >> >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >> >>> > +
>  >> >>> > +%description ldap
>  >> >>> > +Provides the LDAP back end that the SSSD can utilize to
> fetch
>  >> >>> > identity data
>  >> >>> > +from and authenticate against an LDAP server.
>  >> >>> > +
>  >> >>> > +%package krb5
>  >> >>> > +Summary: The Kerberos authentication back end for the SSSD
>  >> >>> > +Group: Applications/System
>  >> >>> > +License: GPLv3+
>  >> >>> > +Conflicts: sssd < %{version}-%{release}
>  >> >>> > +Requires: cyrus-sasl-gssapi
>  >> >>> > +Requires: sssd-common = %{version}-%{release}
>  >> >>> > +
>  >> >>> > +%description krb5
>  >> >>> > +Provides the Kerberos back end that the SSSD can utilize
> authenticate
>  >> >>> > +against a Kerberos server.
>  >> >>> > +
>  >> >>> > +%package ipa
>  >> >>> > +Summary: The IPA back end of the SSSD
>  >> >>> > +Group: Applications/System
>  >> >>> > +License: GPLv3+
>  >> >>> > +Conflicts: sssd < %{version}-%{release}
>  >> >>> > +Requires: sssd-common = %{version}-%{release}
>  >> >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >> >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >> >>> > +Requires: libipa_hbac = %{version}-%{release}
>  >> >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >> >>> > +Requires: bind-utils
>  >> >>> 
>  >> >>> Does the ipa provider really need the sssd-ldap and sssd-krb5
>  >> >>> subpackages ?
>  >> >>> IIRC we statically compile the ldap and krb5 packages bits we need
> in
>  >> >>> the ipa provider.
>  >> >>> If you change this you probably want a require on
> cyrus-sasl-gssapi
>  >> >>> here.
>  >> >>> 
>  >> >>> (if it is just for the ldap and krb child processes shouldn't
> we simply
>  >> >>> keep those binaries in the sssd or sssd-common package ?)
>  >> >>
>  >> >>Yes, the intent was to make sure the ldap child and krb5 child
> processes
>  >> >>are pulled in. But now that we switched to internal shared libraries, I
>  >> >>think a better solution is to have the krb5_common internal shared
>  >> >>library along with the ldap and krb5 child in a subpackage of its own
>  >> >>and let the Kerberos-aware providers pull these in.
>  >> >>
>  >> >>> 
>  >> >>> 
>  >> >>> > +%description ipa
>  >> >>> > +Provides the IPA back end that the SSSD can utilize to fetch
> identity
>  >> >>> > data
>  >> >>> > +from and authenticate against an IPA server.
>  >> >>> > +
>  >> >>> > +%package ad
>  >> >>> > +Summary: The AD back end of the SSSD
>  >> >>> > +Group: Applications/System
>  >> >>> > +License: GPLv3+
>  >> >>> > +Conflicts: sssd < %{version}-%{release}
>  >> >>> > +Requires: sssd-common = %{version}-%{release}
>  >> >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >> >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >> >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >> >>> 
>  >> >>> SAme questions as for the ipa subpackage
>  >> >>> 
>  >> >>> > +%description ad
>  >> >>> > +Provides the Active Directory back end that the SSSD can
> utilize to
>  >> >>> > fetch
>  >> >>> > +identity data from and authenticate against an Active
> Directory
>  >> >>> > server.
>  >> >>> > +
>  >> >>> >  %package -n libsss_idmap
>  >> >>> >  Summary: FreeIPA Idmap library
>  >> >>> >  Group: Development/Libraries
>  >> >>> > @@ -205,7 +271,7 @@ used by Python applications.
>  >> >>> >  Summary: A library to allow communication between SUDO and
> SSSD
>  >> >>> >  Group: Development/Libraries
>  >> >>> >  License: LGPLv3+
>  >> >>> > -Requires: sssd = %{version}-%{release}
>  >> >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >> >>> >  Requires(post): /sbin/ldconfig
>  >> >>> >  Requires(postun): /sbin/ldconfig
>  >> >>> 
>  >> >>> why libsss_idmap would require the sssd-ldap subpakage ?
>  >> >>
>  >> >>I think this was a mass-replace bug, fixed.
>  >> >>
>  >> >>> 
>  >> >>> Simo.
>  >> >>
>  >> >>There are also two patches preceding the one that splits the providers:
>  >> >>
>  >> >>[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main
> SSSD package
>  >> >>https://fedorahosted.org/sssd/ticket/1845
>  >> >>
>  >> >>libsss_sudo and libsss_autofs are separate packages that contain just a
>  >> >>single client library with no additional dependencies. This separation
>  >> >>comes from the F-17 timeframe where the feature was really just a tech
>  >> >>preview so we didn't want it to be packaged in sssd proper. On the
> other
>  >> >>hand users are getting regularly confused about "sudo not
> working" when
>  >> >>all they really miss is the single library.
>  >> >>
>  >> >>This patch moves the files owned by the libsss_autofs and libsss_sudo
>  >> >>packages back to the main sssd package. We also no longer build the
>  >> >>libsss_sudo documentation by default and do not ship the header file as
>  >> >>it was just a private one.
>  >> >>
>  >> >
>  >> >I tested upgrade with installed freeipa-client and sssd
>  >> >and upgrade worked as expected.
>  >> >
>  >> >ACK
>  >> >
>  >> >>
>  >> >>
>  >> >>[PATCH 2/3] rpm: Use hardened flags for RPM build
>  >> >>https://fedorahosted.org/sssd/ticket/1797
>  >> >>
>  >> >>This patch adds relro and bind_now linker flags to produce hardened
>  >> >>binaries. The change amounts to adding "-Wl,-z,now".
>  >> >
>  >> >Rpm packages are built without problems and Stephen blessed this patch.
>  >> >ACK
>  >> Second patch NACK.
>  >> 
>  >> +export CFLAGS="%{optflags} -Wl,-z,now"
>  >>                             ^^^^^^^^^^
>  >>                         This is linker argument and should not be among CFLAGS
>  >
>  >Does it matter if it's preceded by -Wl ?
>  I am not sure, but I inspired with this idea in koji build.
>  
>  For example:
>
> http://kojipkgs.fedoraproject.org//packages/sssd/1.10.0/6.fc19.beta1/data...
>  Only linker process used -Wl flags.
>  "-Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld"
>  
>  >Would LDFLAGS be any better?
>  Yes, this is the right variable, but I am not sure about default values for it.
>  (like "%{optflags}" for CFLAGS)
>  
>  LS 
OK, after some thinking I resorted to copying the approach from the
downstream specfile -- just use the _hardened_build flag.

The upside is that we get the same predictable results as the
Fedora/RHEL builds, the downside is that the approach wouldn't work on
other RPM based distributions like Suse. But I'm not sure if the
specfile would have worked even before the change, I suspect we might be
using some Fedora specific macros anyway.

I'll attach the updated patch to reply to your other review mail.

[Jakub Hrozek]

On Thu, Jun 06, 2013 at 09:56:20PM +0200, Lukas Slebodnik wrote:
>  On (06/06/13 20:11), Lukas Slebodnik wrote:
>  >On (05/06/13 14:50), Jakub Hrozek wrote:
>  >>On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
>  >>> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
>  >>> > 
>  >>> > Hi,
>  >>> > 
>  >>> > the attached patch splits the previously monolithic sssd package into
>  >>> > sssd-common that contains the deamon and the responders and
>  >>> > per-provider
>  >>> > packages such as sssd-ldap or sssd-ipa.
>  >>> > 
>  >>> > This split would benefit two parties:
>  >>> >     1) security auditors who are often trying to find the smallest
>  >>> > package
>  >>> >     set including dependencies needed for the package to function.
>  >>> > They
>  >>> >     would be able to i.e. install sssd-ldap and not bother about
>  >>> >     sssd-ipa or sssd-ad pulling in more dependencies.
>  >>> >     2) 3rd party programs such as realmd or authconfig that would only
>  >>> >     be able to require or install on demand the needed packages.
>  >>> > 
>  >>> > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
>  >>> > b
>  >>> > applied on the two specfile patches I sent earlier (the thread subject
>  >>> > included libsss_sudo).
>  >>> 
>  >>> Questions inline.
>  >>> 
>  >>
>  >>Not even nine months after the initial submission, here comes a revised
>  >>patch. I remember we had a discussion on IRC with Simo about this
>  >>problem, but I'll reply to the thread.
>  >>
>  >>With the Radius provider patches on the list and requiring Samba bits in
>  >>the last couple of releases, I think that splitting the providers is
>  >>something we really should do.
>  >>
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > 
>  >>> > plain text
>  >>> > document
>  >>> > attachment
>  >>> > (0001-Split-the-providers-into-separate-subpackages.patch)
>  >>> > 
>  >>> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
>  >>> > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
>  >>> > Date: Fri, 28 Sep 2012 09:21:18 +0200
>  >>> > Subject: [PATCH] Split the providers into separate subpackages
>  >>> > 
>  >>> > ---
>  >>> >  contrib/sssd.spec.in | 145
>  >>> > ++++++++++++++++++++++++++++++++++++++++-----------
>  >>> >  1 file changed, 115 insertions(+), 30 deletions(-)
>  >>> > 
>  >>> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  >>> > index
>  >>> >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
>  >>> > --- a/contrib/sssd.spec.in
>  >>> > +++ b/contrib/sssd.spec.in
>  >>> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
>  >>> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>  >>> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
>  >>> >  
>  >>> >  ### Dependencies ###
>  >>> > -
>  >>> > -Requires: libldb >= 0.9.3
>  >>> > -Requires: libtdb >= 1.1.3
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
>  >>> > -Requires: libipa_hbac = %{version}-%{release}
>  >>> > -Requires: libsss_idmap = %{version}-%{release}
>  >>> > -Requires: cyrus-sasl-gssapi
>  >>> > -Requires: keyutils-libs
>  >>> > -Requires(post): initscripts chkconfig
>  >>> > -Requires(preun):  initscripts chkconfig
>  >>> > -Requires(postun): initscripts chkconfig
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >>> > +Requires: sssd-ipa = %{version}-%{release}
>  >>> > +Requires: sssd-ad = %{version}-%{release}
>  >>> 
>  >>> 
>  >>> Doesn't this set of requires makes the split useless ?
>  >>> If I read it correctly it means sssd will require all subpackages anyway
>  >>> so you cannot pick and choose to install only one as you say the purpose
>  >>> is in the mail message.
>  >>> 
>  >>
>  >>The intent of the sssd package requiring all the dependencies is to make sure
>  >>that any kickstart that specified "sssd" would get the whole set,
> because
>  >>we can't currently know what functionality and which provider was used.
>  >>
>  >>To pick the minimal set for LDAP, you can run:
>  >># yum install sssd-ldap
>  >>for instance.
>  >>
>  >>> >  %global servicename sssd
>  >>> >  %global sssdstatedir %{_localstatedir}/lib/sss
>  >>> > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
>  >>> > connect to multiple different
>  >>> >  account sources. It is also the basis to provide client auditing and
>  >>> > policy
>  >>> >  services for projects like FreeIPA.
>  >>> >  
>  >>> > +%package common
>  >>> > +Summary: Common files for the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Requires: libldb >= 0.9.3
>  >>> > +Requires: libtdb >= 1.1.3
>  >>> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
>  >>> > +Requires(post): initscripts chkconfig
>  >>> > +Requires(preun):  initscripts chkconfig
>  >>> > +Requires(postun): initscripts chkconfig
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +
>  >>> > +%description common
>  >>> > +Common files for the SSSD.
>  >>> > +
>  >>> >  %package client
>  >>> >  Summary: SSSD Client libraries for NSS and PAM
>  >>> >  Group: Applications/System
>  >>> > @@ -141,7 +152,7 @@ service.
>  >>> >  Summary: Userspace tools for use with the SSSD
>  >>> >  Group: Applications/System
>  >>> >  License: GPLv3+
>  >>> > -Requires: sssd = %{version}-%{release}
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> >  
>  >>> >  %description tools
>  >>> >  Provides userspace tools for manipulating users, groups, and nested
>  >>> > groups in
>  >>> > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
>  >>> >      * sss_seed which pre-creates a user entry for use in kickstarts
>  >>> >      * sss_obfuscate for generating an obfuscated LDAP password
>  >>> >  
>  >>> > +%package ldap
>  >>> > +Summary: The LDAP back end of the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: cyrus-sasl-gssapi
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >>> > +
>  >>> > +%description ldap
>  >>> > +Provides the LDAP back end that the SSSD can utilize to fetch
>  >>> > identity data
>  >>> > +from and authenticate against an LDAP server.
>  >>> > +
>  >>> > +%package krb5
>  >>> > +Summary: The Kerberos authentication back end for the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: cyrus-sasl-gssapi
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +
>  >>> > +%description krb5
>  >>> > +Provides the Kerberos back end that the SSSD can utilize authenticate
>  >>> > +against a Kerberos server.
>  >>> > +
>  >>> > +%package ipa
>  >>> > +Summary: The IPA back end of the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >>> > +Requires: libipa_hbac = %{version}-%{release}
>  >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >>> > +Requires: bind-utils
>  >>> 
>  >>> Does the ipa provider really need the sssd-ldap and sssd-krb5
>  >>> subpackages ?
>  >>> IIRC we statically compile the ldap and krb5 packages bits we need in
>  >>> the ipa provider.
>  >>> If you change this you probably want a require on cyrus-sasl-gssapi
>  >>> here.
>  >>> 
>  >>> (if it is just for the ldap and krb child processes shouldn't we simply
>  >>> keep those binaries in the sssd or sssd-common package ?)
>  >>
>  >>Yes, the intent was to make sure the ldap child and krb5 child processes
>  >>are pulled in. But now that we switched to internal shared libraries, I
>  >>think a better solution is to have the krb5_common internal shared
>  >>library along with the ldap and krb5 child in a subpackage of its own
>  >>and let the Kerberos-aware providers pull these in.
>  >>
>  >>> 
>  >>> 
>  >>> > +%description ipa
>  >>> > +Provides the IPA back end that the SSSD can utilize to fetch identity
>  >>> > data
>  >>> > +from and authenticate against an IPA server.
>  >>> > +
>  >>> > +%package ad
>  >>> > +Summary: The AD back end of the SSSD
>  >>> > +Group: Applications/System
>  >>> > +License: GPLv3+
>  >>> > +Conflicts: sssd < %{version}-%{release}
>  >>> > +Requires: sssd-common = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> > +Requires: sssd-krb5 = %{version}-%{release}
>  >>> > +Requires: libsss_idmap = %{version}-%{release}
>  >>> 
>  >>> SAme questions as for the ipa subpackage
>  >>> 
>  >>> > +%description ad
>  >>> > +Provides the Active Directory back end that the SSSD can utilize to
>  >>> > fetch
>  >>> > +identity data from and authenticate against an Active Directory
>  >>> > server.
>  >>> > +
>  >>> >  %package -n libsss_idmap
>  >>> >  Summary: FreeIPA Idmap library
>  >>> >  Group: Development/Libraries
>  >>> > @@ -205,7 +271,7 @@ used by Python applications.
>  >>> >  Summary: A library to allow communication between SUDO and SSSD
>  >>> >  Group: Development/Libraries
>  >>> >  License: LGPLv3+
>  >>> > -Requires: sssd = %{version}-%{release}
>  >>> > +Requires: sssd-ldap = %{version}-%{release}
>  >>> >  Requires(post): /sbin/ldconfig
>  >>> >  Requires(postun): /sbin/ldconfig
>  >>> 
>  >>> why libsss_idmap would require the sssd-ldap subpakage ?
>  >>
>  >>I think this was a mass-replace bug, fixed.
>  >>
>  >>> 
>  >>> Simo.
>  >>
>  >>There are also two patches preceding the one that splits the providers:
>  >>
>  >>[PATCH 1/3] rpm: Fold libsss_sudo and libsss_autofs back into the main SSSD
> package
>  >>https://fedorahosted.org/sssd/ticket/1845
>  >>
>  >>libsss_sudo and libsss_autofs are separate packages that contain just a
>  >>single client library with no additional dependencies. This separation
>  >>comes from the F-17 timeframe where the feature was really just a tech
>  >>preview so we didn't want it to be packaged in sssd proper. On the other
>  >>hand users are getting regularly confused about "sudo not working"
> when
>  >>all they really miss is the single library.
>  >>
>  >>This patch moves the files owned by the libsss_autofs and libsss_sudo
>  >>packages back to the main sssd package. We also no longer build the
>  >>libsss_sudo documentation by default and do not ship the header file as
>  >>it was just a private one.
>  >>
>  >
>  >I tested upgrade with installed freeipa-client and sssd
>  >and upgrade worked as expected.
>  >
>  >ACK 
Patch 1/3 was pushed to master.

[Lukas Slebodnik]

On (05/06/13 14:50), Jakub Hrozek wrote:
> On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
> > On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
> > > 
> > > Hi,
> > > 
> > > the attached patch splits the previously monolithic sssd package into
> > > sssd-common that contains the deamon and the responders and
> > > per-provider
> > > packages such as sssd-ldap or sssd-ipa.
> > > 
> > > This split would benefit two parties:
> > >     1) security auditors who are often trying to find the smallest
> > > package
> > >     set including dependencies needed for the package to function.
> > > They
> > >     would be able to i.e. install sssd-ldap and not bother about
> > >     sssd-ipa or sssd-ad pulling in more dependencies.
> > >     2) 3rd party programs such as realmd or authconfig that would only
> > >     be able to require or install on demand the needed packages.
> > > 
> > > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
> > > b
> > > applied on the two specfile patches I sent earlier (the thread subject
> > > included libsss_sudo).
> > 
> > Questions inline.
> > 
>
> Not even nine months after the initial submission, here comes a revised
> patch. I remember we had a discussion on IRC with Simo about this
> problem, but I'll reply to the thread.
>
> With the Radius provider patches on the list and requiring Samba bits in
> the last couple of releases, I think that splitting the providers is
> something we really should do.
>
> > > plain text
> > > document
> > > attachment
> > > (0001-Split-the-providers-into-separate-subpackages.patch)
> > > 
> > > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
> > > From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
> > > Date: Fri, 28 Sep 2012 09:21:18 +0200
> > > Subject: [PATCH] Split the providers into separate subpackages
> > > 
> > > ---
> > >  contrib/sssd.spec.in | 145
> > > ++++++++++++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 115 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> > > index
> > >
> e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
> > > --- a/contrib/sssd.spec.in
> > > +++ b/contrib/sssd.spec.in
> > > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
> > > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> > >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
> > >  
> > >  ### Dependencies ###
> > > -
> > > -Requires: libldb >= 0.9.3
> > > -Requires: libtdb >= 1.1.3
> > > +Conflicts: sssd < %{version}-%{release}
> > >  Requires: sssd-client%{?_isa} = %{version}-%{release}
> > > -Requires: libipa_hbac = %{version}-%{release}
> > > -Requires: libsss_idmap = %{version}-%{release}
> > > -Requires: cyrus-sasl-gssapi
> > > -Requires: keyutils-libs
> > > -Requires(post): initscripts chkconfig
> > > -Requires(preun):  initscripts chkconfig
> > > -Requires(postun): initscripts chkconfig
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > > +Requires: sssd-krb5 = %{version}-%{release}
> > > +Requires: sssd-ipa = %{version}-%{release}
> > > +Requires: sssd-ad = %{version}-%{release}
> > 
> > 
> > Doesn't this set of requires makes the split useless ?
> > If I read it correctly it means sssd will require all subpackages anyway
> > so you cannot pick and choose to install only one as you say the purpose
> > is in the mail message.
> > 
>
> The intent of the sssd package requiring all the dependencies is to make sure
> that any kickstart that specified "sssd" would get the whole set, because
> we can't currently know what functionality and which provider was used.
>
> To pick the minimal set for LDAP, you can run:
> # yum install sssd-ldap
> for instance.
>
> > >  %global servicename sssd
> > >  %global sssdstatedir %{_localstatedir}/lib/sss
> > > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
> > > connect to multiple different
> > >  account sources. It is also the basis to provide client auditing and
> > > policy
> > >  services for projects like FreeIPA.
> > >  
> > > +%package common
> > > +Summary: Common files for the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Requires: libldb >= 0.9.3
> > > +Requires: libtdb >= 1.1.3
> > > +Requires: sssd-client%{?_isa} = %{version}-%{release}
> > > +Requires(post): initscripts chkconfig
> > > +Requires(preun):  initscripts chkconfig
> > > +Requires(postun): initscripts chkconfig
> > > +Conflicts: sssd < %{version}-%{release}
> > > +
> > > +%description common
> > > +Common files for the SSSD.
> > > +
> > >  %package client
> > >  Summary: SSSD Client libraries for NSS and PAM
> > >  Group: Applications/System
> > > @@ -141,7 +152,7 @@ service.
> > >  Summary: Userspace tools for use with the SSSD
> > >  Group: Applications/System
> > >  License: GPLv3+
> > > -Requires: sssd = %{version}-%{release}
> > > +Requires: sssd-common = %{version}-%{release}
> > >  
> > >  %description tools
> > >  Provides userspace tools for manipulating users, groups, and nested
> > > groups in
> > > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
> > >      * sss_seed which pre-creates a user entry for use in kickstarts
> > >      * sss_obfuscate for generating an obfuscated LDAP password
> > >  
> > > +%package ldap
> > > +Summary: The LDAP back end of the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: cyrus-sasl-gssapi
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: libsss_idmap = %{version}-%{release}
> > > +
> > > +%description ldap
> > > +Provides the LDAP back end that the SSSD can utilize to fetch
> > > identity data
> > > +from and authenticate against an LDAP server.
> > > +
> > > +%package krb5
> > > +Summary: The Kerberos authentication back end for the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: cyrus-sasl-gssapi
> > > +Requires: sssd-common = %{version}-%{release}
> > > +
> > > +%description krb5
> > > +Provides the Kerberos back end that the SSSD can utilize authenticate
> > > +against a Kerberos server.
> > > +
> > > +%package ipa
> > > +Summary: The IPA back end of the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > > +Requires: sssd-krb5 = %{version}-%{release}
> > > +Requires: libipa_hbac = %{version}-%{release}
> > > +Requires: libsss_idmap = %{version}-%{release}
> > > +Requires: bind-utils
> > 
> > Does the ipa provider really need the sssd-ldap and sssd-krb5
> > subpackages ?
> > IIRC we statically compile the ldap and krb5 packages bits we need in
> > the ipa provider.
> > If you change this you probably want a require on cyrus-sasl-gssapi
> > here.
> > 
> > (if it is just for the ldap and krb child processes shouldn't we simply
> > keep those binaries in the sssd or sssd-common package ?)
>
> Yes, the intent was to make sure the ldap child and krb5 child processes
> are pulled in. But now that we switched to internal shared libraries, I
> think a better solution is to have the krb5_common internal shared
> library along with the ldap and krb5 child in a subpackage of its own
> and let the Kerberos-aware providers pull these in.
>
> > 
> > 
> > > +%description ipa
> > > +Provides the IPA back end that the SSSD can utilize to fetch identity
> > > data
> > > +from and authenticate against an IPA server.
> > > +
> > > +%package ad
> > > +Summary: The AD back end of the SSSD
> > > +Group: Applications/System
> > > +License: GPLv3+
> > > +Conflicts: sssd < %{version}-%{release}
> > > +Requires: sssd-common = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > > +Requires: sssd-krb5 = %{version}-%{release}
> > > +Requires: libsss_idmap = %{version}-%{release}
> > 
> > SAme questions as for the ipa subpackage
> > 
> > > +%description ad
> > > +Provides the Active Directory back end that the SSSD can utilize to
> > > fetch
> > > +identity data from and authenticate against an Active Directory
> > > server.
> > > +
> > >  %package -n libsss_idmap
> > >  Summary: FreeIPA Idmap library
> > >  Group: Development/Libraries
> > > @@ -205,7 +271,7 @@ used by Python applications.
> > >  Summary: A library to allow communication between SUDO and SSSD
> > >  Group: Development/Libraries
> > >  License: LGPLv3+
> > > -Requires: sssd = %{version}-%{release}
> > > +Requires: sssd-ldap = %{version}-%{release}
> > >  Requires(post): /sbin/ldconfig
> > >  Requires(postun): /sbin/ldconfig
> > 
> > why libsss_idmap would require the sssd-ldap subpakage ?
>
> I think this was a mass-replace bug, fixed.
>
> > 
> > Simo. 
1. I like idea of divided subpackages. If someone wants only ldap backend, he
needn't install samba-libs (and its dependencies)

2. There isn't any rpmlint warnings.

I tested yum upgrade upgrade with installed sssd and freeipa-client.
New packages were installed for dependencies:
   sssd-ad sssd-common sssd-ipa sssd-krb5 sssd-krb5-common sssd-ldap
Everything worked as expected.

Then I decided to remove sssd-ad:
   yum remove sssd-ad
and packeges "freeipa-client, sssd" were also removed.
I was little bit confused, because I didn't want to remove sssd
and sssd replied to getent command after packages "freeipa-client, sssd" were
removed.

I think, that other users may be also confused with this situation.
Then I looked to the patch and I found out, that:
  --sssd is only "meta package",which require all backedns subpackages
  --sssd doesn't contain any useful files
  --everything important is in package sssd-common.


Maybe we should update package description of sssd and sssd-common.
I hope that system administrators relies on output of "yum info"
and there isn't it very well explained.

Summary:
Everything works well, but I was little bit confused.

Any other opinions?

One nitpick inline


> From 4498f26c78fcba9da4aaa2e8fb34734106f5227d Mon Sep 17 00:00:00
> 2001
> From: Jakub Hrozek <jhrozek(a)redhat.com&gt;
> Date: Wed, 5 Jun 2013 12:53:30 +0200
> Subject: [PATCH 3/3] rpm: Split providers into separate subpackages
>
> https://fedorahosted.org/sssd/ticket/1510
>
> This patch splits the previously monolithic sssd package into sssd-common
> that contains the deamon and the responders and per-provider packages
> such as sssd-ldap or sssd-ipa.
>
> This split would benefit two parties:
> 1) security auditors who are often trying to find the smallest package
>    set including dependencies needed for the package to function.
>    They would be able to i.e. install sssd-ldap and not bother
>    about sssd-ipa or sssd-ad pulling in more dependencies.
> 2) 3rd party programs such as realmd or authconfig
>    that would only be able to require or install on demand the
>    needed packages.
> ---
>  contrib/sssd.spec.in | 188 +++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 145 insertions(+), 43 deletions(-)
>
> diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> index d59f684400b6eab4f65a93552ae73a85445ac3e2..ddc364f878bf859116ed79a8aa4f55963d90dfc8
> 100644
> --- a/contrib/sssd.spec.in
> +++ b/contrib/sssd.spec.in
> @@ -56,30 +56,13 @@ Patch0001:  sssd-1.9-man-change-default-ccache.patch
>  
>  ### Dependencies ###
>  
> -Requires: libldb >= 0.9.3
> -Requires: libtdb >= 1.1.3
> -Requires: sssd-client%{?_isa} = %{version}-%{release}
> -Requires: libipa_hbac = %{version}-%{release}
> -Requires: libsss_idmap = %{version}-%{release}
> +Conflicts: sssd < %{version}-%{release}
> +Requires: sssd-common = %{version}-%{release}
> +Requires: sssd-ldap = %{version}-%{release}
> +Requires: sssd-krb5 = %{version}-%{release}
> +Requires: sssd-ipa = %{version}-%{release}
> +Requires: sssd-ad = %{version}-%{release}
>  Requires: python-sssdconfig = %{version}-%{release}
> -Requires: cyrus-sasl-gssapi
> -%if (0%{?use_systemd} == 1)
> -Requires(post): systemd-units systemd-sysv
> -Requires(preun): systemd-units
> -Requires(postun): systemd-units
> -%else
> -Requires(post): initscripts chkconfig
> -Requires(preun):  initscripts chkconfig
> -Requires(postun): initscripts chkconfig
> -%endif
> -
> -### Provides ###
> -Provides: libsss_sudo = %{version}-%{release}
> -Obsoletes: libsss_sudo < %{version}-%{release}
> -Provides: libsss_sudo-devel = %{version}-%{release}
> -Obsoletes: libsss_sudo-devel < %{version}-%{release}
> -Provides: libsss_autofs = %{version}-%{release}
> -Obsoletes: libsss_autofs < %{version}-%{release}
>  
>  %global servicename sssd
>  %global sssdstatedir %{_localstatedir}/lib/sss
> @@ -155,6 +138,35 @@ the system and a pluggable backend system to connect to multiple
> different
>  account sources. It is also the basis to provide client auditing and policy
>  services for projects like FreeIPA.
>  
> +%package common
> +Summary: Common files for the SSSD
> +Group: Applications/System
> +License: GPLv3+
> +Requires: libldb >= 0.9.3
> +Requires: libtdb >= 1.1.3
> +Requires: sssd-client%{?_isa} = %{version}-%{release}
> +Conflicts: sssd < %{version}-%{release}
> +%if (0%{?use_systemd} == 1)
> +Requires(post): systemd-units systemd-sysv
> +Requires(preun): systemd-units
> +Requires(postun): systemd-units
> +%else
> +Requires(post): initscripts chkconfig
> +Requires(preun):  initscripts chkconfig
> +Requires(postun): initscripts chkconfig
> +%endif
> +
> +### Provides ###
> +Provides: libsss_sudo = %{version}-%{release}
> +Obsoletes: libsss_sudo < %{version}-%{release}
> +Provides: libsss_sudo-devel = %{version}-%{release}
> +Obsoletes: libsss_sudo-devel < %{version}-%{release}
> +Provides: libsss_autofs = %{version}-%{release}
> +Obsoletes: libsss_autofs < %{version}-%{release}
> +
> +%description common
> +Common files for the SSSD.
> +
>  %package client
>  Summary: SSSD Client libraries for NSS and PAM
>  Group: Applications/System
> @@ -170,7 +182,7 @@ service.
>  Summary: Userspace tools for use with the SSSD
>  Group: Applications/System
>  License: GPLv3+
> -Requires: sssd = %{version}-%{release}
> +Requires: sssd-common = %{version}-%{release}
>  
>  %description tools
>  Provides userspace tools for manipulating users, groups, and nested groups in
> @@ -190,6 +202,73 @@ BuildArch: noarch
>  %description -n python-sssdconfig
>  Provides python files for manipulation SSSD and IPA configuration files.
>  
> +%package ldap
> +Summary: The LDAP back end of the SSSD
> +Group: Applications/System
> +License: GPLv3+
> +Conflicts: sssd < %{version}-%{release}
> +Requires: sssd-common = %{version}-%{release}
> +Requires: libsss_idmap = %{version}-%{release}
> +Requires: sssd-krb5-common = %{version}-%{release}
> +
> +%description ldap
> +Provides the LDAP back end that the SSSD can utilize to fetch identity data
> +from and authenticate against an LDAP server.
> +
> +%package krb5-common
> +Summary: SSSD helpers needed for Kerberos and GSSAPI authentication
> +Group: Applications/System
> +License: GPLv3+
> +Conflicts: sssd < %{version}-%{release}
> +Requires: cyrus-sasl-gssapi
> +Requires: sssd-common = %{version}-%{release}
> +Requires: sssd-krb5-common = %{version}-%{release} 
            ^^^^^^^^^^^^^^^^
Package sssd-krb5-common requires itself?
> +
> +%description krb5-common
> +Provides helper processes that the LDAP and Kerberos back ends can use for
> +Kerberos user or host authentication.
> +
> +%package krb5
> +Summary: The Kerberos authentication back end for the SSSD
> +Group: Applications/System
> +License: GPLv3+
> +Conflicts: sssd < %{version}-%{release}
> +Requires: sssd-common = %{version}-%{release}
> +Requires: sssd-krb5-common = %{version}-%{release}
> +
> +%description krb5
> +Provides the Kerberos back end that the SSSD can utilize authenticate
> +against a Kerberos server.
> +
> +%package ipa
> +Summary: The IPA back end of the SSSD
> +Group: Applications/System
> +License: GPLv3+
> +Conflicts: sssd < %{version}-%{release}
> +Requires: sssd-common = %{version}-%{release}
> +Requires: sssd-krb5-common = %{version}-%{release}
> +Requires: libipa_hbac = %{version}-%{release}
> +Requires: libsss_idmap = %{version}-%{release}
> +Requires: bind-utils
> +
> +%description ipa
> +Provides the IPA back end that the SSSD can utilize to fetch identity data
> +from and authenticate against an IPA server.
> +
> +%package ad
> +Summary: The AD back end of the SSSD
> +Group: Applications/System
> +License: GPLv3+
> +Conflicts: sssd < %{version}-%{release}
> +Requires: sssd-common = %{version}-%{release}
> +Requires: sssd-krb5-common = %{version}-%{release}
> +Requires: libsss_idmap = %{version}-%{release}
> +Requires: bind-utils
> +
> +%description ad
> +Provides the Active Directory back end that the SSSD can utilize to fetch
> +identity data from and authenticate against an Active Directory server.
> +
>  %package -n libsss_idmap
>  Summary: FreeIPA Idmap library
>  Group: Development/Libraries
> @@ -386,7 +465,11 @@ touch $RPM_BUILD_ROOT/%{mcpath}/group
>  %clean
>  rm -rf $RPM_BUILD_ROOT
>  
> -%files -f sssd.lang
> +%files
> +%defattr(-,root,root,-)
> +%doc COPYING
> +
> +%files common -f sssd.lang
>  %defattr(-,root,root,-)
>  %doc COPYING
>  %doc src/examples/sssd-example.conf
> @@ -398,8 +481,6 @@ rm -rf $RPM_BUILD_ROOT
>  %endif
>  
>  %dir %{_libexecdir}/%{servicename}
> -%{_libexecdir}/%{servicename}/krb5_child
> -%{_libexecdir}/%{servicename}/ldap_child
>  %{_libexecdir}/%{servicename}/proxy_child
>  %{_libexecdir}/%{servicename}/sssd_be
>  %{_libexecdir}/%{servicename}/sssd_nss
> @@ -409,17 +490,7 @@ rm -rf $RPM_BUILD_ROOT
>  %{_libexecdir}/%{servicename}/sssd_ssh
>  %{_libexecdir}/%{servicename}/sssd_sudo
>  
> -# RHEL 5 is too old to support the PAC responder
> -%if !0%{?is_rhel5}
> -%{_libexecdir}/%{servicename}/sssd_pac
> -
> -%endif
> -
>  %dir %{_libdir}/%{name}
> -%{_libdir}/%{name}/libsss_ad.so
> -%{_libdir}/%{name}/libsss_ipa.so
> -%{_libdir}/%{name}/libsss_krb5.so
> -%{_libdir}/%{name}/libsss_ldap.so
>  %{_libdir}/%{name}/libsss_proxy.so
>  %{_libdir}/%{name}/libsss_simple.so
>  
> @@ -427,7 +498,6 @@ rm -rf $RPM_BUILD_ROOT
>  %{_libdir}/%{name}/libsss_child.so
>  %{_libdir}/%{name}/libsss_crypt.so
>  %{_libdir}/%{name}/libsss_debug.so
> -%{_libdir}/%{name}/libsss_krb5_common.so
>  %{_libdir}/%{name}/libsss_ldap_common.so
>  %{_libdir}/%{name}/libsss_util.so
>  
> @@ -448,7 +518,6 @@ rm -rf $RPM_BUILD_ROOT
>  %ghost %attr(0644,root,root) %verify(not md5 size mtime) %{mcpath}/group
>  %attr(755,root,root) %dir %{pipepath}
>  %attr(755,root,root) %dir %{pubconfpath}
> -%attr(755,root,root) %dir %{pubconfpath}/krb5.include.d
>  %attr(700,root,root) %dir %{pipepath}/private
>  %attr(750,root,root) %dir %{_var}/log/%{name}
>  %attr(711,root,root) %dir %{_sysconfdir}/sssd
> @@ -459,10 +528,6 @@ rm -rf $RPM_BUILD_ROOT
>  %{_datadir}/sssd/sssd.api.conf
>  %{_datadir}/sssd/sssd.api.d
>  %{_mandir}/man5/sssd.conf.5*
> -%{_mandir}/man5/sssd-ipa.5*
> -%{_mandir}/man5/sssd-ad.5*
> -%{_mandir}/man5/sssd-krb5.5*
> -%{_mandir}/man5/sssd-ldap.5*
>  %{_mandir}/man5/sssd-simple.5*
>  %{_mandir}/man5/sssd-sudo.5*
>  %{_mandir}/man8/sssd.8*
> @@ -472,6 +537,43 @@ rm -rf $RPM_BUILD_ROOT
>  %{python_sitearch}/pysss.so
>  %{python_sitearch}/pysss_murmur.so
>  
> +%files ldap
> +%defattr(-,root,root,-)
> +%doc COPYING
> +%{_libdir}/%{name}/libsss_ldap.so
> +%{_mandir}/man5/sssd-ldap.5*
> +
> +%files krb5-common
> +%defattr(-,root,root,-)
> +%doc COPYING
> +%{_libdir}/%{name}/libsss_krb5_common.so
> +%{_libexecdir}/%{servicename}/ldap_child
> +%{_libexecdir}/%{servicename}/krb5_child
> +
> +%files krb5
> +%defattr(-,root,root,-)
> +%doc COPYING
> +%{_libdir}/%{name}/libsss_krb5.so
> +%{_mandir}/man5/sssd-krb5.5*
> +
> +%files ipa
> +%defattr(-,root,root,-)
> +%doc COPYING
> +# RHEL 5 is too old to support the PAC responder
> +%if !0%{?is_rhel5}
> +%{_libexecdir}/%{servicename}/sssd_pac
> +%endif
> +
> +%attr(755,root,root) %dir %{pubconfpath}/krb5.include.d
> +%{_libdir}/%{name}/libsss_ipa.so
> +%{_mandir}/man5/sssd-ipa.5*
> +
> +%files ad
> +%defattr(-,root,root,-)
> +%doc COPYING
> +%{_libdir}/%{name}/libsss_ad.so
> +%{_mandir}/man5/sssd-ad.5*
> +
>  %files client -f sssd_client.lang
>  %defattr(-,root,root,-)
>  %doc src/sss_client/COPYING src/sss_client/COPYING.LESSER
> -- 
> 1.8.2.1
>  
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas Slebodnik wrote:
>  1. I like idea of divided subpackages. If someone wants only ldap
> backend, he
>  needn't install samba-libs (and its dependencies)
>  
>  2. There isn't any rpmlint warnings.
>  
>  I tested yum upgrade upgrade with installed sssd and freeipa-client.
>  New packages were installed for dependencies:
>     sssd-ad sssd-common sssd-ipa sssd-krb5 sssd-krb5-common sssd-ldap
>  Everything worked as expected.
>  
>  Then I decided to remove sssd-ad:
>     yum remove sssd-ad
>  and packeges "freeipa-client, sssd" were also removed.
>  I was little bit confused, because I didn't want to remove sssd
>  and sssd replied to getent command after packages "freeipa-client, sssd" were
>  removed. 
That's because freeipa-client currently requires sssd, we might want open a
ticket to make them require just sssd-ipa.

>  
>  I think, that other users may be also confused with this situation.
>  Then I looked to the patch and I found out, that:
>    --sssd is only "meta package",which require all backedns subpackages
>    --sssd doesn't contain any useful files
>    --everything important is in package sssd-common.
>  
>  
>  Maybe we should update package description of sssd and sssd-common.
>  I hope that system administrators relies on output of "yum info"
>  and there isn't it very well explained.
>   
Thanks, I updated the summary of both sssd and sssd-common. Hopefully it
would be clearer now.

>  Summary:
>  Everything works well, but I was little bit confused.
>  
>  Any other opinions?
>  
>  One nitpick inline
>  
>  
>  >+%package krb5-common
>  >+Summary: SSSD helpers needed for Kerberos and GSSAPI authentication
>  >+Group: Applications/System
>  >+License: GPLv3+
>  >+Conflicts: sssd < %{version}-%{release}
>  >+Requires: cyrus-sasl-gssapi
>  >+Requires: sssd-common = %{version}-%{release}
>  >+Requires: sssd-krb5-common = %{version}-%{release}
>              ^^^^^^^^^^^^^^^^
>  Package sssd-krb5-common requires itself? 
Sorry, this was meant to be placed only in the part with the sssd-krb5
subpackage, I think I copied the Requires to both. New patch is
attached.

[Lukas Slebodnik]

On (10/06/13 10:03), Jakub Hrozek wrote:
> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas Slebodnik wrote:
> > 1. I like idea of divided subpackages. If someone wants only ldap backend, he
> > needn't install samba-libs (and its dependencies)
> > 
> > 2. There isn't any rpmlint warnings.
> > 
> > I tested yum upgrade upgrade with installed sssd and freeipa-client.
> > New packages were installed for dependencies:
> >    sssd-ad sssd-common sssd-ipa sssd-krb5 sssd-krb5-common sssd-ldap
> > Everything worked as expected.
> > 
> > Then I decided to remove sssd-ad:
> >    yum remove sssd-ad
> > and packeges "freeipa-client, sssd" were also removed.
> > I was little bit confused, because I didn't want to remove sssd
> > and sssd replied to getent command after packages "freeipa-client, sssd"
> were
> > removed.
>
> That's because freeipa-client currently requires sssd, we might want open a
> ticket to make them require just sssd-ipa.
>  
The most confusing thing was for me that sssd was removed.
Yes, we should file a ticket to freeipa-client after releasing sssd.

> > 
> > I think, that other users may be also confused with this situation.
> > Then I looked to the patch and I found out, that:
> >   --sssd is only "meta package",which require all backedns subpackages
> >   --sssd doesn't contain any useful files
> >   --everything important is in package sssd-common.
> > 
> > 
> > Maybe we should update package description of sssd and sssd-common.
> > I hope that system administrators relies on output of "yum info"
> > and there isn't it very well explained.
> > 
>
> Thanks, I updated the summary of both sssd and sssd-common. Hopefully it
> would be clearer now.
>  
Thank you.

> > Summary:
> > Everything works well, but I was little bit confused.
> >
> > Any other opinions?
> >
> > One nitpick inline
> >
> > 
I think, that patches are OK.
Does anybody have another comments or objections?

LS

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
>  On (10/06/13 10:03), Jakub Hrozek wrote:
> > On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas Slebodnik wrote:
> >> 1. I like idea of divided subpackages. If someone wants only
> >> ldap backend, he needn't install samba-libs (and its
> >> dependencies)
> >> 
> >> 2. There isn't any rpmlint warnings.
> >> 
> >> I tested yum upgrade upgrade with installed sssd and
> >> freeipa-client. New packages were installed for dependencies: 
> >> sssd-ad sssd-common sssd-ipa sssd-krb5 sssd-krb5-common
> >> sssd-ldap Everything worked as expected.
> >> 
> >> Then I decided to remove sssd-ad: yum remove sssd-ad and
> >> packeges "freeipa-client, sssd" were also removed. I was little
> >> bit confused, because I didn't want to remove sssd and sssd
> >> replied to getent command after packages "freeipa-client, sssd"
> >> were removed.
> > 
> > That's because freeipa-client currently requires sssd, we might
> > want open a ticket to make them require just sssd-ipa.
> > 
>  
>  The most confusing thing was for me that sssd was removed. Yes, we
>  should file a ticket to freeipa-client after releasing sssd.
>  
> >> 
> >> I think, that other users may be also confused with this
> >> situation. Then I looked to the patch and I found out, that: 
> >> --sssd is only "meta package",which require all backedns
> >> subpackages --sssd doesn't contain any useful files 
> >> --everything important is in package sssd-common.
> >> 
> >> 
> >> Maybe we should update package description of sssd and
> >> sssd-common. I hope that system administrators relies on output
> >> of "yum info" and there isn't it very well explained.
> >> 
> > 
> > Thanks, I updated the summary of both sssd and sssd-common.
> > Hopefully it would be clearer now.
> > 
>  
>  Thank you.
>  
> >> Summary: Everything works well, but I was little bit confused.
> >> 
> >> Any other opinions?
> >> 
> >> One nitpick inline
> >> 
> >> 
>  
>  I think, that patches are OK. Does anybody have another comments or
>  objections?
>   
Nack.

When adding "Provides:" and "Obsoletes:", it is inappropriate to do
Obsoletes: libsss_sudo < %{version}-%{release}

The Fedora guidelines require that the Obsoletes: be a specific
version (the last one before the change occurred). So

Obsoletes: libsss_sudo <= 1.9.5

The reason for this is so we don't force yum to handle Obsoletes
processing at every update (which I'm told is expensive).

We only need to carry this explicit Obsoletes: until Fedora no longer
has install media with the older version (so until F19 is the oldest
supported version).


Why do you have Conflicts: sssd < %{version}-%{release}? Given that
the packages are identically-named (and that name isn't 'kernel'),
this should be unnecessary.


Is there a particular reason that the proxy provider doesn't have a
subpackage as well?

If we're breaking up packages to make it easier to create a minimal
install, might it be best to just drop all of the manpages into an
sssd-docs subpackage (that gets pulled in by the sssd metapackage)?
Including translations, those can add up. Which brings me to my next
point:

The provider subpackages have only the English manpage included in
them. The translated ones are all landing in the sssd-common package.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlG1zQUACgkQeiVVYja6o6O8RQCfactr8jeLDXzn05Qaxt139U1s
> X+8AoK61xE6bvqWroEoAyVokKoTQ2arC
> =ky+T
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
>  > On (10/06/13 10:03), Jakub Hrozek wrote:
>  >> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas Slebodnik wrote:
>  >>> 1. I like idea of divided subpackages. If someone wants only
>  >>> ldap backend, he needn't install samba-libs (and its
>  >>> dependencies)
>  >>> 
>  >>> 2. There isn't any rpmlint warnings.
>  >>> 
>  >>> I tested yum upgrade upgrade with installed sssd and
>  >>> freeipa-client. New packages were installed for dependencies: 
>  >>> sssd-ad sssd-common sssd-ipa sssd-krb5 sssd-krb5-common
>  >>> sssd-ldap Everything worked as expected.
>  >>> 
>  >>> Then I decided to remove sssd-ad: yum remove sssd-ad and
>  >>> packeges "freeipa-client, sssd" were also removed. I was little
>  >>> bit confused, because I didn't want to remove sssd and sssd
>  >>> replied to getent command after packages "freeipa-client, sssd"
>  >>> were removed.
>  >> 
>  >> That's because freeipa-client currently requires sssd, we might
>  >> want open a ticket to make them require just sssd-ipa.
>  >> 
>  > 
>  > The most confusing thing was for me that sssd was removed. Yes, we
>  > should file a ticket to freeipa-client after releasing sssd.
>  > 
>  >>> 
>  >>> I think, that other users may be also confused with this
>  >>> situation. Then I looked to the patch and I found out, that: 
>  >>> --sssd is only "meta package",which require all backedns
>  >>> subpackages --sssd doesn't contain any useful files 
>  >>> --everything important is in package sssd-common.
>  >>> 
>  >>> 
>  >>> Maybe we should update package description of sssd and
>  >>> sssd-common. I hope that system administrators relies on output
>  >>> of "yum info" and there isn't it very well explained.
>  >>> 
>  >> 
>  >> Thanks, I updated the summary of both sssd and sssd-common.
>  >> Hopefully it would be clearer now.
>  >> 
>  > 
>  > Thank you.
>  > 
>  >>> Summary: Everything works well, but I was little bit confused.
>  >>> 
>  >>> Any other opinions?
>  >>> 
>  >>> One nitpick inline
>  >>> 
>  >>> 
>  > 
>  > I think, that patches are OK. Does anybody have another comments or
>  > objections?
>  > 
>  
>  Nack.
>  
>  When adding "Provides:" and "Obsoletes:", it is inappropriate to do
>  Obsoletes: libsss_sudo < %{version}-%{release}
>  
>  The Fedora guidelines require that the Obsoletes: be a specific
>  version (the last one before the change occurred). So
>  
>  Obsoletes: libsss_sudo <= 1.9.5
>   
OK, in my defense I've seen these in some of the specfiles I've got cloned
from Fedora's dist-git (not that it would make them correct).

I made the changes in the version that is attached, just with a
differens version to compare against. I think your suggestion would make
sense for a Fedora package, but in upstream, we want to obsolete all the
nightly builds prior to the switch as well.

So I added another point version number (minor versions come cheap) and
compared against 1.9.93.

>  The reason for this is so we don't force yum to handle Obsoletes
>  processing at every update (which I'm told is expensive).
>   
I see, I didn't know that.

>  We only need to carry this explicit Obsoletes: until Fedora no
> longer
>  has install media with the older version (so until F19 is the oldest
>  supported version).
>  
>  
>  Why do you have Conflicts: sssd < %{version}-%{release}? Given that
>  the packages are identically-named (and that name isn't 'kernel'),
>  this should be unnecessary.
>   
I wanted to be really defensive and make sure that everything the old version
contained is gone with the new version. That Conflicts: is gone with the
attached patches.

>  
>  Is there a particular reason that the proxy provider doesn't have a
>  subpackage as well?
>   
Just that the proxy provider has no dependencies. But from a logical
standpoint it makes sense, so there is a proxy subpackage now, too.

>  If we're breaking up packages to make it easier to create a
> minimal
>  install, might it be best to just drop all of the manpages into an
>  sssd-docs subpackage (that gets pulled in by the sssd metapackage)?
>  Including translations, those can add up. Which brings me to my next
>  point:
>  
>  The provider subpackages have only the English manpage included in
>  them. The translated ones are all landing in the sssd-common package. 
I don't really like this option. As a user of SSSD I would expect the sssd-ad
man page to be present if I installed the sssd-ad package. Also, I think
it is better to have a single man page installed (even if it's along with
the translations) for the provider I care about than the whole bunch. The
space savings provided by this split would also come by the means of not
installing all the dependencies rather than not installing the files that
form the providers.

Thank you for the review, new patches are attached.

[Jakub Hrozek]

On Mon, Jun 10, 2013 at 08:24:10PM +0200, Jakub Hrozek wrote:
>  > When adding "Provides:" and "Obsoletes:", it
> is inappropriate to do
>  > Obsoletes: libsss_sudo < %{version}-%{release}
>  > 
>  > The Fedora guidelines require that the Obsoletes: be a specific
>  > version (the last one before the change occurred). So
>  > 
>  > Obsoletes: libsss_sudo <= 1.9.5
>  > 
>  
>  OK, in my defense I've seen these in some of the specfiles I've got cloned
>  from Fedora's dist-git (not that it would make them correct).
>  
>  I made the changes in the version that is attached, just with a
>  differens version to compare against. I think your suggestion would make
>  sense for a Fedora package, but in upstream, we want to obsolete all the
>  nightly builds prior to the switch as well.
>  
>  So I added another point version number (minor versions come cheap) and
>  compared against 1.9.93. 
I forgot to add that the new minor number would be removed after
releasing the next upstream version.

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/10/2013 02:24 PM, Jakub Hrozek wrote:
>  On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen Gallagher wrote:
> > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> > 
> > On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
> >> On (10/06/13 10:03), Jakub Hrozek wrote:
> >>> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas Slebodnik
> >>> wrote:
> >>>> 1. I like idea of divided subpackages. If someone wants
> >>>> only ldap backend, he needn't install samba-libs (and its 
> >>>> dependencies)
> >>>> 
> >>>> 2. There isn't any rpmlint warnings.
> >>>> 
> >>>> I tested yum upgrade upgrade with installed sssd and 
> >>>> freeipa-client. New packages were installed for
> >>>> dependencies: sssd-ad sssd-common sssd-ipa sssd-krb5
> >>>> sssd-krb5-common sssd-ldap Everything worked as expected.
> >>>> 
> >>>> Then I decided to remove sssd-ad: yum remove sssd-ad and 
> >>>> packeges "freeipa-client, sssd" were also removed. I was
> >>>> little bit confused, because I didn't want to remove sssd
> >>>> and sssd replied to getent command after packages
> >>>> "freeipa-client, sssd" were removed.
> >>> 
> >>> That's because freeipa-client currently requires sssd, we
> >>> might want open a ticket to make them require just sssd-ipa.
> >>> 
> >> 
> >> The most confusing thing was for me that sssd was removed. Yes,
> >> we should file a ticket to freeipa-client after releasing
> >> sssd.
> >> 
> >>>> 
> >>>> I think, that other users may be also confused with this 
> >>>> situation. Then I looked to the patch and I found out,
> >>>> that: --sssd is only "meta package",which require all
> >>>> backedns subpackages --sssd doesn't contain any useful
> >>>> files --everything important is in package sssd-common.
> >>>> 
> >>>> 
> >>>> Maybe we should update package description of sssd and 
> >>>> sssd-common. I hope that system administrators relies on
> >>>> output of "yum info" and there isn't it very well
> >>>> explained.
> >>>> 
> >>> 
> >>> Thanks, I updated the summary of both sssd and sssd-common. 
> >>> Hopefully it would be clearer now.
> >>> 
> >> 
> >> Thank you.
> >> 
> >>>> Summary: Everything works well, but I was little bit
> >>>> confused.
> >>>> 
> >>>> Any other opinions?
> >>>> 
> >>>> One nitpick inline
> >>>> 
> >>>> 
> >> 
> >> I think, that patches are OK. Does anybody have another
> >> comments or objections?
> >> 
> > 
> > Nack.
> > 
> > When adding "Provides:" and "Obsoletes:", it is inappropriate to
> > do Obsoletes: libsss_sudo < %{version}-%{release}
> > 
> > The Fedora guidelines require that the Obsoletes: be a specific 
> > version (the last one before the change occurred). So
> > 
> > Obsoletes: libsss_sudo <= 1.9.5
> > 
>  
>  OK, in my defense I've seen these in some of the specfiles I've got
>  cloned from Fedora's dist-git (not that it would make them
>  correct).
>  
>  I made the changes in the version that is attached, just with a 
>  differens version to compare against. I think your suggestion would
>  make sense for a Fedora package, but in upstream, we want to
>  obsolete all the nightly builds prior to the switch as well.
>  
>  So I added another point version number (minor versions come cheap)
>  and compared against 1.9.93.
>   
It needs to be <= 1.9.93. Just less-than won't upgrade packages *at*
1.9.93

Wouldn't things be easier to remember if you just bumped version.m4 to
1.9.94 instead? Then you don't need to remember to revert the specfile
patch. (You can just release 1.9.94. No one is really going to notice
if the upstream minor release number jumped two digits instead of one
between pre-releases).


> > The reason for this is so we don't force yum to handle
> Obsoletes 
> > processing at every update (which I'm told is expensive).
> > 
>  
>  I see, I didn't know that.
>   
Technically, it also allows the capability to add that back in as a
package or subpackage at a higher version at some later date without
breakage (because the re-added one would be at a higher N-V-R). That's
not something that matters to us though, just a bit of academia.

> > We only need to carry this explicit Obsoletes: until Fedora no
> > longer has install media with the older version (so until F19 is
> > the oldest supported version).
> > 
> > 
> > Why do you have Conflicts: sssd < %{version}-%{release}? Given
> > that the packages are identically-named (and that name isn't
> > 'kernel'), this should be unnecessary.
> > 
>  
>  I wanted to be really defensive and make sure that everything the
>  old version contained is gone with the new version. That Conflicts:
>  is gone with the attached patches.
>  
> > 
> > Is there a particular reason that the proxy provider doesn't have
> > a subpackage as well?
> > 
>  
>  Just that the proxy provider has no dependencies. But from a
>  logical standpoint it makes sense, so there is a proxy subpackage
>  now, too.
>   
Thanks, but you forgot to add it to the dependencies of the 'sssd'
meta-package.


If we're being pedantic about this, we should probably split out the
'simple' provider as well, but I'm not going to nack over it. It's <
40k.


> > If we're breaking up packages to make it easier to create a
> > minimal install, might it be best to just drop all of the
> > manpages into an sssd-docs subpackage (that gets pulled in by the
> > sssd metapackage)? Including translations, those can add up.
> > Which brings me to my next point:
> > 
> > The provider subpackages have only the English manpage included
> > in them. The translated ones are all landing in the sssd-common
> > package.
>  
>  I don't really like this option. As a user of SSSD I would expect
>  the sssd-ad man page to be present if I installed the sssd-ad
>  package. Also, I think it is better to have a single man page
>  installed (even if it's along with the translations) for the
>  provider I care about than the whole bunch. The space savings
>  provided by this split would also come by the means of not 
>  installing all the dependencies rather than not installing the
>  files that form the providers.
>  
>  Thank you for the review, new patches are attached.
>   

Hmm, the translated manpages seem to have disappeared from the builds
entirely:


[sgallagh@sgallagh520:x86_64 (master *)]$ rpm -qpl
/home/sgallagh/workspace/sssd/x86_64/rpmbuild/RPMS/x86_64/sssd-*.rpm
|grep man
/usr/share/man/man5/sssd-ad.5.gz
/usr/share/man/man8/pam_sss.8.gz
/usr/share/man/man8/sssd_krb5_locator_plugin.8.gz
/usr/share/man/man1/sss_ssh_authorizedkeys.1.gz
/usr/share/man/man1/sss_ssh_knownhostsproxy.1.gz
/usr/share/man/man5/sssd-simple.5.gz
/usr/share/man/man5/sssd-sudo.5.gz
/usr/share/man/man5/sssd.conf.5.gz
/usr/share/man/man8/sss_cache.8.gz
/usr/share/man/man8/sssd.8.gz
/usr/share/man/man5/sssd-ipa.5.gz
/usr/share/man/man5/sssd-krb5.5.gz
/usr/share/man/man5/sssd-ldap.5.gz
/usr/share/man/man8/sss_debuglevel.8.gz
/usr/share/man/man8/sss_groupadd.8.gz
/usr/share/man/man8/sss_groupdel.8.gz
/usr/share/man/man8/sss_groupmod.8.gz
/usr/share/man/man8/sss_groupshow.8.gz
/usr/share/man/man8/sss_obfuscate.8.gz
/usr/share/man/man8/sss_seed.8.gz
/usr/share/man/man8/sss_useradd.8.gz
/usr/share/man/man8/sss_userdel.8.gz
/usr/share/man/man8/sss_usermod.8.gz


I'm running it as a Koji scratch-build right now to make sure it's not
some peculiarity of my system, but I think there's something wrong
with the *.lang files :-/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlG2Lc4ACgkQeiVVYja6o6M4oACeL5MfE5GrFIiBHY8VgLqOHtFA
> pU4AniDjHpq3aa+5vAqhHvFPSQ7Fr/5o
> =Cvwq
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Mon, Jun 10, 2013 at 03:49:34PM -0400, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 06/10/2013 02:24 PM, Jakub Hrozek wrote:
>  > On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen Gallagher wrote:
>  >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>  >> 
>  >> On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
>  >>> On (10/06/13 10:03), Jakub Hrozek wrote:
>  >>>> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas Slebodnik
>  >>>> wrote:
>  >>>>> 1. I like idea of divided subpackages. If someone wants
>  >>>>> only ldap backend, he needn't install samba-libs (and its 
>  >>>>> dependencies)
>  >>>>> 
>  >>>>> 2. There isn't any rpmlint warnings.
>  >>>>> 
>  >>>>> I tested yum upgrade upgrade with installed sssd and 
>  >>>>> freeipa-client. New packages were installed for
>  >>>>> dependencies: sssd-ad sssd-common sssd-ipa sssd-krb5
>  >>>>> sssd-krb5-common sssd-ldap Everything worked as expected.
>  >>>>> 
>  >>>>> Then I decided to remove sssd-ad: yum remove sssd-ad and 
>  >>>>> packeges "freeipa-client, sssd" were also removed. I was
>  >>>>> little bit confused, because I didn't want to remove sssd
>  >>>>> and sssd replied to getent command after packages
>  >>>>> "freeipa-client, sssd" were removed.
>  >>>> 
>  >>>> That's because freeipa-client currently requires sssd, we
>  >>>> might want open a ticket to make them require just sssd-ipa.
>  >>>> 
>  >>> 
>  >>> The most confusing thing was for me that sssd was removed. Yes,
>  >>> we should file a ticket to freeipa-client after releasing
>  >>> sssd.
>  >>> 
>  >>>>> 
>  >>>>> I think, that other users may be also confused with this 
>  >>>>> situation. Then I looked to the patch and I found out,
>  >>>>> that: --sssd is only "meta package",which require all
>  >>>>> backedns subpackages --sssd doesn't contain any useful
>  >>>>> files --everything important is in package sssd-common.
>  >>>>> 
>  >>>>> 
>  >>>>> Maybe we should update package description of sssd and 
>  >>>>> sssd-common. I hope that system administrators relies on
>  >>>>> output of "yum info" and there isn't it very well
>  >>>>> explained.
>  >>>>> 
>  >>>> 
>  >>>> Thanks, I updated the summary of both sssd and sssd-common. 
>  >>>> Hopefully it would be clearer now.
>  >>>> 
>  >>> 
>  >>> Thank you.
>  >>> 
>  >>>>> Summary: Everything works well, but I was little bit
>  >>>>> confused.
>  >>>>> 
>  >>>>> Any other opinions?
>  >>>>> 
>  >>>>> One nitpick inline
>  >>>>> 
>  >>>>> 
>  >>> 
>  >>> I think, that patches are OK. Does anybody have another
>  >>> comments or objections?
>  >>> 
>  >> 
>  >> Nack.
>  >> 
>  >> When adding "Provides:" and "Obsoletes:", it is
> inappropriate to
>  >> do Obsoletes: libsss_sudo < %{version}-%{release}
>  >> 
>  >> The Fedora guidelines require that the Obsoletes: be a specific 
>  >> version (the last one before the change occurred). So
>  >> 
>  >> Obsoletes: libsss_sudo <= 1.9.5
>  >> 
>  > 
>  > OK, in my defense I've seen these in some of the specfiles I've got
>  > cloned from Fedora's dist-git (not that it would make them
>  > correct).
>  > 
>  > I made the changes in the version that is attached, just with a 
>  > differens version to compare against. I think your suggestion would
>  > make sense for a Fedora package, but in upstream, we want to
>  > obsolete all the nightly builds prior to the switch as well.
>  > 
>  > So I added another point version number (minor versions come cheap)
>  > and compared against 1.9.93.
>  > 
>  
>  It needs to be <= 1.9.93. Just less-than won't upgrade packages *at*
>  1.9.93
>  
>  Wouldn't things be easier to remember if you just bumped version.m4 to
>  1.9.94 instead? Then you don't need to remember to revert the specfile
>  patch. (You can just release 1.9.94. No one is really going to notice
>  if the upstream minor release number jumped two digits instead of one
>  between pre-releases).
>   
Yes, if it's OK to bump the version then it's definitely easier. Done.

>  
>  >> The reason for this is so we don't force yum to handle Obsoletes 
>  >> processing at every update (which I'm told is expensive).
>  >> 
>  > 
>  > I see, I didn't know that.
>  > 
>  
>  Technically, it also allows the capability to add that back in as a
>  package or subpackage at a higher version at some later date without
>  breakage (because the re-added one would be at a higher N-V-R). That's
>  not something that matters to us though, just a bit of academia.
>  
>  >> We only need to carry this explicit Obsoletes: until Fedora no
>  >> longer has install media with the older version (so until F19 is
>  >> the oldest supported version).
>  >> 
>  >> 
>  >> Why do you have Conflicts: sssd < %{version}-%{release}? Given
>  >> that the packages are identically-named (and that name isn't
>  >> 'kernel'), this should be unnecessary.
>  >> 
>  > 
>  > I wanted to be really defensive and make sure that everything the
>  > old version contained is gone with the new version. That Conflicts:
>  > is gone with the attached patches.
>  > 
>  >> 
>  >> Is there a particular reason that the proxy provider doesn't have
>  >> a subpackage as well?
>  >> 
>  > 
>  > Just that the proxy provider has no dependencies. But from a
>  > logical standpoint it makes sense, so there is a proxy subpackage
>  > now, too.
>  > 
>  
>  Thanks, but you forgot to add it to the dependencies of the 'sssd'
>  meta-package.
>   
Oops, fixed.

>  
>  If we're being pedantic about this, we should probably split out the
>  'simple' provider as well, but I'm not going to nack over it. It's <
> 40k.
>   
I think it's better if the simple provider stays in the common package.
It can be used along with any other provider, so the other providers
would have to require it anyway. Also, it has no dependencies.

>  
>  >> If we're breaking up packages to make it easier to create a
>  >> minimal install, might it be best to just drop all of the
>  >> manpages into an sssd-docs subpackage (that gets pulled in by the
>  >> sssd metapackage)? Including translations, those can add up.
>  >> Which brings me to my next point:
>  >> 
>  >> The provider subpackages have only the English manpage included
>  >> in them. The translated ones are all landing in the sssd-common
>  >> package.
>  > 
>  > I don't really like this option. As a user of SSSD I would expect
>  > the sssd-ad man page to be present if I installed the sssd-ad
>  > package. Also, I think it is better to have a single man page
>  > installed (even if it's along with the translations) for the
>  > provider I care about than the whole bunch. The space savings
>  > provided by this split would also come by the means of not 
>  > installing all the dependencies rather than not installing the
>  > files that form the providers.
>  > 
>  > Thank you for the review, new patches are attached.
>  > 
>  
>  
>  Hmm, the translated manpages seem to have disappeared from the builds
>  entirely:
>   
I see the same behaviour with parallel build (also with master w/o these
patches), but with an in-tree build, the man pages are generated fine.

Here is a scratch build I built:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5490069

For instance, check out the sssd-ldap subpackage:
$ rpm -qpl sssd-ldap-1.9.94-0.fc19.x86_64.rpm | grep man
/usr/share/man/ja/man5/sssd-ldap.5.gz
/usr/share/man/man5/sssd-ldap.5.gz
/usr/share/man/uk/man5/sssd-ldap.5.gz

>  
>  [sgallagh@sgallagh520:x86_64 (master *)]$ rpm -qpl
>  /home/sgallagh/workspace/sssd/x86_64/rpmbuild/RPMS/x86_64/sssd-*.rpm
>  |grep man
>  /usr/share/man/man5/sssd-ad.5.gz
>  /usr/share/man/man8/pam_sss.8.gz
>  /usr/share/man/man8/sssd_krb5_locator_plugin.8.gz
>  /usr/share/man/man1/sss_ssh_authorizedkeys.1.gz
>  /usr/share/man/man1/sss_ssh_knownhostsproxy.1.gz
>  /usr/share/man/man5/sssd-simple.5.gz
>  /usr/share/man/man5/sssd-sudo.5.gz
>  /usr/share/man/man5/sssd.conf.5.gz
>  /usr/share/man/man8/sss_cache.8.gz
>  /usr/share/man/man8/sssd.8.gz
>  /usr/share/man/man5/sssd-ipa.5.gz
>  /usr/share/man/man5/sssd-krb5.5.gz
>  /usr/share/man/man5/sssd-ldap.5.gz
>  /usr/share/man/man8/sss_debuglevel.8.gz
>  /usr/share/man/man8/sss_groupadd.8.gz
>  /usr/share/man/man8/sss_groupdel.8.gz
>  /usr/share/man/man8/sss_groupmod.8.gz
>  /usr/share/man/man8/sss_groupshow.8.gz
>  /usr/share/man/man8/sss_obfuscate.8.gz
>  /usr/share/man/man8/sss_seed.8.gz
>  /usr/share/man/man8/sss_useradd.8.gz
>  /usr/share/man/man8/sss_userdel.8.gz
>  /usr/share/man/man8/sss_usermod.8.gz
>  
>  
>  I'm running it as a Koji scratch-build right now to make sure it's not
>  some peculiarity of my system, but I think there's something wrong
>  with the *.lang files :-/
>  -----BEGIN PGP SIGNATURE-----
>  Version: GnuPG v1.4.13 (GNU/Linux)
>  Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>  
>  iEYEARECAAYFAlG2Lc4ACgkQeiVVYja6o6M4oACeL5MfE5GrFIiBHY8VgLqOHtFA
>  pU4AniDjHpq3aa+5vAqhHvFPSQ7Fr/5o
>  =Cvwq
>  -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/11/2013 06:03 AM, Jakub Hrozek wrote:
>  On Mon, Jun 10, 2013 at 03:49:34PM -0400, Stephen Gallagher wrote: 
>  On 06/10/2013 02:24 PM, Jakub Hrozek wrote:
> >>> On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen Gallagher
> >>> wrote:
> >>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>>> 
> >>>> On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
> >>>>> On (10/06/13 10:03), Jakub Hrozek wrote:
> >>>>>> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas
> >>>>>> Slebodnik wrote:
> >>>>>>> 1. I like idea of divided subpackages. If someone
> >>>>>>> wants only ldap backend, he needn't install
> >>>>>>> samba-libs (and its dependencies)
> >>>>>>> 
> >>>>>>> 2. There isn't any rpmlint warnings.
> >>>>>>> 
> >>>>>>> I tested yum upgrade upgrade with installed sssd and
> >>>>>>>  freeipa-client. New packages were installed for 
> >>>>>>> dependencies: sssd-ad sssd-common sssd-ipa sssd-krb5 
> >>>>>>> sssd-krb5-common sssd-ldap Everything worked as
> >>>>>>> expected.
> >>>>>>> 
> >>>>>>> Then I decided to remove sssd-ad: yum remove sssd-ad
> >>>>>>> and packeges "freeipa-client, sssd" were also
> >>>>>>> removed. I was little bit confused, because I didn't
> >>>>>>> want to remove sssd and sssd replied to getent
> >>>>>>> command after packages "freeipa-client, sssd" were
> >>>>>>> removed.
> >>>>>> 
> >>>>>> That's because freeipa-client currently requires sssd,
> >>>>>> we might want open a ticket to make them require just
> >>>>>> sssd-ipa.
> >>>>>> 
> >>>>> 
> >>>>> The most confusing thing was for me that sssd was
> >>>>> removed. Yes, we should file a ticket to freeipa-client
> >>>>> after releasing sssd.
> >>>>> 
> >>>>>>> 
> >>>>>>> I think, that other users may be also confused with
> >>>>>>> this situation. Then I looked to the patch and I
> >>>>>>> found out, that: --sssd is only "meta
> package",which
> >>>>>>> require all backedns subpackages --sssd doesn't
> >>>>>>> contain any useful files --everything important is in
> >>>>>>> package sssd-common.
> >>>>>>> 
> >>>>>>> 
> >>>>>>> Maybe we should update package description of sssd
> >>>>>>> and sssd-common. I hope that system administrators
> >>>>>>> relies on output of "yum info" and there isn't
> it
> >>>>>>> very well explained.
> >>>>>>> 
> >>>>>> 
> >>>>>> Thanks, I updated the summary of both sssd and
> >>>>>> sssd-common. Hopefully it would be clearer now.
> >>>>>> 
> >>>>> 
> >>>>> Thank you.
> >>>>> 
> >>>>>>> Summary: Everything works well, but I was little bit 
> >>>>>>> confused.
> >>>>>>> 
> >>>>>>> Any other opinions?
> >>>>>>> 
> >>>>>>> One nitpick inline
> >>>>>>> 
> >>>>>>> 
> >>>>> 
> >>>>> I think, that patches are OK. Does anybody have another 
> >>>>> comments or objections?
> >>>>> 
> >>>> 
> >>>> Nack.
> >>>> 
> >>>> When adding "Provides:" and "Obsoletes:", it is
> >>>> inappropriate to do Obsoletes: libsss_sudo <
> >>>> %{version}-%{release}
> >>>> 
> >>>> The Fedora guidelines require that the Obsoletes: be a
> >>>> specific version (the last one before the change occurred).
> >>>> So
> >>>> 
> >>>> Obsoletes: libsss_sudo <= 1.9.5
> >>>> 
> >>> 
> >>> OK, in my defense I've seen these in some of the specfiles
> >>> I've got cloned from Fedora's dist-git (not that it would
> >>> make them correct).
> >>> 
> >>> I made the changes in the version that is attached, just with
> >>> a differens version to compare against. I think your
> >>> suggestion would make sense for a Fedora package, but in
> >>> upstream, we want to obsolete all the nightly builds prior to
> >>> the switch as well.
> >>> 
> >>> So I added another point version number (minor versions come
> >>> cheap) and compared against 1.9.93.
> >>> 
>  
>  It needs to be <= 1.9.93. Just less-than won't upgrade packages
>  *at* 1.9.93
>  
>  Wouldn't things be easier to remember if you just bumped version.m4
>  to 1.9.94 instead? Then you don't need to remember to revert the
>  specfile patch. (You can just release 1.9.94. No one is really
>  going to notice if the upstream minor release number jumped two
>  digits instead of one between pre-releases).
>  
>  
> > Yes, if it's OK to bump the version then it's definitely easier.
> > Done.
>  
>  
> >>>> The reason for this is so we don't force yum to handle
> >>>> Obsoletes processing at every update (which I'm told is
> >>>> expensive).
> >>>> 
> >>> 
> >>> I see, I didn't know that.
> >>> 
>  
>  Technically, it also allows the capability to add that back in as
>  a package or subpackage at a higher version at some later date
>  without breakage (because the re-added one would be at a higher
>  N-V-R). That's not something that matters to us though, just a bit
>  of academia.
>  
> >>>> We only need to carry this explicit Obsoletes: until Fedora
> >>>> no longer has install media with the older version (so
> >>>> until F19 is the oldest supported version).
> >>>> 
> >>>> 
> >>>> Why do you have Conflicts: sssd < %{version}-%{release}?
> >>>> Given that the packages are identically-named (and that
> >>>> name isn't 'kernel'), this should be unnecessary.
> >>>> 
> >>> 
> >>> I wanted to be really defensive and make sure that everything
> >>> the old version contained is gone with the new version. That
> >>> Conflicts: is gone with the attached patches.
> >>> 
> >>>> 
> >>>> Is there a particular reason that the proxy provider
> >>>> doesn't have a subpackage as well?
> >>>> 
> >>> 
> >>> Just that the proxy provider has no dependencies. But from a 
> >>> logical standpoint it makes sense, so there is a proxy
> >>> subpackage now, too.
> >>> 
>  
>  Thanks, but you forgot to add it to the dependencies of the 'sssd' 
>  meta-package.
>  
>  
> > Oops, fixed.
>  
>  
>  If we're being pedantic about this, we should probably split out
>  the 'simple' provider as well, but I'm not going to nack over it.
>  It's < 40k.
>  
>  
> > I think it's better if the simple provider stays in the common
> > package. It can be used along with any other provider, so the
> > other providers would have to require it anyway. Also, it has no
> > dependencies.
>  
>  
> >>>> If we're breaking up packages to make it easier to create
> >>>> a minimal install, might it be best to just drop all of
> >>>> the manpages into an sssd-docs subpackage (that gets pulled
> >>>> in by the sssd metapackage)? Including translations, those
> >>>> can add up. Which brings me to my next point:
> >>>> 
> >>>> The provider subpackages have only the English manpage
> >>>> included in them. The translated ones are all landing in
> >>>> the sssd-common package.
> >>> 
> >>> I don't really like this option. As a user of SSSD I would
> >>> expect the sssd-ad man page to be present if I installed the
> >>> sssd-ad package. Also, I think it is better to have a single
> >>> man page installed (even if it's along with the translations)
> >>> for the provider I care about than the whole bunch. The space
> >>> savings provided by this split would also come by the means
> >>> of not installing all the dependencies rather than not
> >>> installing the files that form the providers.
> >>> 
> >>> Thank you for the review, new patches are attached.
> >>> 
>  
>  
>  Hmm, the translated manpages seem to have disappeared from the
>  builds entirely:
>  
>  
> > I see the same behaviour with parallel build (also with master
> > w/o these patches), but with an in-tree build, the man pages are
> > generated fine.
>  
> > Here is a scratch build I built: 
> > http://koji.fedoraproject.org/koji/taskinfo?taskID=5490069
>  
> > For instance, check out the sssd-ldap subpackage: $ rpm -qpl
> > sssd-ldap-1.9.94-0.fc19.x86_64.rpm | grep man 
> > /usr/share/man/ja/man5/sssd-ldap.5.gz 
> > /usr/share/man/man5/sssd-ldap.5.gz 
> > /usr/share/man/uk/man5/sssd-ldap.5.gz
>  
>  
>  [sgallagh@sgallagh520:x86_64 (master *)]$ rpm -qpl 
>  /home/sgallagh/workspace/sssd/x86_64/rpmbuild/RPMS/x86_64/sssd-*.rpm
>
>   
|grep man
>  /usr/share/man/man5/sssd-ad.5.gz /usr/share/man/man8/pam_sss.8.gz 
>  /usr/share/man/man8/sssd_krb5_locator_plugin.8.gz 
>  /usr/share/man/man1/sss_ssh_authorizedkeys.1.gz 
>  /usr/share/man/man1/sss_ssh_knownhostsproxy.1.gz 
>  /usr/share/man/man5/sssd-simple.5.gz 
>  /usr/share/man/man5/sssd-sudo.5.gz 
>  /usr/share/man/man5/sssd.conf.5.gz 
>  /usr/share/man/man8/sss_cache.8.gz /usr/share/man/man8/sssd.8.gz 
>  /usr/share/man/man5/sssd-ipa.5.gz 
>  /usr/share/man/man5/sssd-krb5.5.gz 
>  /usr/share/man/man5/sssd-ldap.5.gz 
>  /usr/share/man/man8/sss_debuglevel.8.gz 
>  /usr/share/man/man8/sss_groupadd.8.gz 
>  /usr/share/man/man8/sss_groupdel.8.gz 
>  /usr/share/man/man8/sss_groupmod.8.gz 
>  /usr/share/man/man8/sss_groupshow.8.gz 
>  /usr/share/man/man8/sss_obfuscate.8.gz 
>  /usr/share/man/man8/sss_seed.8.gz 
>  /usr/share/man/man8/sss_useradd.8.gz 
>  /usr/share/man/man8/sss_userdel.8.gz 
>  /usr/share/man/man8/sss_usermod.8.gz
>  
>  
>  I'm running it as a Koji scratch-build right now to make sure it's
>  not some peculiarity of my system, but I think there's something
>  wrong with the *.lang files :-/ 

My scratch-build against F20 also did not include translated manpages:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5488765

Something broke somewhere, I assume. Let's track this down before we
push these patches, please.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlG3KC0ACgkQeiVVYja6o6M7dQCgiIiF2XjWApPpPPBgfht9KWmM
> rfEAnA4e/Iim9vhLA8p/co0tlNBBSAMP
> =V1l3
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Tue, Jun 11, 2013 at 09:37:49AM -0400, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 06/11/2013 06:03 AM, Jakub Hrozek wrote:
>  > On Mon, Jun 10, 2013 at 03:49:34PM -0400, Stephen Gallagher wrote: 
>  > On 06/10/2013 02:24 PM, Jakub Hrozek wrote:
>  >>>> On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen Gallagher
>  >>>> wrote:
>  >>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>  >>>>> 
>  >>>>> On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
>  >>>>>> On (10/06/13 10:03), Jakub Hrozek wrote:
>  >>>>>>> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas
>  >>>>>>> Slebodnik wrote:
>  >>>>>>>> 1. I like idea of divided subpackages. If someone
>  >>>>>>>> wants only ldap backend, he needn't install
>  >>>>>>>> samba-libs (and its dependencies)
>  >>>>>>>> 
>  >>>>>>>> 2. There isn't any rpmlint warnings.
>  >>>>>>>> 
>  >>>>>>>> I tested yum upgrade upgrade with installed sssd and
>  >>>>>>>>  freeipa-client. New packages were installed for 
>  >>>>>>>> dependencies: sssd-ad sssd-common sssd-ipa sssd-krb5 
>  >>>>>>>> sssd-krb5-common sssd-ldap Everything worked as
>  >>>>>>>> expected.
>  >>>>>>>> 
>  >>>>>>>> Then I decided to remove sssd-ad: yum remove sssd-ad
>  >>>>>>>> and packeges "freeipa-client, sssd" were also
>  >>>>>>>> removed. I was little bit confused, because I
> didn't
>  >>>>>>>> want to remove sssd and sssd replied to getent
>  >>>>>>>> command after packages "freeipa-client, sssd"
> were
>  >>>>>>>> removed.
>  >>>>>>> 
>  >>>>>>> That's because freeipa-client currently requires sssd,
>  >>>>>>> we might want open a ticket to make them require just
>  >>>>>>> sssd-ipa.
>  >>>>>>> 
>  >>>>>> 
>  >>>>>> The most confusing thing was for me that sssd was
>  >>>>>> removed. Yes, we should file a ticket to freeipa-client
>  >>>>>> after releasing sssd.
>  >>>>>> 
>  >>>>>>>> 
>  >>>>>>>> I think, that other users may be also confused with
>  >>>>>>>> this situation. Then I looked to the patch and I
>  >>>>>>>> found out, that: --sssd is only "meta
> package",which
>  >>>>>>>> require all backedns subpackages --sssd doesn't
>  >>>>>>>> contain any useful files --everything important is in
>  >>>>>>>> package sssd-common.
>  >>>>>>>> 
>  >>>>>>>> 
>  >>>>>>>> Maybe we should update package description of sssd
>  >>>>>>>> and sssd-common. I hope that system administrators
>  >>>>>>>> relies on output of "yum info" and there
> isn't it
>  >>>>>>>> very well explained.
>  >>>>>>>> 
>  >>>>>>> 
>  >>>>>>> Thanks, I updated the summary of both sssd and
>  >>>>>>> sssd-common. Hopefully it would be clearer now.
>  >>>>>>> 
>  >>>>>> 
>  >>>>>> Thank you.
>  >>>>>> 
>  >>>>>>>> Summary: Everything works well, but I was little bit 
>  >>>>>>>> confused.
>  >>>>>>>> 
>  >>>>>>>> Any other opinions?
>  >>>>>>>> 
>  >>>>>>>> One nitpick inline
>  >>>>>>>> 
>  >>>>>>>> 
>  >>>>>> 
>  >>>>>> I think, that patches are OK. Does anybody have another 
>  >>>>>> comments or objections?
>  >>>>>> 
>  >>>>> 
>  >>>>> Nack.
>  >>>>> 
>  >>>>> When adding "Provides:" and "Obsoletes:", it is
>  >>>>> inappropriate to do Obsoletes: libsss_sudo <
>  >>>>> %{version}-%{release}
>  >>>>> 
>  >>>>> The Fedora guidelines require that the Obsoletes: be a
>  >>>>> specific version (the last one before the change occurred).
>  >>>>> So
>  >>>>> 
>  >>>>> Obsoletes: libsss_sudo <= 1.9.5
>  >>>>> 
>  >>>> 
>  >>>> OK, in my defense I've seen these in some of the specfiles
>  >>>> I've got cloned from Fedora's dist-git (not that it would
>  >>>> make them correct).
>  >>>> 
>  >>>> I made the changes in the version that is attached, just with
>  >>>> a differens version to compare against. I think your
>  >>>> suggestion would make sense for a Fedora package, but in
>  >>>> upstream, we want to obsolete all the nightly builds prior to
>  >>>> the switch as well.
>  >>>> 
>  >>>> So I added another point version number (minor versions come
>  >>>> cheap) and compared against 1.9.93.
>  >>>> 
>  > 
>  > It needs to be <= 1.9.93. Just less-than won't upgrade packages
>  > *at* 1.9.93
>  > 
>  > Wouldn't things be easier to remember if you just bumped version.m4
>  > to 1.9.94 instead? Then you don't need to remember to revert the
>  > specfile patch. (You can just release 1.9.94. No one is really
>  > going to notice if the upstream minor release number jumped two
>  > digits instead of one between pre-releases).
>  > 
>  > 
>  >> Yes, if it's OK to bump the version then it's definitely easier.
>  >> Done.
>  > 
>  > 
>  >>>>> The reason for this is so we don't force yum to handle
>  >>>>> Obsoletes processing at every update (which I'm told is
>  >>>>> expensive).
>  >>>>> 
>  >>>> 
>  >>>> I see, I didn't know that.
>  >>>> 
>  > 
>  > Technically, it also allows the capability to add that back in as
>  > a package or subpackage at a higher version at some later date
>  > without breakage (because the re-added one would be at a higher
>  > N-V-R). That's not something that matters to us though, just a bit
>  > of academia.
>  > 
>  >>>>> We only need to carry this explicit Obsoletes: until Fedora
>  >>>>> no longer has install media with the older version (so
>  >>>>> until F19 is the oldest supported version).
>  >>>>> 
>  >>>>> 
>  >>>>> Why do you have Conflicts: sssd < %{version}-%{release}?
>  >>>>> Given that the packages are identically-named (and that
>  >>>>> name isn't 'kernel'), this should be unnecessary.
>  >>>>> 
>  >>>> 
>  >>>> I wanted to be really defensive and make sure that everything
>  >>>> the old version contained is gone with the new version. That
>  >>>> Conflicts: is gone with the attached patches.
>  >>>> 
>  >>>>> 
>  >>>>> Is there a particular reason that the proxy provider
>  >>>>> doesn't have a subpackage as well?
>  >>>>> 
>  >>>> 
>  >>>> Just that the proxy provider has no dependencies. But from a 
>  >>>> logical standpoint it makes sense, so there is a proxy
>  >>>> subpackage now, too.
>  >>>> 
>  > 
>  > Thanks, but you forgot to add it to the dependencies of the 'sssd' 
>  > meta-package.
>  > 
>  > 
>  >> Oops, fixed.
>  > 
>  > 
>  > If we're being pedantic about this, we should probably split out
>  > the 'simple' provider as well, but I'm not going to nack over it.
>  > It's < 40k.
>  > 
>  > 
>  >> I think it's better if the simple provider stays in the common
>  >> package. It can be used along with any other provider, so the
>  >> other providers would have to require it anyway. Also, it has no
>  >> dependencies.
>  > 
>  > 
>  >>>>> If we're breaking up packages to make it easier to create
>  >>>>> a minimal install, might it be best to just drop all of
>  >>>>> the manpages into an sssd-docs subpackage (that gets pulled
>  >>>>> in by the sssd metapackage)? Including translations, those
>  >>>>> can add up. Which brings me to my next point:
>  >>>>> 
>  >>>>> The provider subpackages have only the English manpage
>  >>>>> included in them. The translated ones are all landing in
>  >>>>> the sssd-common package.
>  >>>> 
>  >>>> I don't really like this option. As a user of SSSD I would
>  >>>> expect the sssd-ad man page to be present if I installed the
>  >>>> sssd-ad package. Also, I think it is better to have a single
>  >>>> man page installed (even if it's along with the translations)
>  >>>> for the provider I care about than the whole bunch. The space
>  >>>> savings provided by this split would also come by the means
>  >>>> of not installing all the dependencies rather than not
>  >>>> installing the files that form the providers.
>  >>>> 
>  >>>> Thank you for the review, new patches are attached.
>  >>>> 
>  > 
>  > 
>  > Hmm, the translated manpages seem to have disappeared from the
>  > builds entirely:
>  > 
>  > 
>  >> I see the same behaviour with parallel build (also with master
>  >> w/o these patches), but with an in-tree build, the man pages are
>  >> generated fine.
>  > 
>  >> Here is a scratch build I built: 
>  >> http://koji.fedoraproject.org/koji/taskinfo?taskID=5490069
>  > 
>  >> For instance, check out the sssd-ldap subpackage: $ rpm -qpl
>  >> sssd-ldap-1.9.94-0.fc19.x86_64.rpm | grep man 
>  >> /usr/share/man/ja/man5/sssd-ldap.5.gz 
>  >> /usr/share/man/man5/sssd-ldap.5.gz 
>  >> /usr/share/man/uk/man5/sssd-ldap.5.gz
>  > 
>  > 
>  > [sgallagh@sgallagh520:x86_64 (master *)]$ rpm -qpl 
>  > /home/sgallagh/workspace/sssd/x86_64/rpmbuild/RPMS/x86_64/sssd-*.rpm
>  >
>  > 
>  |grep man
>  > /usr/share/man/man5/sssd-ad.5.gz /usr/share/man/man8/pam_sss.8.gz 
>  > /usr/share/man/man8/sssd_krb5_locator_plugin.8.gz 
>  > /usr/share/man/man1/sss_ssh_authorizedkeys.1.gz 
>  > /usr/share/man/man1/sss_ssh_knownhostsproxy.1.gz 
>  > /usr/share/man/man5/sssd-simple.5.gz 
>  > /usr/share/man/man5/sssd-sudo.5.gz 
>  > /usr/share/man/man5/sssd.conf.5.gz 
>  > /usr/share/man/man8/sss_cache.8.gz /usr/share/man/man8/sssd.8.gz 
>  > /usr/share/man/man5/sssd-ipa.5.gz 
>  > /usr/share/man/man5/sssd-krb5.5.gz 
>  > /usr/share/man/man5/sssd-ldap.5.gz 
>  > /usr/share/man/man8/sss_debuglevel.8.gz 
>  > /usr/share/man/man8/sss_groupadd.8.gz 
>  > /usr/share/man/man8/sss_groupdel.8.gz 
>  > /usr/share/man/man8/sss_groupmod.8.gz 
>  > /usr/share/man/man8/sss_groupshow.8.gz 
>  > /usr/share/man/man8/sss_obfuscate.8.gz 
>  > /usr/share/man/man8/sss_seed.8.gz 
>  > /usr/share/man/man8/sss_useradd.8.gz 
>  > /usr/share/man/man8/sss_userdel.8.gz 
>  > /usr/share/man/man8/sss_usermod.8.gz
>  > 
>  > 
>  > I'm running it as a Koji scratch-build right now to make sure it's
>  > not some peculiarity of my system, but I think there's something
>  > wrong with the *.lang files :-/
>  
>  
>  My scratch-build against F20 also did not include translated manpages:
>  http://koji.fedoraproject.org/koji/taskinfo?taskID=5488765
>  
>  Something broke somewhere, I assume. Let's track this down before we
>  push these patches, please. 
The version of the Koji build shows that you tested with the previous patches.

How exactly did you create the SRPM?
Does the same sequence work without the patches?

Did you create the SRPM that was rebuilt in koji using a parallel build
or an in-tree build? A parallel build didn't work for me either most
probably because po4a couldn't find the strings:

In-tree build works for me 100%:
$ git clone git://fedorahosted.org/sssd.git
$ ls -l *.patch
-rw-------. 1 jhrozek jhrozek  1161 Jun 11 15:54
0001-rpm-Use-hardened-flags-for-RPM-build.patch
-rw-------. 1 jhrozek jhrozek 12165 Jun 11 15:54
0002-rpm-Split-providers-into-separate-subpackages.patch
$ autoreconf -if && configure && make srpm
$ fedpkg --dist f20 scratch-build --srpm
/home/remote/jhrozek/tmp/sssd/rpmbuild/SRPMS/sssd-1.9.94-0.fc19.src.rpm

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/11/2013 10:07 AM, Jakub Hrozek wrote:
>  On Tue, Jun 11, 2013 at 09:37:49AM -0400, Stephen Gallagher wrote:
> > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> > 
> > On 06/11/2013 06:03 AM, Jakub Hrozek wrote:
> >> On Mon, Jun 10, 2013 at 03:49:34PM -0400, Stephen Gallagher
> >> wrote: On 06/10/2013 02:24 PM, Jakub Hrozek wrote:
> >>>>> On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen
> >>>>> Gallagher wrote:
> >>>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>>>>> 
> >>>>>> On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
> >>>>>>> On (10/06/13 10:03), Jakub Hrozek wrote:
> >>>>>>>> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas 
> >>>>>>>> Slebodnik wrote:
> >>>>>>>>> 1. I like idea of divided subpackages. If
> >>>>>>>>> someone wants only ldap backend, he needn't
> >>>>>>>>> install samba-libs (and its dependencies)
> >>>>>>>>> 
> >>>>>>>>> 2. There isn't any rpmlint warnings.
> >>>>>>>>> 
> >>>>>>>>> I tested yum upgrade upgrade with installed sssd
> >>>>>>>>> and freeipa-client. New packages were installed
> >>>>>>>>> for dependencies: sssd-ad sssd-common sssd-ipa
> >>>>>>>>> sssd-krb5 sssd-krb5-common sssd-ldap Everything
> >>>>>>>>> worked as expected.
> >>>>>>>>> 
> >>>>>>>>> Then I decided to remove sssd-ad: yum remove
> >>>>>>>>> sssd-ad and packeges "freeipa-client, sssd"
> were
> >>>>>>>>> also removed. I was little bit confused, because
> >>>>>>>>> I didn't want to remove sssd and sssd replied to
> >>>>>>>>> getent command after packages "freeipa-client,
> >>>>>>>>> sssd" were removed.
> >>>>>>>> 
> >>>>>>>> That's because freeipa-client currently requires
> >>>>>>>> sssd, we might want open a ticket to make them
> >>>>>>>> require just sssd-ipa.
> >>>>>>>> 
> >>>>>>> 
> >>>>>>> The most confusing thing was for me that sssd was 
> >>>>>>> removed. Yes, we should file a ticket to
> >>>>>>> freeipa-client after releasing sssd.
> >>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> I think, that other users may be also confused
> >>>>>>>>> with this situation. Then I looked to the patch
> >>>>>>>>> and I found out, that: --sssd is only "meta
> >>>>>>>>> package",which require all backedns subpackages
> >>>>>>>>> --sssd doesn't contain any useful files
> >>>>>>>>> --everything important is in package
> >>>>>>>>> sssd-common.
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> Maybe we should update package description of
> >>>>>>>>> sssd and sssd-common. I hope that system
> >>>>>>>>> administrators relies on output of "yum
> info" and
> >>>>>>>>> there isn't it very well explained.
> >>>>>>>>> 
> >>>>>>>> 
> >>>>>>>> Thanks, I updated the summary of both sssd and 
> >>>>>>>> sssd-common. Hopefully it would be clearer now.
> >>>>>>>> 
> >>>>>>> 
> >>>>>>> Thank you.
> >>>>>>> 
> >>>>>>>>> Summary: Everything works well, but I was little
> >>>>>>>>> bit confused.
> >>>>>>>>> 
> >>>>>>>>> Any other opinions?
> >>>>>>>>> 
> >>>>>>>>> One nitpick inline
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>> 
> >>>>>>> I think, that patches are OK. Does anybody have
> >>>>>>> another comments or objections?
> >>>>>>> 
> >>>>>> 
> >>>>>> Nack.
> >>>>>> 
> >>>>>> When adding "Provides:" and "Obsoletes:", it
> is 
> >>>>>> inappropriate to do Obsoletes: libsss_sudo < 
> >>>>>> %{version}-%{release}
> >>>>>> 
> >>>>>> The Fedora guidelines require that the Obsoletes: be a 
> >>>>>> specific version (the last one before the change
> >>>>>> occurred). So
> >>>>>> 
> >>>>>> Obsoletes: libsss_sudo <= 1.9.5
> >>>>>> 
> >>>>> 
> >>>>> OK, in my defense I've seen these in some of the
> >>>>> specfiles I've got cloned from Fedora's dist-git (not
> >>>>> that it would make them correct).
> >>>>> 
> >>>>> I made the changes in the version that is attached, just
> >>>>> with a differens version to compare against. I think
> >>>>> your suggestion would make sense for a Fedora package,
> >>>>> but in upstream, we want to obsolete all the nightly
> >>>>> builds prior to the switch as well.
> >>>>> 
> >>>>> So I added another point version number (minor versions
> >>>>> come cheap) and compared against 1.9.93.
> >>>>> 
> >> 
> >> It needs to be <= 1.9.93. Just less-than won't upgrade
> >> packages *at* 1.9.93
> >> 
> >> Wouldn't things be easier to remember if you just bumped
> >> version.m4 to 1.9.94 instead? Then you don't need to remember
> >> to revert the specfile patch. (You can just release 1.9.94. No
> >> one is really going to notice if the upstream minor release
> >> number jumped two digits instead of one between pre-releases).
> >> 
> >> 
> >>> Yes, if it's OK to bump the version then it's definitely
> >>> easier. Done.
> >> 
> >> 
> >>>>>> The reason for this is so we don't force yum to handle 
> >>>>>> Obsoletes processing at every update (which I'm told
> >>>>>> is expensive).
> >>>>>> 
> >>>>> 
> >>>>> I see, I didn't know that.
> >>>>> 
> >> 
> >> Technically, it also allows the capability to add that back in
> >> as a package or subpackage at a higher version at some later
> >> date without breakage (because the re-added one would be at a
> >> higher N-V-R). That's not something that matters to us though,
> >> just a bit of academia.
> >> 
> >>>>>> We only need to carry this explicit Obsoletes: until
> >>>>>> Fedora no longer has install media with the older
> >>>>>> version (so until F19 is the oldest supported
> >>>>>> version).
> >>>>>> 
> >>>>>> 
> >>>>>> Why do you have Conflicts: sssd <
> >>>>>> %{version}-%{release}? Given that the packages are
> >>>>>> identically-named (and that name isn't 'kernel'),
> this
> >>>>>> should be unnecessary.
> >>>>>> 
> >>>>> 
> >>>>> I wanted to be really defensive and make sure that
> >>>>> everything the old version contained is gone with the new
> >>>>> version. That Conflicts: is gone with the attached
> >>>>> patches.
> >>>>> 
> >>>>>> 
> >>>>>> Is there a particular reason that the proxy provider 
> >>>>>> doesn't have a subpackage as well?
> >>>>>> 
> >>>>> 
> >>>>> Just that the proxy provider has no dependencies. But
> >>>>> from a logical standpoint it makes sense, so there is a
> >>>>> proxy subpackage now, too.
> >>>>> 
> >> 
> >> Thanks, but you forgot to add it to the dependencies of the
> >> 'sssd' meta-package.
> >> 
> >> 
> >>> Oops, fixed.
> >> 
> >> 
> >> If we're being pedantic about this, we should probably split
> >> out the 'simple' provider as well, but I'm not going to nack
> >> over it. It's < 40k.
> >> 
> >> 
> >>> I think it's better if the simple provider stays in the
> >>> common package. It can be used along with any other provider,
> >>> so the other providers would have to require it anyway. Also,
> >>> it has no dependencies.
> >> 
> >> 
> >>>>>> If we're breaking up packages to make it easier to
> >>>>>> create a minimal install, might it be best to just drop
> >>>>>> all of the manpages into an sssd-docs subpackage (that
> >>>>>> gets pulled in by the sssd metapackage)? Including
> >>>>>> translations, those can add up. Which brings me to my
> >>>>>> next point:
> >>>>>> 
> >>>>>> The provider subpackages have only the English manpage 
> >>>>>> included in them. The translated ones are all landing
> >>>>>> in the sssd-common package.
> >>>>> 
> >>>>> I don't really like this option. As a user of SSSD I
> >>>>> would expect the sssd-ad man page to be present if I
> >>>>> installed the sssd-ad package. Also, I think it is better
> >>>>> to have a single man page installed (even if it's along
> >>>>> with the translations) for the provider I care about than
> >>>>> the whole bunch. The space savings provided by this split
> >>>>> would also come by the means of not installing all the
> >>>>> dependencies rather than not installing the files that
> >>>>> form the providers.
> >>>>> 
> >>>>> Thank you for the review, new patches are attached.
> >>>>> 
> >> 
> >> 
> >> Hmm, the translated manpages seem to have disappeared from the 
> >> builds entirely:
> >> 
> >> 
> >>> I see the same behaviour with parallel build (also with
> >>> master w/o these patches), but with an in-tree build, the man
> >>> pages are generated fine.
> >> 
> >>> Here is a scratch build I built: 
> >>> http://koji.fedoraproject.org/koji/taskinfo?taskID=5490069
> >> 
> >>> For instance, check out the sssd-ldap subpackage: $ rpm -qpl 
> >>> sssd-ldap-1.9.94-0.fc19.x86_64.rpm | grep man 
> >>> /usr/share/man/ja/man5/sssd-ldap.5.gz 
> >>> /usr/share/man/man5/sssd-ldap.5.gz 
> >>> /usr/share/man/uk/man5/sssd-ldap.5.gz
> >> 
> >> 
> >> [sgallagh@sgallagh520:x86_64 (master *)]$ rpm -qpl 
> >> /home/sgallagh/workspace/sssd/x86_64/rpmbuild/RPMS/x86_64/sssd-*.rpm
> >>
> >>
> >
> >>  
|grep man
> >> /usr/share/man/man5/sssd-ad.5.gz
> >> /usr/share/man/man8/pam_sss.8.gz 
> >> /usr/share/man/man8/sssd_krb5_locator_plugin.8.gz 
> >> /usr/share/man/man1/sss_ssh_authorizedkeys.1.gz 
> >> /usr/share/man/man1/sss_ssh_knownhostsproxy.1.gz 
> >> /usr/share/man/man5/sssd-simple.5.gz 
> >> /usr/share/man/man5/sssd-sudo.5.gz 
> >> /usr/share/man/man5/sssd.conf.5.gz 
> >> /usr/share/man/man8/sss_cache.8.gz
> >> /usr/share/man/man8/sssd.8.gz /usr/share/man/man5/sssd-ipa.5.gz
> >>  /usr/share/man/man5/sssd-krb5.5.gz 
> >> /usr/share/man/man5/sssd-ldap.5.gz 
> >> /usr/share/man/man8/sss_debuglevel.8.gz 
> >> /usr/share/man/man8/sss_groupadd.8.gz 
> >> /usr/share/man/man8/sss_groupdel.8.gz 
> >> /usr/share/man/man8/sss_groupmod.8.gz 
> >> /usr/share/man/man8/sss_groupshow.8.gz 
> >> /usr/share/man/man8/sss_obfuscate.8.gz 
> >> /usr/share/man/man8/sss_seed.8.gz 
> >> /usr/share/man/man8/sss_useradd.8.gz 
> >> /usr/share/man/man8/sss_userdel.8.gz 
> >> /usr/share/man/man8/sss_usermod.8.gz
> >> 
> >> 
> >> I'm running it as a Koji scratch-build right now to make sure
> >> it's not some peculiarity of my system, but I think there's
> >> something wrong with the *.lang files :-/
> > 
> > 
> > My scratch-build against F20 also did not include translated
> > manpages: 
> > http://koji.fedoraproject.org/koji/taskinfo?taskID=5488765
> > 
> > Something broke somewhere, I assume. Let's track this down before
> > we push these patches, please.
>  
>  The version of the Koji build shows that you tested with the
>  previous patches.
>  
>  How exactly did you create the SRPM? Does the same sequence work
>  without the patches?
>  
>  Did you create the SRPM that was rebuilt in koji using a parallel
>  build or an in-tree build? A parallel build didn't work for me
>  either most probably because po4a couldn't find the strings:
>   
Ahh, ok. Yes, I generated the SRPM in a parallel tree. So we need to
figure out eventually why 'make distdir' isn't pulling the
translations in properly. Let's open a separate bug for that.

For now: Ack to these patches.

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlG3MJUACgkQeiVVYja6o6N6TACdG9kQ4j9sOsdcScINOCqcS7YY
> LogAmgMKRnx4XsHjPAImzmwnbWLNu2a0
> =dX2/
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Tue, Jun 11, 2013 at 10:13:41AM -0400, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 06/11/2013 10:07 AM, Jakub Hrozek wrote:
>  > On Tue, Jun 11, 2013 at 09:37:49AM -0400, Stephen Gallagher wrote:
>  >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>  >> 
>  >> On 06/11/2013 06:03 AM, Jakub Hrozek wrote:
>  >>> On Mon, Jun 10, 2013 at 03:49:34PM -0400, Stephen Gallagher
>  >>> wrote: On 06/10/2013 02:24 PM, Jakub Hrozek wrote:
>  >>>>>> On Mon, Jun 10, 2013 at 08:56:37AM -0400, Stephen
>  >>>>>> Gallagher wrote:
>  >>>>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>  >>>>>>> 
>  >>>>>>> On 06/10/2013 07:17 AM, Lukas Slebodnik wrote:
>  >>>>>>>> On (10/06/13 10:03), Jakub Hrozek wrote:
>  >>>>>>>>> On Fri, Jun 07, 2013 at 06:24:48PM +0200, Lukas 
>  >>>>>>>>> Slebodnik wrote:
>  >>>>>>>>>> 1. I like idea of divided subpackages. If
>  >>>>>>>>>> someone wants only ldap backend, he needn't
>  >>>>>>>>>> install samba-libs (and its dependencies)
>  >>>>>>>>>> 
>  >>>>>>>>>> 2. There isn't any rpmlint warnings.
>  >>>>>>>>>> 
>  >>>>>>>>>> I tested yum upgrade upgrade with installed
> sssd
>  >>>>>>>>>> and freeipa-client. New packages were installed
>  >>>>>>>>>> for dependencies: sssd-ad sssd-common sssd-ipa
>  >>>>>>>>>> sssd-krb5 sssd-krb5-common sssd-ldap Everything
>  >>>>>>>>>> worked as expected.
>  >>>>>>>>>> 
>  >>>>>>>>>> Then I decided to remove sssd-ad: yum remove
>  >>>>>>>>>> sssd-ad and packeges "freeipa-client,
> sssd" were
>  >>>>>>>>>> also removed. I was little bit confused,
> because
>  >>>>>>>>>> I didn't want to remove sssd and sssd
> replied to
>  >>>>>>>>>> getent command after packages
> "freeipa-client,
>  >>>>>>>>>> sssd" were removed.
>  >>>>>>>>> 
>  >>>>>>>>> That's because freeipa-client currently
> requires
>  >>>>>>>>> sssd, we might want open a ticket to make them
>  >>>>>>>>> require just sssd-ipa.
>  >>>>>>>>> 
>  >>>>>>>> 
>  >>>>>>>> The most confusing thing was for me that sssd was 
>  >>>>>>>> removed. Yes, we should file a ticket to
>  >>>>>>>> freeipa-client after releasing sssd.
>  >>>>>>>> 
>  >>>>>>>>>> 
>  >>>>>>>>>> I think, that other users may be also confused
>  >>>>>>>>>> with this situation. Then I looked to the patch
>  >>>>>>>>>> and I found out, that: --sssd is only
> "meta
>  >>>>>>>>>> package",which require all backedns
> subpackages
>  >>>>>>>>>> --sssd doesn't contain any useful files
>  >>>>>>>>>> --everything important is in package
>  >>>>>>>>>> sssd-common.
>  >>>>>>>>>> 
>  >>>>>>>>>> 
>  >>>>>>>>>> Maybe we should update package description of
>  >>>>>>>>>> sssd and sssd-common. I hope that system
>  >>>>>>>>>> administrators relies on output of "yum
> info" and
>  >>>>>>>>>> there isn't it very well explained.
>  >>>>>>>>>> 
>  >>>>>>>>> 
>  >>>>>>>>> Thanks, I updated the summary of both sssd and 
>  >>>>>>>>> sssd-common. Hopefully it would be clearer now.
>  >>>>>>>>> 
>  >>>>>>>> 
>  >>>>>>>> Thank you.
>  >>>>>>>> 
>  >>>>>>>>>> Summary: Everything works well, but I was
> little
>  >>>>>>>>>> bit confused.
>  >>>>>>>>>> 
>  >>>>>>>>>> Any other opinions?
>  >>>>>>>>>> 
>  >>>>>>>>>> One nitpick inline
>  >>>>>>>>>> 
>  >>>>>>>>>> 
>  >>>>>>>> 
>  >>>>>>>> I think, that patches are OK. Does anybody have
>  >>>>>>>> another comments or objections?
>  >>>>>>>> 
>  >>>>>>> 
>  >>>>>>> Nack.
>  >>>>>>> 
>  >>>>>>> When adding "Provides:" and
> "Obsoletes:", it is 
>  >>>>>>> inappropriate to do Obsoletes: libsss_sudo < 
>  >>>>>>> %{version}-%{release}
>  >>>>>>> 
>  >>>>>>> The Fedora guidelines require that the Obsoletes: be a 
>  >>>>>>> specific version (the last one before the change
>  >>>>>>> occurred). So
>  >>>>>>> 
>  >>>>>>> Obsoletes: libsss_sudo <= 1.9.5
>  >>>>>>> 
>  >>>>>> 
>  >>>>>> OK, in my defense I've seen these in some of the
>  >>>>>> specfiles I've got cloned from Fedora's dist-git (not
>  >>>>>> that it would make them correct).
>  >>>>>> 
>  >>>>>> I made the changes in the version that is attached, just
>  >>>>>> with a differens version to compare against. I think
>  >>>>>> your suggestion would make sense for a Fedora package,
>  >>>>>> but in upstream, we want to obsolete all the nightly
>  >>>>>> builds prior to the switch as well.
>  >>>>>> 
>  >>>>>> So I added another point version number (minor versions
>  >>>>>> come cheap) and compared against 1.9.93.
>  >>>>>> 
>  >>> 
>  >>> It needs to be <= 1.9.93. Just less-than won't upgrade
>  >>> packages *at* 1.9.93
>  >>> 
>  >>> Wouldn't things be easier to remember if you just bumped
>  >>> version.m4 to 1.9.94 instead? Then you don't need to remember
>  >>> to revert the specfile patch. (You can just release 1.9.94. No
>  >>> one is really going to notice if the upstream minor release
>  >>> number jumped two digits instead of one between pre-releases).
>  >>> 
>  >>> 
>  >>>> Yes, if it's OK to bump the version then it's definitely
>  >>>> easier. Done.
>  >>> 
>  >>> 
>  >>>>>>> The reason for this is so we don't force yum to handle 
>  >>>>>>> Obsoletes processing at every update (which I'm told
>  >>>>>>> is expensive).
>  >>>>>>> 
>  >>>>>> 
>  >>>>>> I see, I didn't know that.
>  >>>>>> 
>  >>> 
>  >>> Technically, it also allows the capability to add that back in
>  >>> as a package or subpackage at a higher version at some later
>  >>> date without breakage (because the re-added one would be at a
>  >>> higher N-V-R). That's not something that matters to us though,
>  >>> just a bit of academia.
>  >>> 
>  >>>>>>> We only need to carry this explicit Obsoletes: until
>  >>>>>>> Fedora no longer has install media with the older
>  >>>>>>> version (so until F19 is the oldest supported
>  >>>>>>> version).
>  >>>>>>> 
>  >>>>>>> 
>  >>>>>>> Why do you have Conflicts: sssd <
>  >>>>>>> %{version}-%{release}? Given that the packages are
>  >>>>>>> identically-named (and that name isn't
> 'kernel'), this
>  >>>>>>> should be unnecessary.
>  >>>>>>> 
>  >>>>>> 
>  >>>>>> I wanted to be really defensive and make sure that
>  >>>>>> everything the old version contained is gone with the new
>  >>>>>> version. That Conflicts: is gone with the attached
>  >>>>>> patches.
>  >>>>>> 
>  >>>>>>> 
>  >>>>>>> Is there a particular reason that the proxy provider 
>  >>>>>>> doesn't have a subpackage as well?
>  >>>>>>> 
>  >>>>>> 
>  >>>>>> Just that the proxy provider has no dependencies. But
>  >>>>>> from a logical standpoint it makes sense, so there is a
>  >>>>>> proxy subpackage now, too.
>  >>>>>> 
>  >>> 
>  >>> Thanks, but you forgot to add it to the dependencies of the
>  >>> 'sssd' meta-package.
>  >>> 
>  >>> 
>  >>>> Oops, fixed.
>  >>> 
>  >>> 
>  >>> If we're being pedantic about this, we should probably split
>  >>> out the 'simple' provider as well, but I'm not going to nack
>  >>> over it. It's < 40k.
>  >>> 
>  >>> 
>  >>>> I think it's better if the simple provider stays in the
>  >>>> common package. It can be used along with any other provider,
>  >>>> so the other providers would have to require it anyway. Also,
>  >>>> it has no dependencies.
>  >>> 
>  >>> 
>  >>>>>>> If we're breaking up packages to make it easier to
>  >>>>>>> create a minimal install, might it be best to just drop
>  >>>>>>> all of the manpages into an sssd-docs subpackage (that
>  >>>>>>> gets pulled in by the sssd metapackage)? Including
>  >>>>>>> translations, those can add up. Which brings me to my
>  >>>>>>> next point:
>  >>>>>>> 
>  >>>>>>> The provider subpackages have only the English manpage 
>  >>>>>>> included in them. The translated ones are all landing
>  >>>>>>> in the sssd-common package.
>  >>>>>> 
>  >>>>>> I don't really like this option. As a user of SSSD I
>  >>>>>> would expect the sssd-ad man page to be present if I
>  >>>>>> installed the sssd-ad package. Also, I think it is better
>  >>>>>> to have a single man page installed (even if it's along
>  >>>>>> with the translations) for the provider I care about than
>  >>>>>> the whole bunch. The space savings provided by this split
>  >>>>>> would also come by the means of not installing all the
>  >>>>>> dependencies rather than not installing the files that
>  >>>>>> form the providers.
>  >>>>>> 
>  >>>>>> Thank you for the review, new patches are attached.
>  >>>>>> 
>  >>> 
>  >>> 
>  >>> Hmm, the translated manpages seem to have disappeared from the 
>  >>> builds entirely:
>  >>> 
>  >>> 
>  >>>> I see the same behaviour with parallel build (also with
>  >>>> master w/o these patches), but with an in-tree build, the man
>  >>>> pages are generated fine.
>  >>> 
>  >>>> Here is a scratch build I built: 
>  >>>> http://koji.fedoraproject.org/koji/taskinfo?taskID=5490069
>  >>> 
>  >>>> For instance, check out the sssd-ldap subpackage: $ rpm -qpl 
>  >>>> sssd-ldap-1.9.94-0.fc19.x86_64.rpm | grep man 
>  >>>> /usr/share/man/ja/man5/sssd-ldap.5.gz 
>  >>>> /usr/share/man/man5/sssd-ldap.5.gz 
>  >>>> /usr/share/man/uk/man5/sssd-ldap.5.gz
>  >>> 
>  >>> 
>  >>> [sgallagh@sgallagh520:x86_64 (master *)]$ rpm -qpl 
>  >>> /home/sgallagh/workspace/sssd/x86_64/rpmbuild/RPMS/x86_64/sssd-*.rpm
>  >>>
>  >>>
>  >>
>  >>> 
>  |grep man
>  >>> /usr/share/man/man5/sssd-ad.5.gz
>  >>> /usr/share/man/man8/pam_sss.8.gz 
>  >>> /usr/share/man/man8/sssd_krb5_locator_plugin.8.gz 
>  >>> /usr/share/man/man1/sss_ssh_authorizedkeys.1.gz 
>  >>> /usr/share/man/man1/sss_ssh_knownhostsproxy.1.gz 
>  >>> /usr/share/man/man5/sssd-simple.5.gz 
>  >>> /usr/share/man/man5/sssd-sudo.5.gz 
>  >>> /usr/share/man/man5/sssd.conf.5.gz 
>  >>> /usr/share/man/man8/sss_cache.8.gz
>  >>> /usr/share/man/man8/sssd.8.gz /usr/share/man/man5/sssd-ipa.5.gz
>  >>>  /usr/share/man/man5/sssd-krb5.5.gz 
>  >>> /usr/share/man/man5/sssd-ldap.5.gz 
>  >>> /usr/share/man/man8/sss_debuglevel.8.gz 
>  >>> /usr/share/man/man8/sss_groupadd.8.gz 
>  >>> /usr/share/man/man8/sss_groupdel.8.gz 
>  >>> /usr/share/man/man8/sss_groupmod.8.gz 
>  >>> /usr/share/man/man8/sss_groupshow.8.gz 
>  >>> /usr/share/man/man8/sss_obfuscate.8.gz 
>  >>> /usr/share/man/man8/sss_seed.8.gz 
>  >>> /usr/share/man/man8/sss_useradd.8.gz 
>  >>> /usr/share/man/man8/sss_userdel.8.gz 
>  >>> /usr/share/man/man8/sss_usermod.8.gz
>  >>> 
>  >>> 
>  >>> I'm running it as a Koji scratch-build right now to make sure
>  >>> it's not some peculiarity of my system, but I think there's
>  >>> something wrong with the *.lang files :-/
>  >> 
>  >> 
>  >> My scratch-build against F20 also did not include translated
>  >> manpages: 
>  >> http://koji.fedoraproject.org/koji/taskinfo?taskID=5488765
>  >> 
>  >> Something broke somewhere, I assume. Let's track this down before
>  >> we push these patches, please.
>  > 
>  > The version of the Koji build shows that you tested with the
>  > previous patches.
>  > 
>  > How exactly did you create the SRPM? Does the same sequence work
>  > without the patches?
>  > 
>  > Did you create the SRPM that was rebuilt in koji using a parallel
>  > build or an in-tree build? A parallel build didn't work for me
>  > either most probably because po4a couldn't find the strings:
>  > 
>  
>  Ahh, ok. Yes, I generated the SRPM in a parallel tree. So we need to
>  figure out eventually why 'make distdir' isn't pulling the
>  translations in properly. Let's open a separate bug for that.
>  
>  For now: Ack to these patches. 
Thank you for the review. I filed
https://fedorahosted.org/sssd/ticket/1978 to track the parallel build
problem.

The patches were pushed to master.