9:13 a.m.
-----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-----