[Bug 487296] Review Request: sssd - System Security Services Daemon

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=487296





--- Comment #5 from Jakub Hrozek <jhrozek at redhat.com>  2009-03-04 09:46:14 EDT ---
(In reply to comment #4)
> rpmlint output:
> sssd.i386: W: no-documentation
> sssd.i386: W: devel-file-in-non-devel-package /usr/lib/sssd/libsss_proxy.so
> sssd.i386: W: devel-file-in-non-devel-package /usr/lib/sssd/libsss_ldap.so
> 
> Can you please comment on these?
> 

No documentation is expected, there really is not one (yet?). See below for the
.so issue.

> %files section:
> %{_sharedstatedir}/sss/ expands to /usr/com/sss/ which doesn't exist. I think
> you meant %{_localstatedir}/%{_lib}/sss/
> 

On my F10 system:
$ rpm --showrc | grep /var/lib
-14: _sharedstatedir /var/lib

RPM documentation really says that sharedstatedir expands to /usr/com/sss but
it's not true. FWIW, the macro is defined in /usr/lib/rpm/platform/*. There is
also a corresponding autotools macro.

> Additionally, I think the %{sssd_release} and %{sssd_version} macros aren't
> really useful. Please consider using standard macros.
> 

Umm, okay. I pretty much continued using them since they were in the specfile I
based this upon and also in all the related samba packages.

> Please take a look at the following MUST and SHOULD items:
> MUST: The package must be licensed with a Fedora approved license and meet the
> Licensing Guidelines.
> - according to the Licensing Guidelines, "In addition, the package must ontain
> a comment explaining the multiple licensing breakdown.", please see section
> 1.2.9

OK.

> MUST: The sources used to build the package must match the upstream source, as
> provided in the spec URL. Reviewers should use md5sum for this task. If no
> upstream URL can be specified for this package, please see the Source URL
> Guidelines for how to deal with this.

Fair point, but there is no upstream tarball yet as there has been no release.
For the time being, I've put a comment that describes how to create the tarball
that's in srpm. If there's no released tarball by the F-11 deadline, we still
might package a git snapshot as described in
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease. If
you consider this important I might do this also for every round of reviewed
packages.

> MUST: Every binary RPM package (or subpackage) which stores shared library
> files (not just symlinks) in any of the dynamic linker's default paths, must
> call ldconfig in %post and %postun.

/usr/lib/sssd/ is not linker's default path. The symlinks are created manually
during "make install". If we want to have them created via ldconfig, we should
drop a config file to /etc/ld.so.conf.d/. But that'd work for rpm only anyway.

> MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
> then library files that end in .so (without suffix) must go in a -devel
> package.

So we would have a -devel subpackage that contains just the two symlinks? It's
certainly doable, but I wonder if we need to ship these versionless symlinks at
all..

> SHOULD: If the source package does not include license text(s) as a separate
> file from upstream, the packager SHOULD query upstream to include it.
> 

OK. Consider upstream queried :-)

> I have some problems with mock, so I will try building the package in mock
> later.

If you're trying to build rawhide packages on F-10 host machine in mock, keep
in mind that there is a bug mock that prevented doing so (#487430). Maybe the
quickest way now is to grab some rawhide box..

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.