On Tue, Apr 17, 2012 at 12:35 PM, Silvio Cesare wrote:
> The Debian Package clonewise-core (currently in the mentors archive)
> http://mentors.debian.net/package/clonewise-core
> http://www.foocodechu.com/downloads/clonewise
Here is a review of the package:
You should file an ITP bug:
http://www.debian.org/devel/wnpp/#l1
Which debian.net domain would you like me to register on your behalf
and what IP address or domain name should I point it to?
clonewise.debian.net? This would be the location for the results of
running this over the Debian archive.
There are a pair of embedded code copies in the libs/ dir :) snap and
glib should be packaged separately.
All the .ex files in the debian/ dir should be removed or renamed.
Please add a debian/watch file:
http://wiki.debian.org/debian/watch
debian/README.source and debian/README.Debian look like they can be removed.
debian/README looks like it should be renamed to debian/README.Debian
debian/docs is empty and can be removed.
Please remove or fix and uncomment the commented-out Vcs-* fields in
debian/control. The Vcs-* fields should point at the VCS containing
the packaging for clonewise-core.
libfuzzy2 should not be in Depends, since the shlibs mechanism should
automatically add that.
Would it be better to link to this page in the Homepage field?
http://www.foocodechu.com/?q=clonewise-automatically-identifying-package-clones%20and-inferring-security-vulnerabilities
The build process isn't very verbose. You should disable that either
upstream or in debian/rules.
The upstream code doesn't contain per-file copyright and license info:
http://tieguy.org/blog/2012/03/17/on-the-importance-of-per-file-license-information/
Would it be possible to either rename the binaries to not include a
capital letter or include symlinks? Lower-case is generally easier to
type.
You mentioned on IRC that ssdeep needs to be in Depends, please add it.
When I run Clonewise-BuildDatabase, I get a lot of this warning, would
it be possible to silence that?
ssdeep: Did not process files large enough to produce meaningful results.
Would it be possible to make Clonewise-BuildDatabase use a local
Debian mirror instead of run apt-get source lots of times? When we
start running it on the Debian archive it would be best to do it that
way.
The package is a native package while it should not be one since it
might be useful to any Debian derivative. I would also encourage you
to structure your upstream development so that the core code is
flexible enough to also support other packaging systems, like those of
Fedora, Gentoo etc and provide package-format-specific parts to do the
right thing on each package format type.
Your upstream build system does not support setting the install prefix
and hardcodes /usr instead of /usr/local. People installing from
source will get non-packaged files in /usr.
Some configuration or command-line options would be useful so that the
database can be located at any path and the Clonewise-BuildDatabase
script does not need to be run as root.
Any particular reason to use -O3? I imagine the performance of
clonewise would mostly be limited by IO and by the speed of ssdeep and
that the compiler optimisation level would make no difference. In
general it is better to use -O2.
Since you are upstream, please take a look at our upstream guide and
the links in the External advice section:
http://wiki.debian.org/UpstreamGuide
lintian complaints:
W: clonewise-core source: dh-make-template-in-source
debian/clonewise-core.cron.d.ex
W: clonewise-core source: dh-make-template-in-source
debian/clonewise-core.default.ex
W: clonewise-core source: dh-make-template-in-source
debian/clonewise-core.doc-base.EX
W: clonewise-core source: dh-make-template-in-source debian/emacsen-install.ex
W: clonewise-core source: dh-make-template-in-source debian/emacsen-remove.ex
W: clonewise-core source: dh-make-template-in-source debian/emacsen-startup.ex
W: clonewise-core source: dh-make-template-in-source debian/init.d.ex
W: clonewise-core source: dh-make-template-in-source debian/manpage.1.ex
W: clonewise-core source: dh-make-template-in-source debian/manpage.sgml.ex
W: clonewise-core source: dh-make-template-in-source debian/manpage.xml.ex
W: clonewise-core source: dh-make-template-in-source debian/menu.ex
W: clonewise-core source: dh-make-template-in-source debian/postinst.ex
W: clonewise-core source: dh-make-template-in-source debian/postrm.ex
W: clonewise-core source: dh-make-template-in-source debian/preinst.ex
W: clonewise-core source: dh-make-template-in-source debian/prerm.ex
W: clonewise-core source: dh-make-template-in-source debian/watch.ex
P: clonewise-core source: unversioned-copyright-format-uri
http://dep.debian.net/deps/dep5
W: clonewise-core source: missing-license-text-in-dep5-copyright
gpl-3.0+ (paragrah at line 5)
W: clonewise-core source: out-of-date-standards-version 3.9.2 (current is 3.9.3)
I: clonewise-core: spelling-error-in-binary usr/bin/Clonewise writting writing
I: clonewise-core: spelling-error-in-binary usr/bin/Clonewise lenght length
W: clonewise-core: readme-debian-contains-debmake-template
I: clonewise-core: description-synopsis-might-not-be-phrased-properly
W: clonewise-core: binary-without-manpage usr/bin/Clonewise
W: clonewise-core: binary-without-manpage usr/bin/Clonewise-BuildDatabase
W: clonewise-core: binary-without-manpage usr/bin/Clonewise-ParseDatabase
W: clonewise-core: binary-without-manpage usr/bin/Clonewise-RunTests
gcc warnings:
In file included from libs/snap/../glib/base.cpp:67:0,
from libs/snap/Snap.cpp:6:
libs/snap/../glib/os.cpp: In static member function 'static TTm
TSysTm::GetTmFromMSecs(const uint64&)':
libs/snap/../glib/os.cpp:631:52: warning: integer overflow in
expression [-Woverflow]
In file included from libs/snap/../glib/base.cpp:86:0,
from libs/snap/Snap.cpp:6:
libs/snap/../glib/html.cpp: In static member function 'static TStr
THtmlLxChDef::GetCSZFromWin1250(const TChA&)':
libs/snap/../glib/html.cpp:137:12: warning: case label value exceeds
maximum value for type [enabled by default]
libs/snap/../glib/html.cpp:138:12: warning: case label value exceeds
maximum value for type [enabled by default]
libs/snap/../glib/html.cpp:139:12: warning: case label value exceeds
maximum value for type [enabled by default]
libs/snap/../glib/html.cpp:140:12: warning: case label value exceeds
maximum value for type [enabled by default]
libs/snap/../glib/html.cpp:141:12: warning: case label value exceeds
maximum value for type [enabled by default]
libs/snap/../glib/html.cpp:142:12: warning: case label value exceeds
maximum value for type [enabled by default]
In file included from libs/snap/Snap.cpp:8:0:
libs/snap/../glib/linalg.cpp: In static member function 'static void
TNumericalStuff::EigSymmetricTridiag(TFltV&, TFltV&, int, TFltVV&)':
libs/snap/../glib/linalg.cpp:652:87: warning: deprecated conversion
from string constant to 'char*' [-Wwrite-strings]
libs/snap/../glib/linalg.cpp: In static member function 'static void
TNumericalStuff::CholeskyDecomposition(TFltVV&, TFltV&)':
libs/snap/../glib/linalg.cpp:705:34: warning: deprecated conversion
from string constant to 'char*' [-Wwrite-strings]
libs/snap/../glib/linalg.cpp: In static member function 'static void
TNumericalStuff::LUDecomposition(TFltVV&, TIntV&, double&)':
libs/snap/../glib/linalg.cpp:820:77: warning: deprecated conversion
from string constant to 'char*' [-Wwrite-strings]
dpkg-gencontrol warning:
dpkg-gencontrol: warning: package clonewise-core: unused substitution
variable ${python:Depends}
cppcheck warning:
[libs/snap/linkpred.cpp:309]: (error) printf format string has 2
parameters but only 1 are given
pyflakes warning:
bin/Clonewise-ParseDatabase:4: 'os' imported but unused
--
bye,
pabs
http://wiki.debian.org/PaulWise