[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#840515: RFS: open-ath9k-htc-firmware/1.4.0-25-gf6af791-1 ITP #751339



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?

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.

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.

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?

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

$ 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

$ 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)

$ 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__

$ grep -riE 'fixme|todo|hack|xxx+|broken' .
<lots>

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


Reply to: