Bug 833411 - Review Request: realmd - Kerberos realm enrollment service
Summary: Review Request: realmd - Kerberos realm enrollment service
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Yanko Kaneti
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-06-19 12:41 UTC by Stef Walter
Modified: 2012-07-14 11:40 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-07-13 15:41:21 UTC
Type: ---
Embargoed:
yaneti: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Stef Walter 2012-06-19 12:41:42 UTC
Spec URL: http://people.freedesktop.org/~stefw/rpm/realmd.spec
SRPM URL: http://people.freedesktop.org/~stefw/rpm/realmd-0.3-1.fc17.src.rpm

Description:
realmd is a dbus system service which manages discovery and enrollment in realms
and domains like Active Directory or IPA. The control center uses realmd as the
backend to 'join' a domain simply and automatically configure things correctly.

Fedora Account System Username: stefw

This is my first Fedora package, I hope I have all my ducks in a row here :)

... and I also need a sponsor. I'm the upstream maintainer of realmd. I also am a contributor to the GNOME project, including gnome-keyring, seahorse and many other parts.

I've done a koji build for this package: http://koji.fedoraproject.org/koji/taskinfo?taskID=4176640

Comment 1 Yanko Kaneti 2012-06-19 13:20:34 UTC
try rpmlint on the result:  (here is an excerpt)
realmd.x86_64: E: explicit-lib-dependency PackageKit-glib
realmd.x86_64: E: explicit-lib-dependency glib2
realmd.x86_64: E: explicit-lib-dependency krb5-libs
realmd.x86_64: W: incoherent-version-in-changelog 0.3 ['0.3-1.fc18', '0.3-1']

You don't need to include explicit deps on shared libraries which will be picked up by the rpm dep generator.

Also from 
BuildRequires:  automake libtool intltool pkgconfig
you only need intltool   Unless you auto(make/conf/reconf).

Explicit BuildRoot: tag is not needed since a release or two ago.
Same for %defattr(-,root,root,-)
Same for the explicit rm -f %{buildroot} in the start of %clean or %install
Group:   is obosolete

You should own %{_libdir}/realmd, as in
%dir %{_libdir}/realmd
and
%dir %{_libdir}/realmd/provider.d

Comment 2 Yanko Kaneti 2012-06-19 13:31:59 UTC
and %doc AUTHORS COPYING ChangeLog NEWS README
seems like a better fit than empty docs

Comment 3 Yanko Kaneti 2012-06-19 13:39:06 UTC
I've looked through some of the source files and they all look like LGPLv2+ not LGPLv2

Comment 4 Stef Walter 2012-06-19 13:52:20 UTC
(In reply to comment #1)
> try rpmlint on the result:  (here is an excerpt)

Oddly enough, my rpmlint doesn't show those warnings:

[stef@stef-rawhide rpm]$ rpmlint SPECS/realmd.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

> realmd.x86_64: E: explicit-lib-dependency PackageKit-glib
> realmd.x86_64: E: explicit-lib-dependency glib2
> realmd.x86_64: E: explicit-lib-dependency krb5-libs
> realmd.x86_64: W: incoherent-version-in-changelog 0.3 ['0.3-1.fc18', '0.3-1']
> 
> You don't need to include explicit deps on shared libraries which will be
> picked up by the rpm dep generator.

Interesting. Thanks, removed them.

> Also from 
> BuildRequires:  automake libtool intltool pkgconfig
> you only need intltool   Unless you auto(make/conf/reconf).

We do run pkg-config, or is that build requirement automatically provided?

> Explicit BuildRoot: tag is not needed since a release or two ago.
> Same for %defattr(-,root,root,-)
> Same for the explicit rm -f %{buildroot} in the start of %clean or %install
> Group:   is obosolete

Removed all those lines.

> You should own %{_libdir}/realmd, as in
> %dir %{_libdir}/realmd
> and
> %dir %{_libdir}/realmd/provider.d

Done.

(In reply to comment #2)
> and %doc AUTHORS COPYING ChangeLog NEWS README
> seems like a better fit than empty docs

Done.

(In reply to comment #3)
> I've looked through some of the source files and they all look like LGPLv2+
> not LGPLv2

Thanks, updated the License: line.

Updated spec file: http://people.freedesktop.org/~stefw/rpm/realmd.spec
New SRPM: http://people.freedesktop.org/~stefw/rpm/realmd-0.3-2.fc17.src.rpm

Comment 5 Yanko Kaneti 2012-06-19 14:06:47 UTC
(In reply to comment #4)
> (In reply to comment #1)
> > try rpmlint on the result:  (here is an excerpt)
> 
> Oddly enough, my rpmlint doesn't show those warnings:
> 
> [stef@stef-rawhide rpm]$ rpmlint SPECS/realmd.spec 
> 0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Probably because you've only tried it on the spec.
Try it on the spec, srpm, built rpms and even the debuginfo.
Here are additional errors it finds here. rpmlint-1.4-6.fc17.noarch

realmd.x86_64: E: incorrect-fsf-address /usr/share/doc/realmd-0.3/COPYING
realmd-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/realmd-0.3/service/realm-command.c
realmd-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/realmd-0.3/service/realm-command.h
 
> > Also from 
> > BuildRequires:  automake libtool intltool pkgconfig
> > you only need intltool   Unless you auto(make/conf/reconf).

> We do run pkg-config, or is that build requirement automatically provided?

Its currently required by rpm-build which is in the minimal build root
http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2
I think it should be in the Expections list but, but its fine if it stays.

Comment 6 Yanko Kaneti 2012-06-19 14:19:34 UTC
I don't personally think the fsf address thing can be considered a blocker.

There is still a lingering 
Requires: polkit

Also I would approve the package and sponsor you, but I am not in the sponsors group.

Comment 7 Stef Walter 2012-06-22 10:05:35 UTC
Thanks. Fixed various rpmlint warnings, and removed polkit dep.

Uploaded new spec and packages here: http://stefw.fedorapeople.org/rpm/realmd/

Comment 8 Matthias Clasen 2012-06-23 02:54:48 UTC
I am a sponsor, and can sponsor you.

Comment 9 Yanko Kaneti 2012-06-26 19:39:37 UTC
Package looks good.

APPROVED

Comment 10 Stef Walter 2012-06-28 12:56:14 UTC
Thanks guys.

New Package SCM Request
=======================
Package Name: realmd
Short Description: realmd is a dbus system service which manages discovery and enrollment in realms and domains like Active Directory or IPA.
Owners: stefw
Branches:
InitialCC: baz

Comment 11 Gwyn Ciesla 2012-06-28 13:51:52 UTC
Git done (by process-git-requests).

Removed baz, not a valid FAS account.

Comment 12 Yanko Kaneti 2012-07-13 15:41:21 UTC
This was built and is already in rawhide.
stefw , on your next package review please close the bug when its built and on its way to the repo.

Comment 13 Stef Walter 2012-07-14 11:40:10 UTC
Alright. Will do.


Note You need to log in before you can comment on or make changes to this bug.