Bug 509445
Summary: | Review Request: sblim-cmpi-rpm - CIM access to rpm and other information about installed packages | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Praveen K Paladugu <praveen_paladugu> |
Component: | Package Review | Assignee: | Martin Gieseking <martin.gieseking> |
Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, martin.gieseking, matt_domsch, notting |
Target Milestone: | --- | Flags: | martin.gieseking:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-09-17 16:18:31 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 468400 |
Description
Praveen K Paladugu
2009-07-02 19:56:54 UTC
The Link for the source rpm is: http://linux.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm-1.0.1-1.fc10.src.rpm It was a typo.... Your account, praveenp, is not in the packager group, so I've indicated that you require a sponsor. Hi Praveen, This is not an official review as am trying to become a package maintainer myself. Things to change I would say: 1) $ rpmlint sblim-cmpi-rpm.spec sblim-cmpi-rpm.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 2) 2) There should be an initial changelog entry. See http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs 3) The build fails with checking for CMPI headers... checking standard... checking /usr/include/cmpi... checking /usr/local/include/cmpi... checking /src/Pegasus/Provider/CMPI... checking /opt/tog-pegasus/include/Pegasus/Provider/CMPI... checking /usr/include/Pegasus/Provider/CMPI... checking /usr/include/openwbem... checking /usr/sniacimom/include... configure: error: no. Sorry cannot find CMPI headers files. so I must be missing a BuildRequires I guess. Steve Steve, Made the require changes. Please verify now. Thank you Praveen Hi Praveen, Slow reply, sorry. It looks to be the same, I don't still have the old one lying around to check if it is. checking for CMPI headers... checking standard... checking /usr/include/cmpi... checking /usr/local/include/cmpi... checking /src/Pegasus/Provider/CMPI... checking /opt/tog-pegasus/include/Pegasus/Provider/CMPI... checking /usr/include/Pegasus/Provider/CMPI... checking /usr/include/openwbem... checking /usr/sniacimom/include... configure: error: no. Sorry cannot find CMPI headers files. error: Bad exit status from /var/tmp/rpm-tmp.r6y1BD (%build) $ md5sum /tmp/sblim-cmpi-rpm-1.0.1-1.fc10.src.rpm cc21850d37fe2ba4d62cd259add38ae6 /tmp/sblim-cmpi-rpm-1.0.1-1.fc10.src.rpm When you make a new rpm can you increase the Release number at least to avoid confusion. Thanks, Steve http://linux.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm-1.0.1-2.fc10.src.rpm http://linux.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm.spec I built the above code on mock, It seems I missed a dependency before. I changed the version number as you mentioned. Thanks Praveen Praveen, just a few annotations: :) - Source0 must be http://downloads.sourceforge.net/sblim/%{name}-%{version}.tar.bz2 (see https://fedoraproject.org/wiki/Packaging/SourceURL) - remove INSTALL from %doc - add a changelog entry for each revision $ rpmlint sblim-cmpi-rpm-1.0.1-2.fc10.src.rpm sblim-cmpi-rpm.src: E: description-line-too-long These CIM providers list the software packages installed on GNU/Linux system and provide some detail about them sblim-cmpi-rpm.src: E: no-changelogname-tag sblim-cmpi-rpm.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 2) 1 packages and 0 specfiles checked; 2 errors, 1 warnings. Martin Martin, Updated the files. Final links at: http://linux.us.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm-1.0.1-3.fc10.src.rpm http://linux.us.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm.spec Thanks Praveen Hello Praveen, here is my review of your package. :) rpmlint output: sblim-cmpi-rpm.i586: E: devel-dependency rpm-devel sblim-cmpi-rpm.i586: W: incoherent-version-in-changelog .0.1-3 ['1.0.1-3.fc11', '1.0.1-3'] sblim-cmpi-rpm.i586: E: library-without-ldconfig-postin /usr/lib/libcimrpm.so.0.0.0 sblim-cmpi-rpm.i586: E: library-without-ldconfig-postun /usr/lib/libcimrpm.so.0.0.0 sblim-cmpi-rpm.i586: E: library-without-ldconfig-postin /usr/lib/libcimrpmv4.so.0.0.0 sblim-cmpi-rpm.i586: E: library-without-ldconfig-postun /usr/lib/libcimrpmv4.so.0.0.0 sblim-cmpi-rpm-devel.i586: W: no-dependency-on sblim-cmpi-rpm/sblim-cmpi-rpm-libs/libsblim-cmpi-rpm sblim-cmpi-rpm-devel.i586: W: summary-not-capitalized devel files for sblim-cmpi-rpm 4 packages and 0 specfiles checked; 5 errors, 3 warnings. - Add a changelog entry for *every* revision so that the revision history can be reproduced (revision number is 3 now, so there must be 3 changelog entries, newest first) - It wasn't necessary to shorten the description. The lines were just longer than 80 characters. Split long descriptions to several lines. --------------------------------- keys used in following checklist: [+] OK [#] OK, not applicable [-] needs work --------------------------------- [+] MUST: The package must be named according to the Package Naming Guidelines. [+] MUST: The spec file name must match the base package %{name} [-] MUST: The package must meet the Packaging Guidelines. - see rpmlint output [+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. [+] MUST: The License field in the package spec file must match the actual license. [+] MUST: license file added to %doc [+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [-] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. - md5 hashes are different - please use the latest orignal tarball from upstream [+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. [+] MUST: All build dependencies must be listed in BuildRequires [#] MUST: The spec file MUST handle locales properly. - no locales [-] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. - add %post and %postun scriptlets - see https://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries [#] MUST: If the package is designed to be relocatable,... - package not relocatable [+] MUST: A package must own all directories that it creates. [+] MUST: A Fedora package must not list a file more than once in the spec file's %files listings. [+] MUST: Permissions on files must be set properly. [+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: Each package must consistently use macros. [+] MUST: The package must contain code, or permissable content. [#] MUST: Large documentation files must go in a -doc subpackage. - no large documentation [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [+] MUST: Header files must be in a -devel package. [-] MUST: Static libraries must be in a -static package. - run configure with --disable-static to disable build of static libraries - see https://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries [#] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. - no pkgconfig files [+] MUST: .so (without suffix) must go in a -devel package. [-] MUST: devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} - add Requires: %{name} = %{version}-%{release} to the -devel package [-] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. - remove the .la files in the %install section and remove them from %files devel [#] MUST: Packages containing GUI applications must include a %{name}.desktop file - no GUI [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: All filenames in rpm packages must be valid UTF-8. [+] SHOULD: The reviewer should test that the package builds in mock. - builds in mock [-] SHOULD: If scriptlets are used, those scriptlets must be sane. - no scriptlets - add %post and %postun (see above) With the latest sources, had to add a patch to add libd.a among the libraries while linking. Without this there were some build failures because of the dlopen other dlxxxx function definitions missing. http://linux.us.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm-1.0.1-3.fc10.src.rpm http://linux.us.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm.spec I added all the above changes suggested. Please verify. Thank you Praveen Of course, you may apply patches to the sources but you must not change the tarball from upstream. Otherwise it's hard to check its validity. Let rpmbuild do the work for you. Place the patch file into the SOURCES folder of your rpmbuild directory and add a Patch0 and a %patch0 entry to your spec file (this already seems to be done). rpmbuild will add the patch file to the SRPM and will apply it when creating the binary RPM. So again, don't change the tarball (or other upstream files) directly. Some minor things you should fix: - Add a short comment above Patch0 on what the patch does - change the two lines rm -rf $RPM_BUILD_ROOT/%{_libdir}/cmpi/*.la rm -rf $RPM_BUILD_ROOT/%{_libdir}/*.la into these: rm -f $RPM_BUILD_ROOT/%{_libdir}/cmpi/*.la rm -f $RPM_BUILD_ROOT/%{_libdir}/*.la (the -r option is not necessary because you want to delete files here) - The description text is a bit unclear. At least, I didn't understand what the package contains before reading the info on the upstream website. Please use the complete text from upstream: These providers list the software packages (currently for RPM) installed on GNU/Linux systems and provide some detail information about them. Since the text is longer than 80 characters, split it to several lines. - in the changelog, add a blank between ">" and "-": Mon Jul 13 2009 Praveen K Paladugu <praveen_paladugu> - 1.0.1-2 Thu Jul 02 2009 Praveen K Paladugu <praveen_paladugu> - 1.0.1-1 (see http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs) Hey Martin, The upstream source files are intact. I never touched the sources. I added a patch file by the name "sblim_cmpi_rpm_ldl_library.patch" and patched the sources. The md5sum of the tar.bz2 file matches that of the upstream sources at : http://downloads.sourceforge.net/project/sblim/sblim-cmpi-rpm/1.0.1/sblim-cmpi-rpm-1.0.1.tar.bz2 I also made the other suggested changes: http://linux.us.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm-1.0.1-4.fc10.src.rpm http://linux.us.dell.com/files/fedora/sblim-cmpi-rpm/sblim-cmpi-rpm.spec Thank you Praveen Hello Praveen, thanks for the fixes. Now, the md5sums match (5a6ebb04abe884b51416054e97777d64). The tarball in your previous SRPM sblim-cmpi-rpm-1.0.1-3.fc10.src.rpm had a different hash value, and there was no patch file included with the SRPM: $ rpm -qlp sblim-cmpi-rpm-1.0.1-3.fc10.src.rpm sblim-cmpi-rpm-1.0.1.tar.bz2 sblim-cmpi-rpm.spec $ md5sum sblim-cmpi-rpm-1.0.1.tar.bz2 e1130e6a170f40e84e1a87d2cf41af8a sblim-cmpi-rpm-1.0.1.tar.bz2 But now it's fixed. :) Everything seems to be clean now. You should check whether the build also works with make %{?_smp_mflags} If not, you can leave it as it is, and add a comment before the make statement that the smp flags break the builds. Besides of that, I think we can finish here. The package is APPROVED. Hey Martin, I verified that "make %{?_smp_mflags}" doesn't work. So, I will leave it as is. Thanks for your time. Praveen New Package CVS Request ======================= Package Name: sblim-cmpi-rpm Short Description: CIM access to installed software packages Owners: praveenp Branches: F-10 F-11 EL-4 EL-5 InitialCC: praveenp mdomsch cvs done. Was someone going to build this? I tried to build it once and there seem to be some function definitions missing. I checked today and there are no updated sources available. So, the build still fails. If you want to build this package, please go ahead. Praveen You're the maintainer, I don't care one way or another whether it's built. I just thought it was odd that the package has never been built. The packages are on git and the builds are fine. So closing this issue. |