Am 13.10.2016 um 05:55 schrieb Paul Wise:
> On Wed, Oct 12, 2016 at 7:38 PM, Paul Fertser wrote:
>
>> Cc: Oleksij Rempel <linux@rempel-privat.de>
>
> Please use the X-Debbugs-CC pseudo-header when submitting bugs instead
> of CC, so that the recipients get the bug number too. Put it just
> after Severity in the mail body so those CCed can see it:
>
> https://www.debian.org/Bugs/Reporting#xcc
>
>> I am looking for a sponsor for my package "open-ath9k-htc-firmware"
>
> Correct me if I'm wrong but IIRC either yourself or Oleksij have
> commit/release access upstream?
correct.
> The comments for the overrides for lintian tag source-is-missing
> should indicate which file is the source for each prebuilt file. I
> would have one comment per instance of the tag. Just one comment
> saying they are removed at build time is enough for all of the
> overrides for lintian tag source-contains-prebuilt-binary. Personally,
> I would suggest removing all of these files from the upstream git
> repository and always building them from source. If you need to keep
> them, put them in tarballs in the github releases.
I can answer this part, especially because most of comments are related
to sboot/ folder. sboot contains ROM code flashed to the chip or eeprom.
Not all ROM parts seems to be open (fix me if i'm wrong), but at least
contain some binary. If some person will decide to RE closed parts, it
will be easier to know what exactly should be RE instead of work on
complete ROM.
No one will ever touch/patch sboot. At least i will not allow it.
The actual firmware is located in RAM and to reduce memory usage, it is
using ROM functions. To understand what is used and to be able to fix
any thing in the firmware you need to read sboot. The sboot should
reflect state of the code with all this bugs and problems. Even if this
is wrong FSF address.
For most paranoid persons we remove sboot before build is started.
> Personally, I would recommend the generated files docs/*.png be
> removed from the upstream git repo and rendered at build time from
> their source SVG files if they are needed.
ok, i'll remove pngs.
> I think you should also remove the prebuilt files before
> dh_auto_configure so that they can never get used by a build, even a
> manual one where `debian/rules clean` is not run.
>
> What is the reason for removing the docs/ and sboot/ directories in
> override_dh_clean? AFAICS both of these contain source files. IMO you
> should just remove the generated files.
>
> The cmake part of the build process does not print compiler
> invocation. Debian requires full compiler flags/output in build logs.
> For cmake usually debhelper automatically passes
> -DCMAKE_VERBOSE_MAKEFILE=ON but it doesn't seem to be working here?
>
> The debian/watch file needs adjusting for the new source package name:
> s/firmware-ath9k-htc/open-ath9k-htc-firmware/
>
> Personally I would leave "debian uupdate" out of the watch file
> because it can be annoying for people who just want to download.
>
> I would use a debian/clean file instead of override_dh_clean.
>
> Please get the FSF address corrected in the upstream copyright info.
>
> debian/cross-toolchain.mk needs copyright/license info filled in,
> preferably in both the header of it and in debian/control.
>
> The Uploaders field is empty. I would have expected to your name there
> if you intend to co-maintain it with Oleksij.
>
> I would recommend running this command (from the devscripts package)
> so that future diffs of the Debian packaging are easier to read:
>
> wrap-and-sort --short-indent --wrap-always --sort-binary-packages
> --trailing-comma
>
> The Vcs-* fields should point at the repository containing the Debian
> packaging, not upstream:
>
> https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-VCS-fields
>
> The Conflicts/Suggests fields are empty, you can remove them from
> debian/control.
>
> For merged bugs, you only need to close one of them and the rest will
> be closed too.
>
> What is the reason for renaming the upstream firmware files? Does the
> Linux kernel detect the new names?
upstream file names have old name schema without version numbers. Since
we are not able to guarantee 100% backwards compatibility and testing
coverage for all available kernel version we need to have different fw
binaries for different version. For example linux-firmware repo contains
both 1.3 and 1.4 binaries.
The same is about debian firmware-atheros packet, it contains:
/lib/firmware/ath9k_htc/htc_7010-1.4.0.fw
/lib/firmware/ath9k_htc/htc_9271-1.4.0.fw
/lib/firmware/htc_7010.fw
/lib/firmware/htc_9271.fw
Only way to avoid conflict between packages and be able to provide FW
version different from actual realise, for example with security fixes
is to use /lib/firmware/ath9k_htc/htc_9271-1.dev.0.fw name. To tell the
kernel that this should be used, we need to provide ath9k_htc.conf
At same time Adrian Chadd is promising to provide BSD driver for this
HW. Which FW names he will use is not clear, so IMO it makes no sense to
change the names in the project.
>
> What is the reason for debian/ath9k_htc.conf? Why is it
> Debian-specific instead of being committed upstream?
>
> I'd recommend either passing --parallel at the end of the args to dh,
> or upgrading to debian/compat 10, which uses that by default.
>
> Please add some upstream metadata:
>
> https://wiki.debian.org/UpstreamMetadata
>
> I'd suggest signing all tarballs, tags and commits with OpenPGP and
> having uscan verify the tarball sigs:
>
> https://wiki.debian.org/Creating%20signed%20GitHub%20releases
> https://mikegerwitz.com/papers/git-horror-story
> https://wiki.debian.org/debian/watch#Cryptographic_signature_verification
> https://help.riseup.net/en/security/message-security/openpgp/best-practices
>
> Automatic checks:
>
> build:
>
> Various gcc and other warnings.
>
> lintian:
>
> P: open-ath9k-htc-firmware source: debian-watch-may-check-gpg-signature
> P: firmware-ath9k-htc: no-upstream-changelog
> I: firmware-ath9k-htc: extended-description-is-probably-too-short
> P: firmware-ath9k-htc-dbgsym: no-upstream-changelog
> I: firmware-ath9k-htc-dbgsym: extended-description-is-probably-too-short
>
> check-all-the-things:
>
> $ env PERL5OPT=-m-lib=. cme check dpkg
> Warning in 'copyright Format' value
> 'http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/':
> Format uses insecure http protocol instead of https
> you can try 'cme fix dpkg' to fix the warnings shown above
this code has worst quality in many ways, non of it will be fixed before
packaging. And since packaging already took some weeks of my free time
budget, i assume non of it will be fixed any time soon.
> $ codespell --quiet-level=3
> <lots>
>
> $ cppcheck -j1 --quiet -f .
> [sboot/magpie_1_1/sboot/athos/src/xtos/xtos-internal.h:142]: (error)
> syntax error
> [sboot/utility/adjust_dep/adj_dep.c:63]: (error) Buffer overrun
> possible for long command line arguments.
> [sboot/utility/adjust_time/adj_time.c:57]: (error) Buffer overrun
> possible for long command line arguments.
> [sboot/utility/adjust_time/adj_time.c:58]: (error) Buffer overrun
> possible for long command line arguments.
> [sboot/utility/bin2hex/bin2hex.c:197]: (error) Buffer overrun possible
> for long command line arguments.
> [sboot/utility/bin2hex/bin2hex.c:198]: (error) Buffer overrun possible
> for long command line arguments.
> [sboot/utility/bin2hex/bin2hex_swp.c:244]: (error) Buffer overrun
> possible for long command line arguments.
> [sboot/utility/bin2hex/bin2hex_swp.c:245]: (error) Buffer overrun
> possible for long command line arguments.
> [sboot/utility/imghdr/imghdr.c:183]: (error) Buffer overrun possible
> for long command line arguments.
> [sboot/utility/imghdr/imghdr.c:184]: (error) Buffer overrun possible
> for long command line arguments.
> [target_firmware/magpie_fw_dev/target/cmnos/dbg_api.c:474]: (error)
> Uninitialized variable: val
> [target_firmware/magpie_fw_dev/target/init/app_start.c:155]: (error)
> Uninitialized variable: rst_status
> [target_firmware/wlan/if_owl.c:1482]: (error) Uninitialized variable: prate
> [target_firmware/wlan/ratectrl_11n_ln.c:416]: (error) Array
> 'pRc.validRateIndex[54]' accessed at index -4, which is out of bounds.
>
> $ env PERL5OPT=-m-lib=. duck
> I: debian/copyright:116: URL:
> http://sources.redhat.com/ecos/ecos-license/: INFORMATION
> (Certainty:possible)
> Domain redirect detected: http://sources.redhat.com ->
> http://ecos.sourceware.org. Probably a new upstream website?
>
> I: debian/copyright:221: URL:
> http://sources.redhat.com/ecos/ecos-license/: INFORMATION
> (Certainty:possible)
> Domain redirect detected: http://sources.redhat.com ->
> http://ecos.sourceware.org. Probably a new upstream website?
>
> $ find \( -name .git -o -name .svn -o -name .bzr -o -name CVS -o -name
> .hg -o -name _darcs -o -name _FOSSIL_ -o -name .sgdrawer -o -name .pc
> \) -prune -o -empty -print
> ./sboot/k2_1_0/lib/ram/dummy.txt
> ./sboot/magpie_1_1/lib/ram/dummy.txt
> ./sboot/magpie_1_1/lib/rom/dummy.txt
> ./sboot/magpie_1_1/lib/sboot/dummy.txt
sboot...
> $ fdupes -q -r . | grep -vE
> '/(\.(git|svn|bzr|hg|sgdrawer|pc)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s
> <lots>
>
> $ flawfinder -Q -c .
> <lots>
>
> # check if these can be switched to https://
> $ grep -rF http: .
> <lots>
>
> $ find -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o
> -iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o
> -iname '*.hpp' \) -exec include-what-you-use {} \;
> <lots, probably many are false positives>
>
> $ find -type d \( -iname .git -o -iname .svn -o -iname .bzr -o -iname
> CVS -o -iname .hg -o -iname _darcs -o -iname _FOSSIL_ -o -iname
> .sgdrawer -o -iname .pc \) -prune -o -type f ! \( -iname '*.blend' -o
> -iname '*.icns' -o -iname '*.bmp' -o -iname '*.ico' -o -iname '*.png'
> -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o -iname
> '*.tga' -o -iname '*.xcf' -o -iname '*.tif' -o -iname '*.tiff' -o
> -iname '*.mo' -o -iname '*.gmo' -o -iname '*.gz' -o -iname '*.bz2' -o
> -iname '*.xz' -o -iname '*.lz' -o -iname '*.zip' -o -iname '*.tar' -o
> -iname '*.deb' -o -iname '*.pdf' -o -iname '*.odt' -o -iname '*.docx'
> -o -iname '*.doc' -o -iname '*.chm' -o -iname '*.torrent' -o -iname
> '*.pyc' -o -iname '*.pyo' -o -iname '*.o' -o -iname '*.so' -o -iname
> '*.so.*' -o -iname '*.debug' -o -iname '*.wav' -o -iname '*.ogg' -o
> -iname '*.oga' -o -iname '*.ogv' -o -iname '*.mid' -o -iname '*.mp3'
> -o -iname '*.flac' -o -iname '*.ttf' -o -iname '*.otf' -o -iname
> '*.fon' -o -iname '*.pgp' -o -iname '*.gpg' -o -iname '*.dat' \) -exec
> isutf8 {} +
> ./target_firmware/magpie_fw_dev/target/cmnos/cmnos_sflash.c: line 452,
> char 1, byte offset 24: invalid UTF-8 code
> ./NOTICE.TXT: line 15, char 1, byte offset 17: invalid UTF-8 code
> ./sboot/magpie_1_1/sboot/cmnos/sflash/src/cmnos_sflash.c: line 454,
> char 1, byte offset 24: invalid UTF-8 code
>
> $ licensecheck --check=. --recursive --copyright . | grep --text -F
> 'with incorrect FSF address'
> ./sboot/magpie_1_1/sboot/cmnos/printf/src/cmnos_printf.c: GPL (with
> incorrect FSF address)
sboot... not relevant.
> $ env PERL5OPT=-m-lib=. perlcritic -1 . 2>&1 | grep -vF 'No perl files
> were found.'
> <lots>
>
> # Users of binary packages do not need install instructions.
> $ find -type f -iname '*README*' -a ! \( -iname README.md -o -iname
> README.rst -o -iname README.install \) -exec grep --ignore-case
> --fixed-strings --with-filename install {} +
> ./README:* Install the cmake build tool (http://www.cmake.org/).
> ./README:* For FreeBSD - install gmake and wget.
>
> $ find -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh'
> \) -exec shellcheck {} +
> <lots>
>
> $ find -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname
> .svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o
> -iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o
> -iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname
> _sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o
> -iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname
> '~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o
> -iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o
> -iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o
> -iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname
> '*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname
> '*.css.min' -o -iname '*.wav' \) -exec env PERL5OPT=-m-lib=.
> spellintian --picky {} +
> <lots>
>
> # Prevents reproducible builds: https://reproducible-builds.org/
> $ grep -rE ' __DATE__|__TIME__|__TIMESTAMP__' .
> ./sboot/magpie_1_1/sboot/htc/src/htc.c: //
> MIN_BUF_SIZE_FOR_RPTS, MIN_CREDIT_BUFFER_ALLOC_SIZE,__DATE__,
> __TIME__);
> ./sboot/magpie_1_1/sboot/athos/src/athos_main.c:#define
> ATH_DATE_STRING __DATE__" "__TIME__
> ./sboot/magpie_1_1/sboot/inc/magpie/rom_cfg.h:#define ATH_VER_DATES
> __DATE__" "__TIME__
> ./sboot/magpie_1_1/inc/magpie/rom_cfg.h:#define ATH_VER_DATES
> __DATE__" "__TIME__
all of it are in sboot.
> $ grep -riE 'fixme|todo|hack|xxx+|broken' .
> <lots>
this is not relevant for packaging.
--
Regards,
Oleksij
Attachment:
signature.asc
Description: OpenPGP digital signature