Bug 248224
| Summary: | Review Request: iwl4965-firmware - Firmware for Intel� PRO/Wireless 4965 A/G/N network adaptors | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Nicolas Chauvet (kwizart) <kwizart> |
| Component: | Package Review | Assignee: | Jason Tibbitts <j> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, fedora, katzj, linville, matt_domsch, matthias, notting |
| Target Milestone: | --- | Flags: | kwizart:
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: | 2007-08-27 00:08:03 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
Nicolas Chauvet (kwizart)
2007-07-13 21:18:14 UTC
I'll go ahead and review this, but I'm not sure if the plan is to maintain a separate iwl4965 package (and rename the existing iwlwifi-firmware package to iwl3945-firmware) or to just add the 4965 firmware to the iwlwifi package. So before anything gets checked in you should coordinate with the iwl3945-firmware maintainer. Anyway, the only issue I see with this package is the file /lib/firmware/LICENSE.iwlwifi-4965-ucode, which doesn't need to be there. The copy in /ur/share/doc is sufficient. Spec URL: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware.spec SRPM URL: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware-4.44.15-2.kwizart.fc7.src.rpm Description: Firmware for Intel® PRO/Wireless 4965 A/G/N network adaptors Drop License from /lib/firmware Use ExcludeArch: ppc ppc64, as this may prevent koji to provide it only for i386 x86_64 cc'd matthias for having his advice about how to do with iwlwifi-firmware... But i think this is the same case as ipw2100 and ipw2200. there is two separates firmwares for each chipset as there is two separate module. The release date for each firmware are diffenrent. This mean that we can have to update one and not the other Having boht firmwares in iwlfiwif-firmware will lead to problem if we deceide to patch kernel to use versionned firmware... About the current package, I wouldn't use %{?dist} in order to be able to share
the noarch.rpm file across many (all) releases.
Then, the current iwlwifi-firmware package will need to be renamed, just like
upstream did a while back. I'd suggest keeping the prefix from the original
archive and have iwlwifi-3945-firmware and iwlwifi-4965-firmware as the names,
both obsoleting iwlwifi-firmware up to its last know version, in order to get
iwlwifi-4965-firmware installed on all F-7 installs (for instance fresh installs
on laptops applying all updated through the wired connection will automatically
get the new firmware).
Ok renamed to iwlwifi-4965-firmware I have few experience about two packages obsoleting/providing one... I don't know if it will work with something like this: Provides: iwlwifi-fimware Obsoletes: iwlwifi-firmware =< 2.14.4-1 I think it cannot provides versionned iwlwifi-firmware as this may confict with iwlwifi-3945-firmware ?! Spec URL: http://kwizart.free.fr/fedora/7/testing/iwlwifi-4965-firmware/iwlwifi-4965-firmware.spec SRPM URL: http://kwizart.free.fr/fedora/7/testing/iwlwifi-4965-firmware/iwlwifi-4965-firmware-4.44.15-3.src.rpm Description: Firmware for Intel® PRO/Wireless 4965 A/G/N network adaptors Ok renamed, i haven't tested if this provides/Obsoletes works... (In reply to comment #6) > Ok renamed, i haven't tested if this provides/Obsoletes works... Will, but typo in fimware vs. firmware provides :-) BTW, I know rpmlint complains when an obsoletes isn't also provided, but in this case it doesn't make that much sense... Everything else looks good to me, I now have to prepare the rename of iwlwifi-firmware otherwise if this new and different firmware is released now it will replace the different 3945 one and... make many users unhappy! Two more comments : - The LICENSE file name in %description is wrong. - You could do the same as I do in the 3945 package and rename the docs to shorter README and LICENSE names since they'll be in their own directory in the end. The rename of iwlwifi-3945-firmware will be discussed in bug #230096. Do you mean i need to drop Provides: iwlwifi-firmware ? if both firmwares obsoletes iwlwifi-firmware this mean that they need to be pushed at the same time on the repositories ? Considering iwlwifi-firmware never had 4965 firmware, why would you want to provide/obsolete it here? Just have the 3965 version do it. Note that the guidelines say 'Firmware packages should be named <foo>-firmware, where <foo> is the driver or other hardware component that the firmware is for.' So, that would lead to 'iwl4965-firmware', rather than iwlwifi-4965-firmware. Well, the main idea was to bring the firmwares for both modules (3945 and 4965) as an update of iwlwifi-firmware. So firmware for 4965 get installed for users that potentially need it... If not, users that need it, will have to get documented about how to install the right package for it... Actually i don't think that's too much to ask. We can choose this way also... This lead to have this: SRPMS: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware-4.44.15-5.src.rpm SPEC: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware.spec Description: Firmware for Intel® PRO/Wireless 4965 A/G/N network adaptors Current version is 4.44.17 fwiw... Hello! J.Linville Does this mean that this version will be used from a Fedora kernel (F8 F7 ?) which version of the kernel for which firmware ? Or does we are allowed to use versionned firmware ? Can we parallele install firmware (and switch from one to the other with alternative for example..) Then we have differents choice: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware-4.44.17-1.src.rpm http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware.spec-1 (with alternatives - you can try an update or a parrallele install from the previous 4.44.15-6 )... http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware-4.44.17-2.src.rpm http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware.spec-2 (without alternatives) FWIW, current iwlwifi sources don't support firmware versioning. Perhaps they should, but they don't. Is the "alternatives" stuff really the best way to handle versioned firmware packages anyway? It seems a little heavy for firmware. Dunno, maybe there is no other appropriate mechanism...? The above is brainstorming, and should NOT be interpreted as a NAK of any sort. I think both versions are acceptable. Using alternatives seems plain wrong. It's not like anyone wants to "choose" the firmware they use with a given kernel. The solution is to try and convince Intel to go back to how they were doing things previously, as it was very easy to bundle the older firmwares along with the latest, so that people could downgrade and upgrade kernels all they wanted (basically, the latest package always worked with all kernels from the first one of the install media, to the very latest errata one). If they don't want to do it... well... either try to implement something similar inside the Fedora kernel (ugh!), or continue trying to convince them, as I'm pretty sure they'll be "suffering" from the situation too, with feedback about problems related to the firmware version not being the right one, over and over. I think there is a missunderstanding about the alternative: When an update of the firmware is avaible, it takes the newer version of the firmware (this is not a user choice at this step, because of the priority field is upper with a newer version - tested) Also updating the symlink when the interface is used; this won't lead to problems as the firmware is already loaded, (it might be problematic if there is firmware reload but there will be also with package upgrade )... This method is compatible with both non-versionned-firmwares used by kernel and (if later avaible) versionned-firmwares used by kernel. If versionned_firmware methode exist, then the alternatives method will only be usefull for non-versionned_firmware kernels. And users will need to run alternatives --config iwl4965-firmware before to reboot with the relative kernel... Of cource, the better way is to have versionned firmwares into kernel for iwlwifi (and maybe others), But if there is one, there is still the same question we raised for compat-kernel without versionned firmware we will have to solve...) TODO: Have support for parrallele installable firmwares via yum (same as kernel) Is is possible to run a script early enough in the boot process to detect the current kernel version and make a symlink to the appropriate bit of firmware so that the loader will always see the right thing? Latest patches from iwlwifi now _DO_ support API versioning for the iwlwifi firmware packages...FYI. So I guess the fundamental question now is when packages will be able to take advantage of that, and what changes are required in this package to make use of it. SRPMS: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware-4.44.1.18-1.src.rpm SPEC: http://kwizart.free.fr/fedora/7/testing/iwl4965-firmware/iwl4965-firmware.spec Description: Firmware for Intel® PRO/Wireless 4965 A/G/N network adaptors This package now bundle two firmwares (like ipw2200 do previously) ACK Everything looks good to me here. tibbs -- do you have anything else or can we approve this and get it built? I had a promise of some testing om #fedora-devel which I was hoping to see, but
in lieu of that, here's a full review.
The only comment I can make is that it would be good to have, in the spec, some
explanation of why there are multiple versions bundled and some statement of how
long old versions need to be kept bundled in the package.
* source files match upstream:
6c1c870562c259af3df33756a811967e7e5ae6176e5ebdfc3d74a1fdc5f86d8d
iwlwifi-4965-ucode-4.44.1.18.tgz
dc48285a93577aee3a24614c089a24f9aff7e29ab92bbea505704b54aeeed6d5
iwlwifi-4965-ucode-4.44.17.tgz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* build root is OK.
* license field matches the actual license.
* license is acceptable for firmware in Fedora.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none)
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint is silent.
* final provides and requires are sane; provides itself and requires nothing.
* No automated testing possible, but several successful test reports in this
ticket.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* acceptable content (firmware)
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
APPROVED
Thanks for the review! I will remove the dist tag and build for F-7 so it will be available into the rawhide repository without a fc7 tag... I can do EPEL's later if needed... I'm searching for a co-maintainer for this package, because if it often change, we probably need to make an update with a low time to answear... About the time to obsolete firmware version, it's when Fedora 7 will be end-of life (since Fedora Core 6 do not allow firmwares). Then we have to keept the firmware which "will" be used when Fedora 8 will be released...This drop can be done on new firmware release... About this comment, I can make it within the spec file or inside the cvs as a packager_note.txt ... (which will not be bundled - that's our internal kitchen after all) New Package CVS Request ======================= Package Name: iwl4965-firmware Short Description: Firmware for Intel® PRO/Wireless 4965 A/G/N network adaptors Owners: kwizart / kwizart Branches: F-7 devel InitialCC: <empty> Commits by cvsextras: yes Updated cvs request as bjensen proposed me his help... ( don't know if it is good enought for multiple owers...) New Package CVS Request ======================= Package Name: iwl4965-firmware Short Description: Firmware for Intel® PRO/Wireless 4965 A/G/N network adaptors Owners: kwizart,bjensen Branches: F-7 devel InitialCC: <empty> Commits by cvsextras: yes I don't see a 'bjensen' account in the account system. Can you doublecheck and reset fedora-cvs flag for additional changes? In the mean time, cvs done for all except the co-maintainer. Package Change Request ====================== Package Name: iwl4965-firmware New Branches: EL-4 EL-5 I'v made some mistake with request Flags here! cvs done. |