1:24 p.m.
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.