Bug 387261 - Review Request: libcmpiutil - Utility library for CIM providers
Summary: Review Request: libcmpiutil - Utility library for CIM providers
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-11-16 17:01 UTC by Dan Smith
Modified: 2007-12-01 02:32 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-12-01 02:32:08 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Dan Smith 2007-11-16 17:01:55 UTC
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 18:51:29 UTC
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    
   /usr/src/debug/libcmpiutil-0.1/eo_parser.c
  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm 
   /usr/src/debug/libcmpiutil-0.1/libcmpiutil.h
  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm 
   /usr/src/debug/libcmpiutil-0.1/std_indication.c
  libcmpiutil-debuginfo.x86_64: W: spurious-executable-perm 
   /usr/src/debug/libcmpiutil-0.1/instance_util.c
  libcmpiutil-debuginfo.x86_64: E: script-without-shebang 
   /usr/src/debug/libcmpiutil-0.1/eo_util_parser.y
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
  %{_includedir}/libcmpiutil/
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 19:30:22 UTC
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.

Thanks!

Comment 3 Mamoru TASAKA 2007-11-20 10:58:30 UTC
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.
http://koji.fedoraproject.org/koji/taskinfo?taskID=249927

Comment 4 Dan Smith 2007-11-20 17:32:24 UTC
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 17:35:53 UTC
Scratch build in progress:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=250889

Comment 7 Daniel Veillard 2007-11-20 22:29:41 UTC
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 22:40:37 UTC
Here is a .fc9 build:

http://static.danplanet.com/pkg/fedora-9/libcmpiutil-0.1-4.fc9.src.rpm

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

this one seems to go through,

Daniel

Comment 10 Mamoru TASAKA 2007-11-21 07:45:43 UTC
Dan,
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 10:03:37 UTC
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 ?

  thanks,

Daniel

Comment 12 Mamoru TASAKA 2007-11-21 11:26:47 UTC
(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
    option.

* 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 :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

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:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------


Comment 13 Dan Smith 2007-11-21 15:30:42 UTC
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 10:37:56 UTC
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 16:15:52 UTC
Left pre-review feedback for 394651.  I will find a couple more to do.

Comment 16 Mamoru TASAKA 2007-11-23 11:03:22 UTC
Well,

* 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:
http://fedoraproject.org/wiki/PackageMaintainers/Join
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
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on Fedora rebuilding system).

If you have questions, please ask me.


Comment 17 Dan Smith 2007-11-23 16:52:21 UTC
My Fedora account username is `danms'.

Thanks!

Comment 18 Mamoru TASAKA 2007-11-23 17:03:49 UTC
Now I should be sponsoring you.

Comment 19 Mamoru TASAKA 2007-11-29 07:34:09 UTC
Please follow the procedure written in
http://fedoraproject.org/wiki/PackageMaintainers/Join
again.

Comment 20 Dan Smith 2007-11-29 13:46:51 UTC
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 18:30:35 UTC
New Package CVS Request
=======================
Package Name: libcmpiutil
Short Description: Utility library for CIM providers
Owners: danms
Branches: 
InitialCC: 
Cvsextras Commits: No

Comment 22 Kevin Fenzi 2007-11-30 19:22:53 UTC
cvs done.

Comment 23 Dan Smith 2007-12-01 02:32:08 UTC
Successful build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=267904


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