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

Bug#764261: RFS: librocket/1.3-1 [ITP]



On Mon, 2014-10-06 at 22:13 -0400, Harlan Lieberman-Berg wrote:

> Thanks for packaging librocket for Debian.  I've got a couple things for
> you to fix as the next steps.

Good review Harlan, you missed some important things though, see below.

> 2. Your changelog should close an ITP bug - in your case, #764252.

You can do that by adding this in your changelog:

(Closes: #764252)

> 5. Your standards version isn't the latest version - you should update
> it to 3.9.6.

Here is the list of changes since the last version:

https://www.debian.org/doc/debian-policy/upgrading-checklist

> 6. Your packages should have a short description and a longer
> description that should be enough information for a user to decide
> whether they want to install the package.  Check out Debian Policy 3.4
> for more details there.

https://www.debian.org/doc/debian-policy/ch-binary.html#s-descriptions

These issues block the upload of this package:

The package fails to build for me:

 debian/rules build
dh build --buildsystem=cmake --sourcedirectory=Build 
   dh_testdir -O--buildsystem=cmake -O--sourcedirectory=Build
   debian/rules override_dh_auto_configure
make[1]: Entering directory '/tmp/buildd/librocket-1.3'
dh_auto_configure -- -DBUILD_PYTHON_BINDINGS=On
-DCMAKE_INSTALL_LIBDIR=/usr/lib
dh_auto_configure: cmake ../Build -DCMAKE_INSTALL_PREFIX=/usr
-DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=None
-DBUILD_PYTHON_BINDINGS=On -DCMAKE_INSTALL_LIBDIR=/usr/lib failed to to
execute: No such file or directory
debian/rules:11: recipe for target 'override_dh_auto_configure' failed
make[1]: *** [override_dh_auto_configure] Error 2
make[1]: Leaving directory '/tmp/buildd/librocket-1.3'
debian/rules:8: recipe for target 'build' failed
make: *** [build] Error 2

The python package should be named python-rocket:

https://www.debian.org/doc/packaging-manuals/python-policy/ch-module_packages.html#s-package_names

The .so symlink should be installed in the -dev package.

The library package should be named librocketN

Please read the Debian policy sections on libraries:

https://www.debian.org/doc/debian-policy/ch-sharedlibs.html

Some parts (in Samples/tutorial) of the upstream tarball do not have a
clear license. The copyright holder is also different to the rest of the
package. Some other parts (Samples/) have a DFSG-free license but a
different copyright holder. The status should be clarified upstream, a
DFSG-free license applied and the results documented in
debian/copyright.

The fonts in Samples/assets are under a non-free license and cannot be
distributed in Debian main. Please ask upstream to remove them from the
tarball and from their version control system. They can simply use
"font-family: sans-serif" in invader.rcss instead.

http://www.exljbris.com/delicious.html
http://www.exljbris.com/eula.html

Some other issues that you might want to correct:

librocket.pc has an incorrect Version and Description.

Please forward the patches upstream if appropriate.

Please add a debian/watch file based on the example and the docs:

https://wiki.debian.org/debian/watch

debian/README.source is a copy of part of the upstream readme.md and is
not needed.

The static library normally isn't needed, please remove it unless
someone files a bug report asking for it to be added.

The pkg-config file doesn't appear to be installed in the package.
https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
how_to_build_for_mingw.txt is not relevant to Debian users, please drop
it from debian/docs.

changelog.txt should be installed with dh_installchangelogs rather than
dh_installdocs.

debian/copyright doesn't look like it conforms to the format. In
particular the licenses are missing dots on the blank lines. In
addition, you can avoid having two copies of the license by having one
license section separated from the files sections and have those files
sections refer to the files sections. The long lines of the license text
should also be wrapped.

http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/

I wonder about the *.tga files in Samples/*invaders/data and
Samples/assets, some of them look like they might have other files (SVG
or similar) as their source. Please ask upstream to include the SVG or
other source in the tarball and have the build system create the *.tga
files at build time.

If the sample games are fun it might be interesting to make packages for
them. If not, their source could be included in an examples package.

Automatic checks:

https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git

$ cme check dpkg
Warning in 'control source Build-Depends:0' value 'debhelper (>= 8.0.0)': should be (>= 9) not (>= 8.0.0) because compat is 9
Warning in 'control source Standards-Version' value '3.9.4': Current standards version is 3.9.6
Warning in 'control binary:"librocket-dev" Depends:0' value 'librocket (= ${binary:Version})': package librocket is unknown. Check for typos if not a virtual package.
Use of uninitialized value $desc in concatenation (.) or string at /usr/share/perl5/Config/Model/Backend/Dpkg/Control.pm line 171.
Warning: skipping value  because of the following errors:
Undefined mandatory value.

Warning in 'control binary:"librocket-python" Depends:0' value 'librocket (= ${binary:Version})': package librocket is unknown. Check for typos if not a virtual package.
Use of uninitialized value $desc in concatenation (.) or string at /usr/share/perl5/Config/Model/Backend/Dpkg/Control.pm line 171.
Warning: skipping value  because of the following errors:
Undefined mandatory value.

Use of uninitialized value $desc in concatenation (.) or string at /usr/share/perl5/Config/Model/Backend/Dpkg/Control.pm line 171.
Warning: skipping value  because of the following errors:
Undefined mandatory value.

File debian/copyright line 11 has a syntax error:
	Did not find a keyword before: 'The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.''

$ codespell --quiet-level=3
<lots of typos

$ cppcheck -j1 --quiet -f .
[Samples/basic/sfml/src/RenderInterfaceSFML.cpp:305]: (error) Mismatching allocation and deallocation: buffer
[Samples/basic/sfml/src/RenderInterfaceSFML.cpp:310]: (error) Mismatching allocation and deallocation: buffer
[Samples/basic/sfml2/src/RenderInterfaceSFML.cpp:278]: (error) Mismatching allocation and deallocation: buffer
[Samples/basic/sfml2/src/RenderInterfaceSFML.cpp:283]: (error) Mismatching allocation and deallocation: buffer
[Samples/shell/src/ShellFileInterface.cpp:45]: (error) Resource leak: fp
[Samples/shell/src/ShellFileInterface.cpp:49]: (error) Resource leak: fp
[Source/Core/Python/Converters.cpp:205]: (error) Instance of 'StringConverter' object is destroyed immediately.
[Source/Core/Python/Converters.cpp:206]: (error) Instance of 'VariantConverter' object is destroyed immediately.
[Source/Core/Python/Converters.cpp:210]: (error) Instance of 'EventListenerFromPython' object is destroyed immediately.
[Source/Debugger/ElementInfo.cpp:403]: (error) Same iterator is used with different containers 'properties' and 'second'.
<possibly more, I killed it>

$ fdupes -q -r .
<lots of duplicate files>

$ 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 of suggested additions/removals>

$ pep8 --ignore W191 .
<lots of warnings>

$ pyflakes .
./bin/rocket.py:1: 'from _rocketcore import *' used; unable to detect undefined names
./bin/rocket.py:4: 'from _rocketcontrols import *' used; unable to detect undefined names

$ find -type f -iname '*.sh' -exec sh -n {} +
./Build/cmake/gen_samplelists.sh: 9: ./Build/cmake/gen_samplelists.sh: Syntax error: "(" unexpected

$ find -type f -iname '*.sh' -exec shellcheck {} +
<lots of warnings about Build/cmake/gen_filelists.sh>













-- 
bye,
pabs

https://wiki.debian.org/PaulWise

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: