Bug 810010 - Review Request: genders - file based database for cluster managment
Review Request: genders - file based database for cluster managment
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Thomas Spura
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 810019
  Show dependency treegraph
 
Reported: 2012-04-04 17:45 EDT by David Brown
Modified: 2014-07-07 23:47 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-24 19:30:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tomspur: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description David Brown 2012-04-04 17:45:07 EDT
Spec URL: http://dmlb2000.homelinux.org/packages/genders.spec
SRPM URL: http://dmlb2000.homelinux.org/packages/genders-1.18-1.fc18.src.rpm
Description: Genders is a static cluster configuration database used for cluster configuration management. It is used by a variety of tools and scripts for management of large clusters. The Genders database is accessed by every node in a cluster, either through a networked file system or by replicating the database on every node of the cluster. The database describes the layout and configuration of the cluster so that tools and scripts can sense the variations of cluster nodes. By abstracting this information into a plain text file, it becomes possible to change the configuration of a cluster by modifying only one file.
Comment 1 David Brown 2012-04-04 18:51:50 EDT
Oh, by the way this is a new package for me so I'll be needing a sponsor for this one.
Comment 2 Thomas Spura 2012-04-07 19:27:58 EDT
For more information on how to get sponsored see:
http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Did you already do some informal reviews? Could you link them here, please?

The general guidelines are at:
http://fedoraproject.org/wiki/Packaging:Guidelines

As you are using python and perl, this also applies:
http://fedoraproject.org/wiki/Packaging:Python
http://fedoraproject.org/wiki/Packaging:Perl

First iteration of blocker issues:
- use a full url, that is wget'able:
  http://fedoraproject.org/wiki/Packaging:SourceURL
- Please include the manpage as genders.3.* and not as genders.3.gz, so the compression format can change at any time
- *.la should be removed, whenever possible...
- When you really want to package the static libraries, they need to have it's own -static package. In corner cases -static and -devel is the same package, but only, when there are no shared libraries included:
  http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries
- Please use parallel make:
  http://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make
- Be more specific in python %files and use %{python_sitearch}, e.g.
  %{python_sitearch}/%{name}.py*
  %{python_sitearch}/lib%{name}.so
  %{python_sitearch}/lib%{name}-*egg-info
- Always add a changelog entry to the footer and bump the release when you do changes (and describe them in the changelog entry)

- Further rpmlint issues:

$ rpmlint /home/tom/rpmbuild/SRPMS/genders-1.18-1.fc16.src.rpm /home/tom/rpmbuild/RPMS/x86_64/genders-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/genders-compat-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/genders-perl-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/genders-python-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/libgenders-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/libgenders-devel-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/libgendersplusplus-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/libgendersplusplus-devel-1.18-1.fc16.x86_64.rpm /home/tom/rpmbuild/RPMS/x86_64/genders-debuginfo-1.18-1.fc16.x86_64.rpm
genders.src: W: summary-ended-with-dot C Static cluster configuration database.
genders.src: E: no-changelogname-tag
genders.src: W: invalid-license GPL
genders.src:78: W: rpm-buildroot-usage %build --with-extension-destdir="$RPM_BUILD_ROOT"
genders.src:129: E: hardcoded-library-path in %{_prefix}/lib/genders/*
genders.src: W: invalid-url Source0: genders-1.18.tar.gz
genders.x86_64: W: summary-ended-with-dot C Static cluster configuration database.
genders.x86_64: E: no-changelogname-tag
genders.x86_64: W: invalid-license GPL
genders-compat.x86_64: W: spelling-error Summary(en_US) compatability -> comparability, compatibility, communicability
genders-compat.x86_64: W: summary-not-capitalized C compatability library
genders-compat.x86_64: E: no-changelogname-tag
genders-compat.x86_64: W: invalid-license GPL
genders-compat.x86_64: W: only-non-binary-in-usr-lib
genders-compat.x86_64: E: script-without-shebang /usr/lib/genders/hostlist.pl
genders-compat.x86_64: E: script-without-shebang /usr/lib/genders/gendlib.pl
genders-perl.x86_64: W: summary-not-capitalized C perl libraries
genders-perl.x86_64: E: no-changelogname-tag
genders-perl.x86_64: W: invalid-license GPL
genders-perl.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/Libgenders/Libgenders.so Libgenders.so()(64bit)
genders-perl.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/Libgenders/Libgenders.so Libgenders.so()(64bit)
genders-perl.x86_64: E: zero-length /usr/lib64/perl5/vendor_perl/auto/Libgenders/Libgenders.bs
genders-perl.x86_64: W: hidden-file-or-dir /usr/lib64/perl5/vendor_perl/auto/Libgenders/.packlist
genders-perl.x86_64: W: perl-temp-file /usr/lib64/perl5/vendor_perl/auto/Libgenders/.packlist
genders-perl.x86_64: E: non-standard-executable-perm /usr/lib64/perl5/vendor_perl/auto/Libgenders/Libgenders.so 0555L
genders-python.x86_64: W: summary-not-capitalized C python libraries
genders-python.x86_64: E: no-changelogname-tag
genders-python.x86_64: W: invalid-license GPL
genders-python.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/libgenders.so libgenders.so()(64bit)
genders-python.x86_64: W: no-documentation
libgenders.x86_64: W: summary-not-capitalized C genders libraries
libgenders.x86_64: E: no-changelogname-tag
libgenders.x86_64: W: invalid-license GPL
libgenders.x86_64: W: shared-lib-calls-exit /usr/lib64/libgenders.so.0.3.0 exit@GLIBC_2.2.5
libgenders.x86_64: E: library-without-ldconfig-postin /usr/lib64/libgenders.so.0.3.0
libgenders.x86_64: E: library-without-ldconfig-postun /usr/lib64/libgenders.so.0.3.0
libgenders-devel.x86_64: W: no-dependency-on libgenders/libgenders-libs/liblibgenders
libgenders-devel.x86_64: W: summary-not-capitalized C genders development libraries
libgenders-devel.x86_64: W: non-standard-group System Environment/Development
libgenders-devel.x86_64: E: no-changelogname-tag
libgenders-devel.x86_64: W: invalid-license GPL
libgendersplusplus.x86_64: W: summary-not-capitalized C genders libraries
libgendersplusplus.x86_64: E: no-changelogname-tag
libgendersplusplus.x86_64: W: invalid-license GPL
libgendersplusplus.x86_64: W: invalid-url URL: http://genders.sf.net <urlopen error [Errno 104] Connection reset by peer>
libgendersplusplus.x86_64: W: no-documentation
libgendersplusplus.x86_64: E: library-without-ldconfig-postin /usr/lib64/libgendersplusplus.so.1.0.0
libgendersplusplus.x86_64: E: library-without-ldconfig-postun /usr/lib64/libgendersplusplus.so.1.0.0
libgendersplusplus-devel.x86_64: W: no-dependency-on libgendersplusplus/libgendersplusplus-libs/liblibgendersplusplus
libgendersplusplus-devel.x86_64: W: summary-not-capitalized C genders development libraries
libgendersplusplus-devel.x86_64: W: non-standard-group System Environment/Development
libgendersplusplus-devel.x86_64: E: no-changelogname-tag
libgendersplusplus-devel.x86_64: W: invalid-license GPL
libgendersplusplus-devel.x86_64: W: no-documentation
genders-debuginfo.x86_64: E: no-changelogname-tag
genders-debuginfo.x86_64: W: invalid-license GPL
10 packages and 0 specfiles checked; 19 errors, 37 warnings.

You can look, what those errors/warnings mean with "rpmlint -I", e.g.:

$ rpmlint -I library-without-ldconfig-postin
library-without-ldconfig-postin:
This package contains a library and provides no %post scriptlet containing a
call to ldconfig.

  Solution to this is here:
  http://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
Comment 3 David Brown 2012-04-09 11:44:10 EDT
Okay updated the genders.spec and src.rpm in the links above to include some of the fixes. The command rpmlint shows no errors and only a couple of warnings.

I'm not sure how to ignore the autogenerated libfoo.so provides for the python and perl packages.

Thanks for checking out the package.
Comment 4 David Brown 2012-04-09 17:52:14 EDT
I've updated the genders.spec and src.rpm to remove the rhel checks entirely and now it builds fine on RHEL with EPEL and rpmlint shows more warnings about groups not specified.
Comment 5 Tom "spot" Callaway 2012-04-11 16:38:38 EDT
Just some drive-by comments:

* Your BuildRoot value is invalid, please see:
https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag

* The "." on its own line in each %description is unnecessary, please omit.

* When doing explicit Requires, you should be doing it differently, see:
https://fedoraproject.org/wiki/Packaging/Guidelines#Requires

As an example, you currently have:

Requires: libgenders-devel = %{version}, libgendersplusplus = %{version}

This should be:

Requires: libgenders-devel%{?_isa} = %{version}-%{release}
Requires: libgendersplusplus%{?_isa} = %{version}-%{release}

(splitting it across two lines is not required, i just did that for legibility)

* You are mixing macros, specifically, "$RPM_BUILD_ROOT" and "%{buildroot}". You need to choose one and use it consistently. I strongly recommend using "%{buildroot}".

* Your %defattr lines should be "%defattr(-,root,root,-)"

* To filter out those provides, see: https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering

* When you make changes, please increment Release and add a new entry to the %changelog, so we can see the progress over time. :)
Comment 6 David Brown 2012-04-11 17:56:31 EDT
Updated the URLs above with the changes recommended. I also checked out EPEL and that seems to work as well.

http://dmlb2000.homelinux.org/packages/genders-1.18-2.fc18.src.rpm

There's three issues rpmlint still seems to show up with. 

 1. Apparently libgenders.so.0.3.0 calls exit(). This in case flex can't parse the primary genders config file. So I don't blame them for this since the entire library is useless in the condition the config file is wrong.

 2. There's a hardcoded library path in the spec %{_prefix}/lib/genders. This is where the compat libraries live. The automake also has this same hard coded library path and the files put there are old perl scripts. So I'm open for suggestions on what the 'right' thing to do is with these files. Could go out in %{_datadir}/%{name}?

 3. EPEL build keeps complaining about having an unspecified group. However, its not needed when I build it on Fedora. Should I care that much?

Also to the initial review I don't have any 'informal reviews' not sure what they'd be and where I'd get a link for them.
Comment 7 Thomas Spura 2012-04-12 08:00:51 EDT
(In reply to comment #6)
> Updated the URLs above with the changes recommended. I also checked out EPEL
> and that seems to work as well.

Great, looks much better now :)
(A newline between every changelog and it would be fabulous :))

>  1. Apparently libgenders.so.0.3.0 calls exit(). This in case flex can't parse
> the primary genders config file. So I don't blame them for this since the
> entire library is useless in the condition the config file is wrong.

It would be best to report that upstream and suggesting the same like "rpmlint -I":
"""
It is preferred for the library to return an actual error code and let the calling program decide how to handle the situation.
"""

>  2. There's a hardcoded library path in the spec %{_prefix}/lib/genders. This
> is where the compat libraries live. The automake also has this same hard coded
> library path and the files put there are old perl scripts. So I'm open for
> suggestions on what the 'right' thing to do is with these files. Could go out
> in %{_datadir}/%{name}?

arch dependent files need to be in %{_libdir}/%{name}, but these are perl files, so %{_datadir}/%{name} would be correct from packaging point of view.

BUT: It would be best to patch that directly and send it upstream and while looking at this, I found this in genders-1.18/compat/Makefile.am:
"""
# Don't use ${libdir}, the /lib is a legacy path that must be maintained
gendlibdir = ${prefix}/lib/genders
"""

So it would be best, to just leave it in %{_prefix}/lib/genders as that is the forced path by upstream...

>  3. EPEL build keeps complaining about having an unspecified group. However,
> its not needed when I build it on Fedora. Should I care that much?

I'd add the group "Development/Tools" for any non-lib package and to the libraries "Development/Libraries". Tools, because it's a helper tool for cluster administrators and I consider a cluster always as a kind of development going on.

> Also to the initial review I don't have any 'informal reviews' not sure what
> they'd be and where I'd get a link for them.

Once you are sponsored to the packager group you can do reviews like explained here:
http://fedoraproject.org/wiki/Package_Review_Process#Reviewer

But to get sponsored, we need to see, that you are familiar with the guidelines. Here you have 2 possibilities:

* Put several packages for review
* Do 'informal reviews', that means, you can make comments to and improve other pending reviews (preferably one that doesn't require a sponsor, because I'll look to the review after you and can so directly approve it without needing to sponsoring someone immediately).

Once you are sponsored to the packager group, you can directly approve other reviews yourself.

More information about that is here:
http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Feel free to ask, when you have further questions.
(Here or via mail)
Comment 8 David Brown 2012-04-12 17:46:04 EDT
I've updated the genders.spec from the original link and added the groups for epel 5/6 builds.

http://dmlb2000.homelinux.org/packages/genders-1.18-3.fc18.src.rpm

And I'll have to submit more packages of better quality then ;).
Comment 9 David Brown 2012-04-12 18:16:13 EDT
I was also able to get this to build on el5 and el6 as well.

http://dmlb2000.homelinux.org/packages/genders-1.18-3.el5.centos.src.rpm
http://dmlb2000.homelinux.org/packages/genders-1.18-3.el6.src.rpm
Comment 10 Thomas Spura 2012-04-13 06:42:11 EDT
(In reply to comment #8)
> And I'll have to submit more packages of better quality then ;).

Fine. The easiest way is still to do reviews of other packages, which is basically what you did in bug #812161, just a bit more verbosely.
Some have a checklist:
http://fedoraproject.org/wiki/User:Jlaska/Package_Review_Checklist
but writing a list what you checked (and what remains as blocker in the review) manually is fine either.

Submitting more packages is usually more work ;)
Comment 11 Thomas Spura 2012-07-05 15:53:10 EDT
Sorry, this totally fell of my radar... A ping would have been nice :)

REVIEW:

good:
- name ok
- parallel make
- %build ok
- %prep ok, nice python/perl provides scipt
- %install ok
- no *.la *.a
- ldconfig there
- rpmlint /home/tomspur/rpmbuild/SRPMS/genders-1.18-3.fc17.src.rpm /home/tomspur/rpmbuild/RPMS/x86_64/genders-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/genders-compat-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/genders-perl-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/genders-python-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/libgenders-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/libgenders-devel-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/libgendersplusplus-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/libgendersplusplus-devel-1.18-3.fc17.x86_64.rpm /home/tomspur/rpmbuild/RPMS/x86_64/genders-debuginfo-1.18-3.fc17.x86_64.rpm
genders.src: I: enchant-dictionary-not-found en_US
genders.src:113: W: rpm-buildroot-usage %build --with-extension-destdir="%{buildroot}"
genders.src:128: E: hardcoded-library-path in %{_prefix}/lib/genders
genders.src:201: W: macro-in-%changelog %{buildroot}
libgenders.x86_64: W: shared-lib-calls-exit /usr/lib64/libgenders.so.0.3.0 exit@GLIBC_2.2.5
10 packages and 0 specfiles checked; 1 errors, 3 warnings.

Some bad things among them, but they must be, see above... :(

- CFLAGS respected

comments:
- Each library has it's own package, which is a bit too much. But your choice :)
- Each package MUST have a license file, which means, it's fine, when a package Requires an other *%{name}* package, which has a license file.
  But it's fine this way too.

NEEDSWORK:
- Why do you: ?
  mv %{buildroot}/%{_prefix}/lib/genders %{buildroot}/%{_libexecdir}/

  %{_prefix}/lib/genders is unfortunately the best, see comment #7.
- After that, please make the compat package noarch on EL6+ and Fedora. This way, it's verified on each build, that those packages have the same content on each arches, so multilib install is possible.

- It looks like 32bit has some problems. Logs over here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4221409
Comment 12 David Brown 2012-07-05 19:24:24 EDT
I think I've seen this build failure before. I'm wondering if the automake isn't exactly working in parallel correctly. I've removed the parallel build for now since that seemed to work on my system.

I moved the compat stuff back into /usr/lib/genders and added the noarch stuff (I think) to the compat package as well.

http://dmlb2000.homelinux.org/packages/genders-1.18-4.fc17.src.rpm
Comment 13 David Brown 2012-07-06 16:23:18 EDT
I noticed that mock for some reason on my systems chose to make the python shared object 775 instead of 755 so I added a chmod in install.

http://dmlb2000.homelinux.org/packages/genders-1.18-5.fc17.src.rpm
Comment 14 Thomas Spura 2012-07-08 13:53:24 EDT
Please also link to the spec, as that's easier to look/download than the srpm.

The package looks fine:
- koji works:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=4225804

I'll sponsor you to the packager group and this is now:

#######################################

APPROVED

#######################################

Next steps are:
* SCM admin request:
  https://fedoraproject.org/wiki/Package_SCM_admin_requests

* Importing into git:
  https://fedoraproject.org/wiki/Using_Fedora_GIT

#######################################

Looks like there will be a problem with your mail address:
- Your bugzilla address is @pnl.gov
- Your FAS      address is @pnnl.gov

but they both MUST match or you won't have proper bugzilla power.

Please change one of those addresses accordingly.
Comment 15 David Brown 2012-07-09 00:20:16 EDT
New Package SCM Request
=======================
Package Name: genders
Short Description: File based database for cluster management
Owners: dmlb2000
Branches: f16 f17 el5 el6
InitialCC:
Comment 16 Kevin Fenzi 2012-07-10 18:25:45 EDT
Git done (by process-git-requests).
Comment 17 Fedora Update System 2012-07-16 20:52:50 EDT
genders-1.18-6.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/genders-1.18-6.fc17
Comment 18 Fedora Update System 2012-07-16 20:54:24 EDT
genders-1.18-6.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/genders-1.18-6.el6
Comment 19 Fedora Update System 2012-07-16 20:55:20 EDT
genders-1.18-6.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/genders-1.18-6.el5
Comment 20 Fedora Update System 2012-07-16 21:01:46 EDT
genders-1.18-6.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/genders-1.18-6.fc16
Comment 21 Fedora Update System 2012-07-19 05:06:51 EDT
genders-1.18-6.fc17 has been pushed to the Fedora 17 testing repository.
Comment 22 Fedora Update System 2012-07-24 19:30:09 EDT
genders-1.18-6.fc17 has been pushed to the Fedora 17 stable repository.
Comment 23 Fedora Update System 2012-07-24 19:30:30 EDT
genders-1.18-6.fc16 has been pushed to the Fedora 16 stable repository.
Comment 24 Fedora Update System 2012-07-25 16:03:43 EDT
genders-1.18-6.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 25 Fedora Update System 2012-07-25 16:04:01 EDT
genders-1.18-6.el5 has been pushed to the Fedora EPEL 5 stable repository.
Comment 26 David Brown 2014-07-07 23:47:54 EDT
Package Change Request
======================
Package Name: genders
New Branches: el7
Owners: dmlb2000

I'd like to start testing el7

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