Bug 458024 - Review Request: sblim-sfcc - Small Footprint CIM Client Library
Review Request: sblim-sfcc - Small Footprint CIM Client Library
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matt Domsch
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 458012 468400
  Show dependency treegraph
 
Reported: 2008-08-06 02:50 EDT by srinivas
Modified: 2009-09-23 18:26 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-13 12:18:15 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
matt_domsch: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
build.log (128.15 KB, text/plain)
2008-08-06 16:59 EDT, Matt Domsch
no flags Details

  None (edit)
Description srinivas 2008-08-06 02:50:26 EDT
Spec URL: http://linux.dell.com/files/fedora/sblim-sfcc/sfcc.spec
SRPM URL: http://linux.dell.com/files/fedora/sblim-sfcc/
SRPM: sblim-sfcc-2.1.0-0.src.rpm

Description: 
Hello! I have finished packaging sblim-sfcc and I would appreciate a review so that I can get it into Fedora Extras.

Small Footprint CIM Broker(SFCB) is a lightweight CIM daemon that responds to CIM client requests for system management data and/or performs system management tasks. sfcb supports most of the standard CIM XML over http/https protocol.It is highly modular, allowing functionality to be easily added, removed or customized for different management applications.
Comment 1 Matt Domsch 2008-08-06 15:40:56 EDT
building on rawhide, rpmlint throws:

sblim-sfcc.src: E: no-spec-file
sblim-sfcc.src: W: non-standard-group Systems Management/Base
sblim-sfcc.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libcimcClientXML.so
sblim-sfcc.x86_64: E: standard-dir-owned-by-package /usr/share/doc
sblim-sfcc.x86_64: W: non-standard-group Systems Management/Base
sblim-sfcc.x86_64: W: incoherent-version-in-changelog 2.0.0-0 2.1.0-0
sblim-sfcc-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/sblim-sfcc-2.1.0/backend/cimxml/indicationlistener.c
sblim-sfcc-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/sblim-sfcc-2.1.0/backend/cimxml/nativeCimXml.h
sblim-sfcc-devel.x86_64: E: standard-dir-owned-by-package /usr/share/man/man3
sblim-sfcc-devel.x86_64: E: standard-dir-owned-by-package /usr/share/man
sblim-sfcc-devel.x86_64: E: standard-dir-owned-by-package /usr/include
sblim-sfcc-devel.x86_64: W: non-standard-group Systems Management/Base
4 packages and 0 specfiles checked; 5 errors, 7 warnings.
Comment 2 Matt Domsch 2008-08-06 16:33:50 EDT
Review:

Fix all rpmlint errors/warnings.
sblim-sfcc.src: E: no-spec-file
sblim-sfcc.src: W: non-standard-group Systems Management/Base
sblim-sfcc.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libcimcClientXML.so
sblim-sfcc.x86_64: E: standard-dir-owned-by-package /usr/share/doc
sblim-sfcc.x86_64: W: non-standard-group Systems Management/Base
sblim-sfcc.x86_64: W: incoherent-version-in-changelog 2.0.0-0 2.1.0-0
sblim-sfcc-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/sblim-sfcc-2.1.0/backend/cimxml/indicationlistener.c
sblim-sfcc-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/sblim-sfcc-2.1.0/backend/cimxml/nativeCimXml.h
sblim-sfcc-devel.x86_64: E: standard-dir-owned-by-package /usr/share/man/man3
sblim-sfcc-devel.x86_64: E: standard-dir-owned-by-package /usr/share/man
sblim-sfcc-devel.x86_64: E: standard-dir-owned-by-package /usr/include
sblim-sfcc-devel.x86_64: W: non-standard-group Systems Management/Base
4 packages and 0 specfiles checked; 5 errors, 7 warnings.


Naming: ok
spec file name: wrong.  Should be sblim-sfcc.spec
version: ok
release: consider adding %{?dist}  tag
license: EPL OK
Source URL: not ok, see https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

no prebuild binaries: ok
BuildRoot: not ok, see https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

Requires: curl  should get picked up automatically by the dependency processor, don't add it here.

BuildRequires: don't list gcc-c++.

no packager, vendor, copyright, tags: ok
summary and description tags: ok

%setup shouldn't need those extra args
remove the export PATCH_GET
and commented #%patch0

%build make doesn't use the macro for invoking -j for parallel make.  Please fix.

%install missing mandatory initial cleanup.  Should be:
%install
rm -rf %{buildroot}
%install doesn't need the extra paranoia checking.

%clean doesn't need the extra paranoia checking.

no rpaths: ok
no config files: ok
no initscripts: ok
no desktop files: ok
consistent use of macros: ok
no makeinstall: ok
no lang files: ok
scriptlets: ok
no conditional deps: ok
builds with a normal user account: ok
not relocatable: ok
code, not content: ok
directory ownership: not ok.  package must not own %{_includedir} or %{_mandir} itself.
users and groups: ok
not a web app: ok
conflicts: ok
no kmods: ok
no files under /srv: ok
license file not in %doc: must fix
english: ok
legible: ok
source matches: ok
package builds on x86_64 at least: ok
calls ldconfig appropriately: ok
no duplicate files: ok
file permissions: mostly ok, see rpmlint
headers in -devel: ok
no pkgconfig file (should it have one??)
devel package has fully versioned dependency on lib package: no - pls fix
libtool archives removed: ok


Please correct these items and re-submit.

Thanks,
Matt
Comment 3 Matt Domsch 2008-08-06 16:48:23 EDT
while you're at it, check out the compiler warnings thrown in build.log.  It looks like some header files are missing which would make for a good patch to send upstream.  Lots of implicit function definitions for stdlib functions.
Comment 4 Matt Domsch 2008-08-06 16:59:22 EDT
Created attachment 313642 [details]
build.log
Comment 5 srinivas 2008-08-14 08:43:59 EDT
Hello Matt,

I have incorporated the changes as per your suggestion.
Please find the updated spec file and the SRPM at the following location:

http://linux.dell.com/files/fedora/sblim-sfcc/sblim-sfcc.spec
http://linux.dell.com/files/fedora/sblim-sfcc/
SRPM: sblim-sfcc-2.1.0-0.src.rpm


Thanks
Srinivas
Comment 6 Matt Domsch 2008-08-26 11:19:07 EDT
Srinivas and I have gone back and forth a few times privately, and he's getting the hang of it. :-)

Trivial changes yet which can be made at checkin time.  Note the preferred value of BuildRoot I use, and since you are using the standard %setup macro now, you don't need to pass it %{name}-%{version}.

With these changes, I approve.
-Matt

--- sblim-sfcc.spec     Tue Aug 19 09:50:30 2008
+++ /home/mdomsch/sblim-sfcc.spec       Tue Aug 26 10:13:44 2008
@@ -4,7 +4,7 @@
 # Package spec for sblim-sfcc
 #

-BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
+BuildRoot: %(mktemp -ud 
+%{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

 Summary: Small Footprint CIM Client Library
 Name: sblim-sfcc
@@ -30,7 +30,7 @@ Small Footprint CIM Client Library Heade

 %prep

-%setup -q  %{name}-%{version}
+%setup -q

 %build
 chmod a-x backend/cimxml/*.[ch]
Comment 7 srinivas 2008-08-27 05:23:06 EDT
The suggested changes have been implemented.
The updated spec and SRPM could be found at the following location:

http://linux.dell.com/files/fedora/sblim-sfcc/

Thanks
Srinivas.
Comment 8 Matt Domsch 2008-08-27 10:03:04 EDT
One more: the -devel package needs to use a fully versioned requires, not a >=.

Requires: %{name} = %{version}-%{release}
Comment 9 srinivas 2008-08-27 10:13:11 EDT
Done.
Please let me know if there are any other changes required.

Thanks
Srinivas.
Comment 10 Matt Domsch 2008-08-27 10:30:08 EDT
Pretty close: formal review below, noting a few minor changes to make.  Do these then do the CVS requests.

rpmlint: 100% clean, ok
naming: ok
spec file name matches: ok
packaging guidelines: ok
license = EPL: ok
license tag: ok

- COPYING file not included in %doc for main package, only in -devel.  Must fix.

spec in english: ok
spec legible: ok

- Source0 URL incorrect, use
  http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.bz2

complies on i386 and x86_64: ok
all BRs correct: ok
spec doesn't use locales: ok
properly uses ldconfig in scriptlets: ok
not relocatable: ok
dir ownership: ok
no duplicate files: ok
file perms correct: ok
%clean section: ok
consistent use of macros: ok
code, not content: ok
no large docs, no need for -doc subpackage: ok
%doc usage ok (except see above)
headers in -devel: ok
no static libs: ok
no pkgconfig files: ok
.so in -devel: ok

- -devel needs to include fully versioned dependency.
   Requires: %{name} = %{version}-%{release}

all libtool .la files removed: ok
no GUI, no .desktop files: ok
dir ownership correct: ok
%install does rm: ok
filenames UTF8: ok

shoulds:
source includes license: ok
not translated .spec: ok
builds in mock: ok
builds on i386 and x86_64: ok
package installs fine, is a library.  will be tested when dependent
    apps use it (also under review).
scriptlets sane: ok
no other subpackages: ok
no pkgconfig: ok
no extra deps: ok
Comment 11 srinivas 2008-08-28 12:27:45 EDT
New Package CVS Request
=======================
Package Name: sblim-sfcc
Short Description:Small Footprint CIM Client Library Runtime Libraries
Owners: srini
Branches: F-8 F-9 EL-4 EL-5 OLPC-2 OLPC-3
InitialCC:mdomsch
Comment 12 Kevin Fenzi 2008-08-29 00:44:19 EDT
Do you really want OLPC-2 and OLPC-3 branches here? Note that OLPC-3 is using F9 as a base, so unless you package has OLPC specific changes the F9 package can be used there.
Comment 13 srinivas 2008-08-29 03:13:36 EDT
You can remove the OLPC-2 and OLPC-3 branches. The package does not have anything specific to OLPC.
Comment 14 Kevin Fenzi 2008-08-30 16:54:29 EDT
cvs done.
Comment 15 Matt Domsch 2008-11-13 12:18:15 EST
built on all branches.  Closing.
Comment 16 Praveen K Paladugu 2009-09-09 10:18:36 EDT
New Package CVS Request
=======================
Package Name: sblim-sfcc
Short Description:Small Footprint CIM Client Library Runtime Libraries
Owners: srini praveenp
Branches: EPEL4 EPEL5
InitialCC: mdomsch
Comment 17 Praveen K Paladugu 2009-09-09 10:28:12 EDT
New Package CVS Request
=======================
Package Name: sblim-sfcc
Short Description:Small Footprint CIM Client Library Runtime Libraries
Owners: srini praveenp
Branches: F-10 F-11 EPEL-4 EPEL-5
InitialCC: mdomsch
Comment 18 Kevin Fenzi 2009-09-09 12:26:11 EDT
cvs done.
Comment 19 Praveen K Paladugu 2009-09-23 18:26:42 EDT
New Package CVS Request
=======================
Package Name: sblim-sfcc
Short Description:Small Footprint CIM Client Library Runtime Libraries
Owners: srini praveenp
Branches: F-12
InitialCC: mdomsch

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