Bug#860145: RFS: openscap-daemon/0.1.6-1 [ITP] -- SCAP security policy compliance daemon for a complete infrastructure
On Wed, Apr 12, 2017 at 3:10 PM, Philippe Thierry wrote:
> https://mentors.debian.net/debian/pool/main/o/openscap-daemon/openscap-daemon_0.1.6-1.dsc
I don't intend to sponsor this, but here is a quick review:
Please encourage upstream to port the project to Python 3, we are
getting closer to the point where Python 2 may be unsupported in some
distributions.
The Debian systemd service file has an incompatible name with upstream
but is identical. Please remove the Debian copy and just use the
upstream systemd service file. If you want to add the full name
upstream, IIRC systemd supports service aliases so you could talk
upstream into adding an alias.
There are typos in debian/README.Debian, please use a spell checker on it.
I think the sysvinit-start-stop-daemon test should use the service
script instead of directly running the init scripts.
I don't think mentioning DEP-3 in patch headers is useful, thanks for
following DEP-3 though.
Please try to get all the patches included upstream or fix upstream so
they are not needed.
ISTR sysvinit supports initially-disabled services, so I think you
should drop OSCAPD_START and the /etc/default/openscap-daemon file and
use that instead if possible.
The short Description is slightly awkward, I'd suggest running it by
the debian-l10n-english list.
Typos in debian/control:
s/that allow/that allows/
s/check/checks/
s/associated to/associated with/
<missing period at the end>
I would suggest replacing the specific product name 'Docker' in
debian/control with the generic word 'container', unless it doesn't
support checking other container types?
You might want to strip the installation instructions from the
upstream README.md file before installing that into the binary
package, since users of the binary packages do not need install
instructions.
debian/openscap-daemon-docs.docs mentions a README.source file but I
can't see that anywhere in the package.
I'd suggest running this command. It will make diffs of the debian/
dir easier to view:
wrap-and-sort --short-indent --wrap-always --sort-binary-packages
--trailing-comma
Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata
Some of the below automated checks are upstream issues, please talk to
upstream about automatically running these tools in their CI systems.
Automated checks:
build:
E: dh_python2 dh_python2:408: no package to act on (python-foo or one
with ${python:Depends} in Depends)
adequate:
openscap-daemon: py-file-not-bytecompiled
/usr/lib/python2.7/dist-packages/openscap_daemon/__init__.py
<lots more>
lintian:
P: openscap-daemon source: debian-watch-may-check-gpg-signature
check-all-the-things:
$ find . -type f -iname '*.sh' -exec checkbashisms {} +
<lots>
$ env PERL5OPT=-m-lib=. cme check dpkg
Warning in 'control source Build-Depends:2' value 'python (>> 2.7)':
unnecessary versioned dependency: python (>> 2.7). Debian has
oldstable -> 2.7.3-4+deb7u1; stable -> 2.7.9-1; testing -> 2.7.13-2;
unstable -> 2.7.13-2;
Warning in 'control binary:"openscap-daemon" Depends:2' value 'python
(>> 2.7)': unnecessary versioned dependency: python (>> 2.7). Debian
has oldstable -> 2.7.3-4+deb7u1; stable -> 2.7.9-1; testing ->
2.7.13-2; unstable -> 2.7.13-2;
Warning in 'source options tar-ignore' value '.git|.*.pyc': There's a
missing element in Dpkg::Source::Option. Please log a bug against
libconfig-model-dpkg-perl mentioning the missing element and its
relevant documentation.
$ codespell --quiet-level=3
<lots>
$ debmake -k
<says some files are LGPL not GPL>
$ fdupes -q -r . | grep -vE
'/(\.(git|svn|bzr|hg|sgdrawer|pc)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s
./atomic-git/f24_spc/Dockerfile
./atomic-git/f23_spc/Dockerfile
./oscapd.service
./debian/openscap-daemon.service
./atomic/f24_spc/install.sh
./atomic/rhel7_spc/install.sh
./atomic/centos7_spc/install.sh
./atomic/f23_spc/install.sh
./atomic/f24_spc/config.ini
./atomic/rhel7_spc/config.ini
./atomic/f23_spc/config.ini
# check if these can be switched to https://
$ grep -rF http: .
<lots>
$ env PERL5OPT=-m-lib=. license-reconcile
<says some files are LGPL not GPL, says there are copyright mismatches>
# These command check style. While a consistent style
# is a good idea, people who have different style
# preferences will want to ignore some of the output.
# Do not bother adding non-upstreamable patches for these.
$ find . -type f -iname '*.py' -exec pep8 --ignore W191 {} +
$ proselint .
$ pydocstyle .
<lots>
$ find . -type f -iname '*.py' -exec pyflakes {} +
./openscap_daemon/cli_helpers.py:54: local variable 'total_width' is
assigned to but never used
./openscap_daemon/compat.py:46: redefinition of unused
'subprocess_check_output' from line 23
$ find . -type f -iname '*.py' -exec pyflakes3 {} +
./openscap_daemon/cli_helpers.py:27: undefined name 'raw_input'
./openscap_daemon/cli_helpers.py:54: local variable 'total_width' is
assigned to but never used
./openscap_daemon/compat.py:46: redefinition of unused
'subprocess_check_output' from line 23
./debian/lib/release.py:33:34: invalid syntax
print 'Loading ReleaseNotes from ' + BASEURL + VERSION
$ find . -type f -iname '*.py' -exec pylint --rcfile=/dev/null
--msg-template='{path}:{line}:{column}: [{category}:{symbol}] {obj}:
{msg}' --reports=n {} +
$ find . -type f -iname '*.py' -exec pylint3 --rcfile=/dev/null
--msg-template='{path}:{line}:{column}: [{category}:{symbol}] {obj}:
{msg}' --reports=n {} +
<lots>
$ 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>
$ vulture .
<lots>
$ cats -c python2-bandit
<lots>
--
bye,
pabs
https://wiki.debian.org/PaulWise
Reply to: