Bug 833411 - Review Request: realmd - Kerberos realm enrollment service
Review Request: realmd - Kerberos realm enrollment service
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Yanko Kaneti
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-19 08:41 EDT by Stef Walter
Modified: 2012-07-14 07:40 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-07-13 11:41:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
yaneti: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Stef Walter 2012-06-19 08:41:42 EDT
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 09:20:34 EDT
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 09:31:59 EDT
and %doc AUTHORS COPYING ChangeLog NEWS README
seems like a better fit than empty docs
Comment 3 Yanko Kaneti 2012-06-19 09:39:06 EDT
I've looked through some of the source files and they all look like LGPLv2+ not LGPLv2
Comment 4 Stef Walter 2012-06-19 09:52:20 EDT
(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 10:06:47 EDT
(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 10:19:34 EDT
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 06:05:35 EDT
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-22 22:54:48 EDT
I am a sponsor, and can sponsor you.
Comment 9 Yanko Kaneti 2012-06-26 15:39:37 EDT
Package looks good.

APPROVED
Comment 10 Stef Walter 2012-06-28 08:56:14 EDT
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 09:51:52 EDT
Git done (by process-git-requests).

Removed baz, not a valid FAS account.
Comment 12 Yanko Kaneti 2012-07-13 11:41:21 EDT
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 07:40:10 EDT
Alright. Will do.

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