On Sat, 2016-10-15 at 10:38 +0300, Paul Fertser wrote: > Many thanks for your review, please see some answers below. Thanks to both of you for the explanations and updated package. I'll discuss the ROM code from two angles: Debian & Software Freedom 1) Debian The Debian and ftp-master policy is that all the contents of main, including all files in the upstream source tarballs, must be DFSG-free and also build-able using only main. For open-ath9k-htc-firmware there are some files that AFAIK don't meet this criteria and so they should be stripped from the orig.tar.gz (not just removed at build time). I would suggest not just stripping all of sboot/ to preserve the ability of offline Debian users to study the remaining source code. The usual way to repack upstream tarballs for this purpose is to put a Files-Excluded header with associated Comment header explaining the issue in debian/copyright and have uscan do the tarball repack. https://wiki.debian.org/UscanEnhancements At the same time there are some compiled files that do seem to have source present and are probably buildable with the toolchain built during the build process but are probably not useful to have in the tarball, those could be stripped too to avoid confusion. Please correct me if I have wrong information regarding these files: This directory seems like it is solely prebuilt files and is very unlikely to contain freely-buildable files, since the subdirectories are named fpga and asic and the chip was produced before the free icestorm toolchain for Lattice FPGAs was released. I don't know if all the source is present or not but I found a string (see below) that is present in the ROM output dir, but not anywhere else in the tree. To avoid having to do the work to prove that the source is present and buildable using only Debian main, I would suggest removing this dir: ./sboot/magpie_1_1/image/output/ COMMAND TO START FIRMWARE RECEIVED These files appear to have their source in the .S files in the ./sboot/athos/src/xtos/ directory as there are similar names there. Removing them would avoid having to explain where the source is and checking that it is buildable with the toolchain built by the package. ./sboot/magpie_1_1/sboot/athos/src/libhandlers-board.a ./sboot/magpie_1_1/sboot/athos/src/_vectors.o ./sboot/magpie_1_1/sboot/athos/src/crt1-tiny.o ./sboot/make_opt/lib/libhandlers-board.a ./sboot/make_opt/lib/_vectors.o ./sboot/make_opt/lib/crt1-tiny.o The PNG files could also be removed to save a small amount of space, but it is clear that the source is present and that we have Inkscape in Debian main so the PNG files can be rebuilt from their SVG sources, so this isn't strictly necessary. Also, the removal could happen upstream and a Makefile could be added using rsvg to allow building the PNGs, if they are needed to be present for a particular reason. 2) Software Freedom From a Software Freedom PoV, the mask ROM is bad because it is truly read-only software that users can't modify. OTOH, it reduces RAM needed by the firmware so it enables users to do more with the device. This is just an idea for improving the available software freedom for users of this firmware, not something I expect to be done any time soon: Ideally the entire ROM source code could be reproducibly built and during the default build it would compile the code and then compare the resulting hash against the known ROM hash. When someone wants to experiment with modifying a section of the code (a function or similar), the build happens as per normal but they request that certain functions be placed into the firmware for loading into RAM instead. There could also be a test mode where as much of the ROM code is added to RAM as possible and then the RAM version is tested to see if it behaves the same as the ROM version. Some further thoughts on other things: The debian/copyright file does not appear to be fully correct, for example a number of files in the sboot/ directory look like MIT, X11 or Expat licenses rather than BSD-3-clause-from-Qualcomm-Atheros. This would need to be fixed before uploading to Debian. I noticed this in the output of `debmake -k` btw. It might be interesting to have isenkram/apt/etc suggest this firmware when someone plugs in a device that can run this firmware. https://wiki.debian.org/AppStream/Guidelines#Announcing_supported_hardware > This one makes me wonder if it'd be the correct way to go. I assume > only the copyright holder can do this, and spending time contacting > eCos developers (and probably lawyers) just to get it right for some > really old code seems impractical. I'd consider this to be an upstream > issue unrelated to Debian packaging. Please correct me if I'm wrong. Yes, this is an upstream issue. I don't think changing the address needs contacting the copyright holders since it is mainly informational rather than part of the license grant (IMO). It is part of a comment so presumably changing it could have no effect on any ROM binaries. > Heh, this is a bit problematic since openbios maintainers missed that > part. Should I try contacting them for the clarifications? You have done this, great :) > The situation with versions is complicated. IMHO we'll need to prepare > a firmare-ath9k-htc-1.5.0 (including the version in the name) package > when 1.5.0 is released. So if end user install it and a suitable > kernel, it'll get picked up automatically. For that package no > ath9k_htc.conf modrobe file would be needed. For an intermediate > version like the one we have now, it makes sense to label it "dev" and > to add the file to make it work out of the box. That seems reasonable. I wonder if this package should be renamed too? > I've read that compat 10 is an experimental format currently, but if > you say it's ok to use it, great. It is ok to use debhelper compat 10 now. I do hope upstream will take a look at the check-all-the-things output again, especially three of the cppcheck warnings for the target_firmware. -- bye, pabs https://wiki.debian.org/PaulWise
Attachment:
signature.asc
Description: This is a digitally signed message part