Bug#1002617: RFS: carl9170fw/1.9.9-399-gcd480b9-1 [ITP] -- firmware for AR9170 USB wireless adapters
On Thu, 2022-01-13 at 20:54 +, John Scott wrote: > You might like to have a look at this mail from Ben Hutchings: > https://lists.debian.org/msgid-search/179d6d32466dd13962a3aab251c45242fbf2d8ae.ca...@decadent.org.uk > The reason that none of the other Wi-Fi firmware packages have them is > because they're all non-free (with the exception of ath9k_htc). Thanks for the link, I'm not subscribed to that list. Makes sense. > I wasn't sure if there was any established convention; my thread "Naming > convention for udebs: -udeb/-installer suffix" didn't garner any > pertinent responses. I've switched the name though. I did a search of a Debian mirror filesystem and found that only the linux and linux-signed-* source packages use the -di suffix, most use the -udeb suffix. The -installer suffix seems to be used only for udeb packages when the source package has the -installer suffix too, such as bootloader installers like grub-installer, with a few exceptions for other udebs that install things. I suggest using the -udeb suffix. > These are all good catches, I'm working on incorporating them and doing > a slow and careful review. Recently on another project I noticed an upstream commit that added copyright information in the middle of a file alongside the functions that were copied from elsewhere. This sort of thing is hard to notice during the manual review of file headers I usually do using mc (after enabling the preview pane and arrow keys for navigation), but some of the copyright review tools might be helpful. I actually detected the list.h LGPL license using debmake -k (as run by check-all-the-things). Apparently the best option is ScanCode but that isn't in Debian yet. https://wiki.debian.org/CopyrightReviewTools > I think you're mistaken here, you should take a look at > /usr/share/dpkg/buildopts.mk which I include via an include directive in > debian/rules. Basically, DEB_BUILD_OPTION_PARALLEL is a helper macro for > the value of parallel from DEB_BUILD_OPTIONS; these are one and the > same. I see, I wasn't aware of this snippet/variable. > That's true. My intent was that, since it's a hidden directory, it would > help remind one that that directory gets created. It seems to've only > caused confusion, so I'll pull it. That seems reasonable if you want to keep it. Maybe add a comment. > Indeed, the documentation is rather old and terse and doesn't address > this issue. I'll probably ask the Reportbug folks and send them a MR > updating the docs. Great. > > Please ask upstream to make a new release, it has been a very long time > > since the last one. > I will do after making some of the following important changes. Thanks. > > Please ask upstream to send FindUSB-1.0.cmake & libusb-zeropacket.diff > > to libusb upstream and then remove them from carl9170fw. > I will ask, but with all due respect, I think this is lower priority and > that I'll have limited ability to help them. Sure. TBH they don't appear to be used by carl9170fw so I'm not sure why they are in the source repository at all. > I do not have a grasp, let alone a good one, of CMake, so this may be a > challenge. The documentation for CMake is fairly comprehensive, until recently I had never touched CMake and didn't understand it but when I needed to figure it out I found it to be reasonable and well documented. > I actually think I'll do one better: I submitted upstream an AppStream > metadata file a while ago, and although they haven't responded to it > yet, perhaps my sending them other changes will get their attention. > AppStream metadata and Debian upstream metadata files have considerable > overlap, and in my personal opinion having good AppStream metadata makes > an upstream metadata file unnecessary, but I'm willing to entertain > arguments to the contrary. I haven't looked at AppStream but I got the feeling they had different audiences or purposes or tools looking at them. -- bye, pabs https://wiki.debian.org/PaulWise signature.asc Description: This is a digitally signed message part
Bug#1002617: RFS: carl9170fw/1.9.9-399-gcd480b9-1 [ITP] -- firmware for AR9170 USB wireless adapters
Control: tags -1 moreinfo Hi Paul, Thanks for your very detailed review of carl9170fw. I'm still making my changes to the package and will give you a poke and remove the moreinfo tag once I have an upload ready for re-review. > I don't think udebs are needed for firmware packages, none of the other > WiFi firmware packages in Debian have them. You might like to have a look at this mail from Ben Hutchings: https://lists.debian.org/msgid-search/179d6d32466dd13962a3aab251c45242fbf2d8ae.ca...@decadent.org.uk The reason that none of the other Wi-Fi firmware packages have them is because they're all non-free (with the exception of ath9k_htc). My understanding—which was most definitely not articulated by Ben—is that the Debian Installer has a mechanism for loading the (non-free) firmware from the ordinary .debs. Since the installer needs to have logic to look for non-free firmware on removable media at runtime anyway, that's quite a different situation to most that warrant udebs, where the udeb is a "built-in" of the installer. FYI, open-ath9k-htc-firmware is in NEW because my most recent upload likewise adds a udeb for it. > If the package is actually needed it should be named -udeb not -di, > since other udebs use -udeb. I wasn't sure if there was any established convention; my thread "Naming convention for udebs: -udeb/-installer suffix" didn't garner any pertinent responses. I've switched the name though. > Several files have missing/incorrect information in debian/copyright, > please do a full audit of the code looking for copyright/license info. > > * tools/include/list.h is LGPL > * tools/include/frame.h is partly from Linux, unknown copyright > * include/shared/eeprom.h also contains ISC code These are all good catches, I'm working on incorporating them and doing a slow and careful review. > DEB_BUILD_OPTION_PARALLEL doesn't appear to be a standard variable, > please switch to DEB_BUILD_OPTIONS=parallel=N instead, see Debian > Policy, section 4.9.1 and debhelper(7) and dpkg-buildpackage(1). I think you're mistaken here, you should take a look at /usr/share/dpkg/buildopts.mk which I include via an include directive in debian/rules. Basically, DEB_BUILD_OPTION_PARALLEL is a helper macro for the value of parallel from DEB_BUILD_OPTIONS; these are one and the same. There's a chance of the DEB_BUILD_OPTION_PARALLEL Makefile macro still being unset, so what this line does in my Makefile: DEB_BUILD_OPTION_PARALLEL ?= 1 is set it to 1 if it's not set in one's DEB_BUILD_OPTIONS. That way, calls like make -j$(DEB_BUILD_OPTION_PARALLEL) won't become make -j which would be very bad. > Some things that would be nice to fix at some stage: > > Nothing in debian/rules references .config so you can drop that from > before the execute_before_dh_auto_configure target. That's true. My intent was that, since it's a hidden directory, it would help remind one that that directory gets created. It seems to've only caused confusion, so I'll pull it. > It seems like the Homepage should be the carl9170.fw firmware wiki page > instead of the carl9170 driver wiki page. > > https://wireless.wiki.kernel.org/en/users/drivers/carl9170.fw Good catch, I will fix that. > I would express the license of include/shared/fwcmd.h etc as GPL-2-only > && ISC rather than putting a copy of the ISC license in a comment. I agree that this is sensible. > bug-presubj isn't well wrapped. I'm not sure if wrapped or unwrapped is > the best option for this file though. Indeed, the documentation is rather old and terse and doesn't address this issue. I'll probably ask the Reportbug folks and send them a MR updating the docs. > Please ask upstream to make a new release, it has been a very long time > since the last one. I will do after making some of the following important changes. > Please ask upstream to update their copies of code from the Linux > kernel and other external sources to the latest versions. I can probably help them with this. > Please ask upstream to send FindUSB-1.0.cmake & libusb-zeropacket.diff > to libusb upstream and then remove them from carl9170fw. I will ask, but with all due respect, I think this is lower priority and that I'll have limited ability to help them. > Please ask upstream to delete FindPackageHandleStandardArgs.cmake, > since that is now available from cmake upstream and from Debian cmake. > Potentially cmake_minimum_required will need to be bumped for this. Will do. > Please ask upstream to include the copyright information > for carlfw/src/memcpy.S and carlfw/src/memset.S in the files. Especially since it is copyleft, I will definitely prioritize this. > Please ask upstream to update the COPYRIGHT file. I will be happy to do this. > Please send upstream some changes that would allow building the > upstream source using a pre-packaged toolchain like the Debian one. > > Please also figure out how to eliminate the other debian/rules > workarounds. I do not have a grasp,
Bug#1002617: RFS: carl9170fw/1.9.9-399-gcd480b9-1 [ITP] -- firmware for AR9170 USB wireless adapters
On Sat, 2021-12-25 at 18:25 +, John Scott wrote: > https://mentors.debian.net/debian/pool/main/c/carl9170fw/carl9170fw_1.9.9-399-gcd480b9-1.dsc Some things that prevent the upload of this package: I don't think udebs are needed for firmware packages, none of the other WiFi firmware packages in Debian have them. If the package is actually needed it should be named -udeb not -di, since other udebs use -udeb. Several files have missing/incorrect information in debian/copyright, please do a full audit of the code looking for copyright/license info. * tools/include/list.h is LGPL * tools/include/frame.h is partly from Linux, unknown copyright * include/shared/eeprom.h also contains ISC code DEB_BUILD_OPTION_PARALLEL doesn't appear to be a standard variable, please switch to DEB_BUILD_OPTIONS=parallel=N instead, see Debian Policy, section 4.9.1 and debhelper(7) and dpkg-buildpackage(1). Some things that would be nice to fix at some stage: Nothing in debian/rules references .config so you can drop that from before the execute_before_dh_auto_configure target. It seems like the Homepage should be the carl9170.fw firmware wiki page instead of the carl9170 driver wiki page. https://wireless.wiki.kernel.org/en/users/drivers/carl9170.fw I would express the license of include/shared/fwcmd.h etc as GPL-2-only && ISC rather than putting a copy of the ISC license in a comment. bug-presubj isn't well wrapped. I'm not sure if wrapped or unwrapped is the best option for this file though. Please ask upstream to make a new release, it has been a very long time since the last one. Please ask upstream to update their copies of code from the Linux kernel and other external sources to the latest versions. Please ask upstream to send FindUSB-1.0.cmake & libusb-zeropacket.diff to libusb upstream and then remove them from carl9170fw. Please ask upstream to delete FindPackageHandleStandardArgs.cmake, since that is now available from cmake upstream and from Debian cmake. Potentially cmake_minimum_required will need to be bumped for this. Please ask upstream to include the copyright information for carlfw/src/memcpy.S and carlfw/src/memset.S in the files. Please ask upstream to update the COPYRIGHT file. Please send upstream some changes that would allow building the upstream source using a pre-packaged toolchain like the Debian one. Please also figure out how to eliminate the other debian/rules workarounds. It would be nice if the Linux kernel community or the Debian src:linux package could split kconfig off into a reusable component. Please add an upstream metadata file: https://wiki.debian.org/UpstreamMetadata I suggest these arguments to wrap-and-sort: wrap-and-sort --short-indent --wrap-always --sort-binary-packages --trailing-comma --dry-run These tests from check-all-the-things suggest some polishing for upstream and or yourself: anorack codespell cppcheck cpuinfo debmake-k duck http path-max proselint shellcheck spellintian todo https://github.com/collab-qa/check-all-the-things -- bye, pabs https://wiki.debian.org/PaulWise signature.asc Description: This is a digitally signed message part
Bug#1002617: RFS: carl9170fw/1.9.9-399-gcd480b9-1 [ITP] -- firmware for AR9170 USB wireless adapters
Package: sponsorship-requests Severity: wishlist X-Debbugs-CC: debian-ker...@lists.debian.org Dear mentors and Kernel Team, I'm looking for a sponsor for my package "carl9170fw": * Package name : carl9170fw Version : 1.9.9-399-gcd480b9-1 Upstream Author : linux-wirel...@vger.kernel.org * URL : https://wireless.wiki.kernel.org/en/users/Drivers/carl9170 * License : many; the binary is effectively GPL 2.0 only * Vcs : https://salsa.debian.org/kernel-team/carl9170fw Section : kernel It builds those binary packages: firmware-carl9170 - firmware for AR9170 USB wireless adapters firmware-carl9170-di - firmware for AR9170 USB wireless adapters - udeb To access further information about this package, please visit the following URL: https://mentors.debian.net/package/carl9170fw/ Alternatively, one can download the package with dget using this command: dget -x https://mentors.debian.net/debian/pool/main/c/carl9170fw/carl9170fw_1.9.9-399-gcd480b9-1.dsc This is the initial upload of the carl9170 libre wireless firmware. Although it's already shipped in firmware-linux-free, this package will build it from source using the already-packaged SuperH cross-compiler. The package will not yet be installable unless one manually deletes the carl9170fw binary on their system. The goal of this upload is just to get the package through NEW. After it clears NEW, we'll be able to do the handover of the firmware by (in order, hopefully within the span of a day or so): * having firmware-carl9170 add Breaks+Replaces on firmware-linux-free and uploading to unstable * taking the firmware out of firmware-linux-free * having firmware-linux-free add Recommends: firmware-carl9170 * and then uploading firmware-linux-free to unstable I don't expect that the udeb will be used by the Debian Installer anytime soon because I still have to learn how to make that happen, but based on previous discussions I know it will be necessary. It's not strictly necessary (they can be done in either order), but a sponsor who wants to help could also upload gcc-sh-elf v2 for me. That RFS is #1002582. I own the hardware and can affirm that this package works with it. signature.asc Description: This is a digitally signed message part
