Bug 459828

Summary: Review Request: ql2500-firmware - Firmware for qlogic 2500 devices
Product: [Fedora] Fedora Reporter: Tom "spot" Callaway <tcallawa>
Component: Package ReviewAssignee: Peter Lemenkov <lemenkov>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, lemenkov, notting
Target Milestone: ---Flags: lemenkov: fedora-review+
tcallawa: 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: 2008-09-12 19:55:57 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:

Description Tom "spot" Callaway 2008-08-22 18:41:17 UTC
Spec URL: http://www.auroralinux.org/people/spot/review/new/ql2500-firmware.spec
SRPM URL: http://www.auroralinux.org/people/spot/review/new/ql2500-firmware-4.04.04-1.fc10.src.rpm
Description: 
Firmware for qlogic 2500 devices.

Comment 1 Peter Lemenkov 2008-08-23 14:49:43 UTC
I'll review it.

Comment 2 Peter Lemenkov 2008-08-23 15:09:43 UTC
REVIEW:

MUST Items:

+ rpmlint is silent.
+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines .
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ The text of the license(s) is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package must match the upstream source:

[petro@Sulaco SOURCES]$ md5sum LICENSE* ql2500_fw.bin*
4005328a134054f0fa077bdc37aa64f2  LICENSE
4005328a134054f0fa077bdc37aa64f2  LICENSE.orig
e0eee74a60109539f1ea39c89ef05a92  ql2500_fw.bin
da9804a0f6c216ea79355ee18fbd3d95  ql2500_fw.bin_mid
da9804a0f6c216ea79355ee18fbd3d95  ql2500_fw.bin_mid.orig
e0eee74a60109539f1ea39c89ef05a92  ql2500_fw.bin.orig
[petro@Sulaco SOURCES]$ 

+ The package successfully compiles and builds into binary rpms on at least one supported architecture (ppc).

- MUST: All build dependencies must be listed in BuildRequires.

There is something, that I can't understand - this package requires /lib/firmware, and "rpm -qf /lib/firmware" shows that that's a udev's "Provides:". I don't know whethet this firmware should require udev, so I dont know whether this is a blocker.On the other hand there atre the following two rules:

- MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. Refer to the Guidelines for examples.

- MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time.

+ A package does not contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package must have a %clean section, which contains rm -rf or $RPM_BUILD_ROOT.
+ The package consistently uses macros, as described in the macros section of Packaging Guidelines .
+ The package contains permissable content.
+ Everything, the package includes as %doc, does not affect the runtime of the application.

+ At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.
+ All filenames in rpm packages must be valid UTF-8.

And some remarks:

* You should use %{_lib} instead of /lib 
* You should invoke 'install' command with -p key in order to preserve timestamp.


Please resolve (or explain me, if I misunderstood something) these issues, and I'll approve it.

Comment 3 Peter Lemenkov 2008-08-23 15:21:53 UTC
Hmm.
Seems that I did misunderstood someting - according tho this

http://wwoods.fedorapeople.org/review/bluez-firmware.spec

use /lib instead of the _lib macro (which could expand to
  /lib64 on some arches, which would be the wrong place)

Are there any Firmware Packaging Guidelines?

Comment 4 Peter Lemenkov 2008-09-09 17:59:21 UTC
OK, since firmware should e installed in /lib explicitly, my first remark should be ignored. The next one (according timestamp preservation) doesn't carry much sense in case of installation of blobs, so finally


APPROVED.

Comment 5 Tom "spot" Callaway 2008-09-12 19:36:00 UTC
New Package CVS Request
=======================
Package Name: ql2500-firmware
Short Description: Firmware for qlogic 2500 devices
Owners: spot
Branches: F-8 F-9 devel
InitialCC:

... and it's done.

Comment 6 Tom "spot" Callaway 2008-09-12 19:55:57 UTC
Built in F-8, F-9, rawhide.

Comment 7 Fedora Update System 2008-09-12 20:07:14 UTC
ql2500-firmware-4.04.04-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/ql2500-firmware-4.04.04-1.fc9

Comment 8 Fedora Update System 2008-09-12 20:07:17 UTC
ql2500-firmware-4.04.04-1.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/ql2500-firmware-4.04.04-1.fc8

Comment 9 Fedora Update System 2008-09-14 06:48:37 UTC
ql2500-firmware-4.04.04-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 10 Fedora Update System 2008-09-14 06:52:32 UTC
ql2500-firmware-4.04.04-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.