Bug 387261 - Review Request: libcmpiutil - Utility library for CIM providers
Review Request: libcmpiutil - Utility library for CIM providers
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2007-11-16 12:01 EST by Dan Smith
Modified: 2007-11-30 21:32 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-11-30 21:32:08 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Dan Smith 2007-11-16 12:01:55 EST
Spec URL: http://static.danplanet.com/pkg/fedora-8/libcmpiutil.spec
SRPM URL: http://static.danplanet.com/pkg/fedora-8/libcmpiutil-0.1-1.fc8.src.rpm
Description: This is a utility library for writing CIM providers.  It will be directly used by the (upcoming) libvirt-cim package.

This is my first package and I need a sponsor.
Comment 1 Jason Tibbitts 2007-11-16 13:51:29 EST
Just some quick comments:

Builds OK; rpmlint says:

  libcmpiutil.x86_64: W: invalid-license LGPL
You must specify LGPL version; see http://fedoraproject.org/wiki/Licensing.

  libcmpiutil.x86_64: W: one-line-command-in-%post /sbin/ldconfig
  libcmpiutil.x86_64: W: one-line-command-in-%postun /sbin/ldconfig
Best to use %post -p /sbin/ldconfig

  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm    
  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm 
  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm 
  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm 
  libcmpiutil-debuginfo.x86_64: E: script-without-shebang 
For some reason, your source files are executable.  Best to make them not
executable with a quick line in %prep.

  libcmpiutil-devel.x86_64: E: standard-dir-owned-by-package /usr/include
Definitely not OK for a package to own /usr/include.  You probably just want
in your %files section.

  libcmpiutil-devel.x86_64: W: no-dependency-on libcmpiutil
The -devel subpackage needs to require the main package.  Just add
  Requires: %{name} = %{version}-%{release}

Some other bits:

Why no parallel make?  (Well, it is a small package, but still...)

Dn't use %makeinstall; see http://fedoraproject.org/wiki/Packaging/Guidelines
and search for "Why the %makeinstall macro should not be used".

You should be consistent in your usage of %{buildroot} or $RPM_BUILD_ROOT.  You
can use whichever you like, but pick one and stick with it.

There's no reason for the -devel package to include the same documentation files
as the main package.
Comment 2 Dan Smith 2007-11-16 14:30:22 EST
Thanks for the tips, I think I have fixed them all.  I didn't realize I was
supposed to run rpmlint on the actual RPMs, so I missed those errors the first
time through.

I'm not sure if I am supposed to bump any of the version numbers between
reviews, so for now, I have left them the same.  As such, the previous set of
spec and srpm urls point to the updated packages.

Comment 3 Mamoru TASAKA 2007-11-20 05:58:30 EST
Please bump release number every time you modify your spec file
(when version number does not change) from next time.

By the way:
rebuild fails on dist-f9, at least on i386.
Comment 4 Dan Smith 2007-11-20 12:32:24 EST
Okay, I'm not able to reproduce the error on my fedora-8 machine, and since the
tog-pegasus package hasn't changed in fedora-9 (as far as I can tell).  However,
I took an educated guess about what the problem might be and re-rolled a new
RPM.  If someone could test build it in the dist-f9 environment, I'd appreciate
it.  I'll try to get a rawhide build installed to reproduce locally in case this
doesn't fix the build.

SRPM: http://static.danplanet.com/pkg/fedora-8/libcmpiutil-0.1-2.fc8.src.rpm
SPEC: http://static.danplanet.com/pkg/fedora-8/libcmpiutil.spec
Comment 5 Jason Tibbitts 2007-11-20 12:35:53 EST
Scratch build in progress:
Comment 7 Daniel Veillard 2007-11-20 17:29:41 EST
Launched a new scratch build for 0.1-4.fc8 (should be renamed .fc9 now BTW :-)
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=251895
Comment 8 Dan Smith 2007-11-20 17:40:37 EST
Here is a .fc9 build:

Comment 9 Daniel Veillard 2007-11-20 18:52:22 EST
restarted as http://koji.fedoraproject.org/koji/taskinfo?taskID=252186

this one seems to go through,

Comment 10 Mamoru TASAKA 2007-11-21 02:45:43 EST
It seems that you changed the tarball directly. Can we assume that
this 0.1 version tarball have not released formally?

We require that the Source must be given with full URL and the
tarball in srpm must coincide with what is written as source URL.
Comment 11 Daniel Veillard 2007-11-21 05:03:37 EST
Mamoru, you are right we didn't yet made a formal release of libcmpiutil,
but I think there is nothing preventing it at this point. Assuming we make
this release is there anything else which would block approving the
package for Fedora ?


Comment 12 Mamoru TASAKA 2007-11-21 06:26:47 EST
(In reply to comment #11)
> Mamoru, you are right we didn't yet made a formal release of libcmpiutil,
> but I think there is nothing preventing it at this point.

Yes, I just wanted to confirm.

For 1.0-4:

* Permission of the files in srpm
  - misc issue, however please change the permission of
    the tarball and spec file in srpm to 0644.

* configure option
  - Please check if this configure accepts --disable-static

* Groups
  - Usually the rpm of "libfoo" has "System Environment/Libraries"
    group for main package and "Development/Libraries" for
    -devel package.

* timestamps
  - Please add 'INSTALL="install -p"' option to 'make install'
    to keep timestamps on installed files.
    While sometimes this does not work, this method usually works
    for recent Makefiles (and for this package).

* %defattr
  - We now recommend %defattr(-,root,root,-)

* Documentation
  - Please add COPYING

And as this is NEEDSPONSOR ticket:
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
(NOTE: please don't choose "Merge Review")

Review guidelines are described mainly on:
Comment 13 Dan Smith 2007-11-21 10:30:42 EST
Updated to address almost everything:

SPEC: http://static.danplanet.com/pkg/fedora-9/libcmpiutil.spec
SRPM: http://static.danplanet.com/pkg/fedora-9/libcmpiutil-0.1-5.fc9.src.rpm

I'm confused about why rpmlint still says the spec file has 0600 permissions. 
In the tarball I build the RPMs from, it is 0600.  I will continue trying to
figure out the solution, but since it's minor, I thought I'd post an updated
version first.
Comment 14 Mamoru TASAKA 2007-11-22 05:37:56 EST
I will comment on -5 if any issue is left.
Anyway I will wait for your pre-review or another review request
(my comment 12)
Comment 15 Dan Smith 2007-11-22 11:15:52 EST
Left pre-review feedback for 394651.  I will find a couple more to do.
Comment 16 Mamoru TASAKA 2007-11-23 06:03:22 EST

* This package itself is good
  Note: Please put a formal release tarball somewhere and
        write the full URL in spec file as Source.

* Your pre-review comments seems good for initial comments

     This package (libcmpiutil) is APPROVED by me

Please follow the procedure according to:
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor (at the stage, please also write on
this bug for confirmation that you requested for sponsorship)
Then I will sponsor you.

If you want to import this package into Fedora 7/8, you also have
to look at
(after once you rebuilt this package on Fedora rebuilding system).

If you have questions, please ask me.
Comment 17 Dan Smith 2007-11-23 11:52:21 EST
My Fedora account username is `danms'.

Comment 18 Mamoru TASAKA 2007-11-23 12:03:49 EST
Now I should be sponsoring you.
Comment 19 Mamoru TASAKA 2007-11-29 02:34:09 EST
Please follow the procedure written in
Comment 20 Dan Smith 2007-11-29 08:46:51 EST
We are in the process of preparing a formal release source tarball.  I had
planned to continue the packaging process after we had done so.  Should have
that done today or tomorrow, if that's okay.
Comment 21 Dan Smith 2007-11-30 13:30:35 EST
New Package CVS Request
Package Name: libcmpiutil
Short Description: Utility library for CIM providers
Owners: danms
Cvsextras Commits: No
Comment 22 Kevin Fenzi 2007-11-30 14:22:53 EST
cvs done.
Comment 23 Dan Smith 2007-11-30 21:32:08 EST
Successful build:


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