Bug 516312 - Review Request: ueagle-atm4-firmware - Firmware for USB ADSL Modems based on Eagle IV Chipset
Summary: Review Request: ueagle-atm4-firmware - Firmware for USB ADSL Modems based on ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 537526
TreeView+ depends on / blocked
 
Reported: 2009-08-08 00:30 UTC by Hicham HAOUARI
Modified: 2010-03-11 07:21 UTC (History)
4 users (show)

Fixed In Version: ueagle-atm4-firmware-1.0-4.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-11 07:18:28 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Hicham HAOUARI 2009-08-08 00:30:13 UTC
Spec URL: http://hicham.fedorapeople.org/ueagle-atm-firmware4.spec
SRPM URL: http://hicham.fedorapeople.org/ueagle-atm-firmware4-1.0-1.fc11.src.rpm
Description: Firmwares for usb adsl modems based on chipsets Eagle IV chipset

rpmlint output : none

Comment 1 Hicham HAOUARI 2009-09-04 17:42:30 UTC
ping ?

Comment 2 Dominik 'Rathann' Mierzejewski 2009-09-13 12:26:33 UTC
A couple of comments after a cursory look:

1. Name: ueagle-atm-firmware4 <- where does it come from? The project name is uEagle-ATM and the tarball name is ueagle4-data.

2. Summary is inconsistent with description (Eagle IV in summary vs. Eagle I-III in description).

3. Missing Requires: udev for ownership of /lib/firmware.

4. Unowned directory /lib/firmware/ueagle-atm.

5. Inconsistent macro usage (%{buildroot} vs. $RPM_BUILD_ROOT).

6. cp -rf -p <- surely cp -p is enough?

7. A part of the licence text looks fishy:
[...]
| USER ACKNOWLEDGES AND AGREES THAT THE PURCHASE OR USE OF THIS SOFTWARE WILL
| NOT CREATE OR GIVE GROUNDS FOR A
|  LICENSE BY IMPLICATION, ESTOPPEL, OR OTHERWISE IN ANY INTELLECTUAL
|  PROPERTY RIGHTS (PATENT, COPYRIGHT, TRADE SECRET, MASK WORK, OR OTHER
|  PROPRIETARY RIGHT) EMBODIED IN ANY OTHER IKANOS HARDWARE OR SOFTWARE
|  EITHER SOLELY OR IN COMBINATION WITH THIS SOFTWARE.
[...]
and should be vetted by Fedora Legal.

Comment 3 Tom "spot" Callaway 2009-10-06 15:58:57 UTC
Eh, its okay for a firmware license. Lifting FE-Legal.

Comment 4 Jason Tibbitts 2009-11-08 04:52:26 UTC
So it's been nearly two months since comment 2 and over a month since FE-Legal was lifted; did you intend to comment on the issues raised?  Or should this ticket be closed?

Comment 5 Hicham HAOUARI 2009-11-08 21:08:03 UTC
1. Origin of the Name:

From Fedora Package Guidelines :

http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Binary_Firmware

"Firmware packages must be named <foo>-firmware, where <foo> is the driver or other hardware component that the firmware is for."

The driver name is ueagle-atm, the number 4 because this firmware is for Chipset 4.

2.Summary is inconsistent with description : Fixed

3.Missing Requires: udev for ownership of /lib/firmware : Fixed

4.Unowned directory /lib/firmware/ueagle-atm : Fixed

5.Inconsistent macro usage (%{buildroot} vs. $RPM_BUILD_ROOT) : Fixed

6.cp -rf -p <- surely cp -p is enough? : fixed

7. Fedora Legal status : Fixed in comment 3

Spec URL : http://hicham.fedorapeople.org/ueagle-atm-firmware4.spec

SRPM URL : http://hicham.fedorapeople.org/ueagle-atm-firmware4-1.0-2.fc11.src.rpm

Comment 6 Tom "spot" Callaway 2009-11-09 01:18:22 UTC
You should either move the 4 to the version or have it appended to the name. I suggest: ueagle-atm4-firmware

Comment 7 Hicham HAOUARI 2009-11-09 01:41:31 UTC
ok spot, seems a good suggestion

Spec  URL: http://hicham.fedorapeople.org/ueagle-atm4-firmware.spec
SRPM URL: http://hicham.fedorapeople.org/ueagle-atm4-firmware-1.0-3.fc11.src.rpm

Comment 9 Peter Lemenkov 2010-03-07 14:49:04 UTC
I'll review it.

Comment 10 Peter Lemenkov 2010-03-07 15:15:09 UTC
Notes:

1) Source0 link should be http://download.gna.org/ueagleatm/ikanos/ueagle4-data-1.0.tar.gz
2) I don't like the idea to modify license file in any means, even fixing CRLF.
3) Typo. You definitely mean "%dir /lib/firmware/ueagle-atm" in %files section, not simply "%dir /lib/firmware"

The rest of the spec looks good. Here is my formal 

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is not silent, however I suppose that these two warnings may be safely ignored.

Sulaco ~/rpmbuild/SPECS: rpmlint ../RPMS/noarch/ueagle-atm4-firmware-1.0-3.fc12.noarch.rpm 
ueagle-atm4-firmware.noarch: W: spelling-error Summary(en_US) Chipset -> Chip set, Chip-set, Chipped
ueagle-atm4-firmware.noarch: W: spelling-error %description -l en_US Chipset -> Chip set, Chip-set, Chipped
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
Sulaco ~/rpmbuild/SPECS:

+ 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 file, containing the text of the license(s) for the package, 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, match the upstream source, as provided in the spec URL.

Sulaco ~/rpmbuild/SPECS: sha256sum ../SOURCES/ueagle4-data-1.0.tar.gz*
1e3547821d69b9f576add1e35223df159aadfd9e3dae913b6429a1cbbe1a3691  ../SOURCES/ueagle4-data-1.0.tar.gz
1e3547821d69b9f576add1e35223df159aadfd9e3dae913b6429a1cbbe1a3691  ../SOURCES/ueagle4-data-1.0.tar.gz.1
Sulaco ~/rpmbuild/SPECS: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All additional build dependencies are listed in BuildRequires. None, actually.
0 No need to handle locales.
0 No shared library files.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
- The package must own all directories that it creates. See my note #3 above regarding typo.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
- The package must not own files or directories already owned by other packages. Again, see my note #3 above.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Please fix issues, mentioned above, and I'll continue.

Comment 11 Hicham HAOUARI 2010-03-07 16:38:02 UTC
Updated.

Spec  URL: http://hicham.fedorapeople.org/ueagle-atm4-firmware.spec
SRPM URL:
http://hicham.fedorapeople.org/ueagle-atm4-firmware-1.0-4.fc12.src.rpm

1) Fixed.
2) The CRLF fix was done to remove an rpmlint warning.
3) Fixed.

Comment 12 Peter Lemenkov 2010-03-07 17:40:47 UTC
Regarding license - I'd rather package it as is, disregarding rpmling warning, so, please, consider removing this fix.

Anyway, this package is

APPROVED

Hicham, are you already sponsored by someone?

Comment 13 Hicham HAOUARI 2010-03-07 17:53:09 UTC
Yes, my fas account is hicham

Comment 14 Hicham HAOUARI 2010-03-07 17:57:19 UTC
New Package CVS Request
=======================
Package Name: ueagle-atm4-firmware
Short Description: Firmware for USB ADSL Modems based on Eagle IV Chipset
Owners: hicham
Branches: F-11 F-12
InitialCC: hicham

Comment 15 Kevin Fenzi 2010-03-09 06:06:42 UTC
CVS done (by process-cvs-requests.py).

I added a F-13 branch to the request.

Comment 16 Fedora Update System 2010-03-09 12:59:19 UTC
ueagle-atm4-firmware-1.0-4.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/ueagle-atm4-firmware-1.0-4.fc13

Comment 17 Fedora Update System 2010-03-09 13:10:48 UTC
ueagle-atm4-firmware-1.0-4.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/ueagle-atm4-firmware-1.0-4.fc12

Comment 18 Fedora Update System 2010-03-09 13:22:31 UTC
ueagle-atm4-firmware-1.0-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/ueagle-atm4-firmware-1.0-4.fc11

Comment 19 Fedora Update System 2010-03-11 07:18:21 UTC
ueagle-atm4-firmware-1.0-4.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 20 Fedora Update System 2010-03-11 07:21:05 UTC
ueagle-atm4-firmware-1.0-4.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2010-03-11 07:21:54 UTC
ueagle-atm4-firmware-1.0-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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