[SSSD] [PATCH] Split the providers into separate subpackages
Jakub Hrozek
jhrozek at redhat.com
Tue Jun 11 14:42:28 UTC 2013
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 at 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.
More information about the sssd-devel
mailing list