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

Re: RFS: opencpn



On Mon, Feb 8, 2010 at 4:06 PM, Paul Wise <pabs@debian.org> wrote:

> I'll take a closer look at the package when I'm on a Debian system.

I took a much closer look at the package and in summary; I'd really
like to sponsor this package but it needs a lot of work upstream,
especially in the copyright/license department. Please do contact this
list again when you have worked with upstream to resolve the issues
listed below. Should you have further questions, please don't hesitate
to contact this list.


I think upstream is installing the icons in the wrong directory. IIRC
it should be be installed as
/usr/share/icons/hicolor/48x48/apps/opencpn.png for bitmaps of size 48
x 48. XPM is not a good format for modern desktops since it has only
1-bit transparency and therefore no possibility of anti-aliased
transparency. Unfortunately the Debian menu system requires it. I'd
suggest that the .xpm file be installed in /usr/share/pixmaps, IIRC
that has a lower priority in most desktops than /usr/share/icons/.
Also, one of the opencpn.svg files could be shipped as
/usr/share/icons/hicolor/scalable/apps/opencpn.svg


The file 136ReleaseNotes might be relevant to install, in
/usr/share/doc/opencpn perhaps. Hmm, half of it seems to be exactly
the same as the changelog so maybe not. I suggest contacting upstream
about it. Looks like upstream is using the ChangeLog as a NEWS file
but have abandoned the NEWS file after the 1.2.6 release and now store
NEWS entries in the ChangeLog instead. Whee! The NEWS file should
summarise user-visible changes between releases and the ChangeLog file
should be similar to VCS logs. The GNU Coding Standards document has
more detail here:

http://www.gnu.org/prep/standards/standards.html#NEWS-File
http://www.gnu.org/prep/standards/standards.html#Change-Logs


I'm confused, is upstream using both autotools and CMake?


src/grib/bzip2 contains an embedded code copy of bzip2. Please ask
upstream to remove it from the tarball. Same for src/grib/zlib-1.2.3.
Software maintained elsewhere should not be in the tarball. Same for
src/grib.  All 3 are already in Debian:

http://packages.debian.org/zygrib
http://packages.debian.org/zlib
http://packages.debian.org/bzip2

src/mygdal, src/myiso8211, src/nmea0183 all look like embedded code
copies too. If the reason that they are in the tarball is for
operating systems like Windows that don't have good
packaging/repository/dependency systems, I'd suggest making a separate
tarball containing tarballs for all the dependencies. warzone2100 does
this and it seems to work well.


src/nmea0183 has no copyright information and this as a license:

You can use it any way you like.

Unfortunately this does not give Debian permission to copy, modify or
distribute.


Another embedded code copy, Stackwalker.cpp, probably could be
replaced by valgrind.


bbox.cpp looks like it was copied from a really old version of wxWidgets;

http://wxwindows2.4.sourcearchive.com/documentation/2.4.4.1.1ubuntu2/bbox_8cpp-source.html

Surely newer versions of wxWidgets contain similar functionality that
could be used instead of this code copy. There are other files that
look like they are similar; inpcons.h dymemdc.h inphand.h uniparts.h
and anything containing the phrase "Purpose:  Optimized".


The OpenCPN code contains this template:

 *   Copyright (C) $YEAR$ by $AUTHOR$   *
 *   $EMAIL$   *

Upstream really needs to replace these by real information. Some files
have the right info but still have the template. BTW, the template is
wrong since (C) is not a valid symbol for copyright, IIRC only
"Copyright", "Copyr." and "©" are.


src/georef.c has this non-free license (non-commercial use/etc is non-free):

/* Permission  to use, copy,  modify, and distribute  this software and its */
/* documentation for non-commercial purpose, is hereby granted without fee, */
/* providing that the  copyright notice  appear in all copies and that both */
/* the  copyright notice and  this permission  notice appear in  supporting */
/* documentation.  I make no representations about  the suitability of this */
/* software  for any  purpose.  It is  provides "as is" without  express or */
/* implid warranty.                                                         */

Similar for gpc.h georef.h


macsercomm.cpp looks slightly suspect. I'd like to know its lineage,
copyright and license. Same for macutils.c routeprop.cpp
scrollingdialog.cpp sercomm.cpp macsercomm.h macutils.h routeprop.h
scrollingdialog.h sercomm.h tcmgr.h triangulate.h data/s57data/*


data/sounds/alarm2.wav contain these comments:

Copyright . Cinematronics 1995
Microsoft Plus! . for Windows 95

It is highly likely that it is non-free, and I'd wager that alarm1.wav is too.


The source code for HARMONIC.IDX does not seem to be distributed in
the package. It says "This file is automatically generated by
Tide2idx", so source code should exist.


data/wvsdata/ is claimed to be open source, but is a subset. I'd
suggest that it be packaged separately since it is from wxTide. It is
also in a binary format so I wonder how it is supposed to be edited or
what the source code really is.


debian/copyright needs to document all the licenses and copyright
information. You may also want to consider DEP5 (Machine-readable
debian/copyright):

http://dep.debian.net/deps/dep5/


Unfortunately source.debian.net is down otherwise I'd try to find
other packages containing some of the above files.


opencp.cpp is empty, weird.


Normally, .mo files wouldn't be distributed in the source package.


lintian complaints not mentioned:

I: opencpn: arch-dep-package-has-big-usr-share 16884kB 88%
I: opencpn: desktop-entry-contains-encoding-key
/usr/share/applications/opencpn.desktop:4 Encoding
I: opencpn: possible-documentation-but-no-doc-base-registration


dpkg-shlibdeps warnings. Some of these are possibly issues in
wxWidgets so you may not be able to do anything about them short of
filing a bug.

dpkg-shlibdeps: warning: dependency on libwx_gtk2u_richtext-2.8.so.0
could be avoided if "debian/opencpn/usr/bin/opencpn" were not
uselessly linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libwx_gtk2u_html-2.8.so.0 could
be avoided if "debian/opencpn/usr/bin/opencpn" were not uselessly
linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libwx_gtk2u_aui-2.8.so.0 could
be avoided if "debian/opencpn/usr/bin/opencpn" were not uselessly
linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libwx_gtk2u_xrc-2.8.so.0 could
be avoided if "debian/opencpn/usr/bin/opencpn" were not uselessly
linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libwx_gtk2u_qa-2.8.so.0 could
be avoided if "debian/opencpn/usr/bin/opencpn" were not uselessly
linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libGL.so.1 could be avoided if
"debian/opencpn/usr/bin/opencpn" were not uselessly linked against it
(they use none of its symbols).


Some gcc warnings you might want to investigate and forward upstream,
patches preferred of course :)

In file included from src/chart1.cpp:266:
././include/grib.h:83: warning: 'typedef' was ignored in this declaration
src/cutil.c:106: warning: 'cvsid_aw' defined but not used
src/georef.c:59: warning: 'cvsid_aw' defined but not used
src/cm93.cpp: In member function 'bool
cm93compchart::RenderNextSmallerCellOutlines(wxDC*, ViewPort&, bool)':
src/cm93.cpp:4581: warning: suggest parentheses around '&&' within '||'
src/s57chart.cpp: In member function 'void
s57chart::CreateSENCRecord(OGRFeature*, FILE*, int, S57Reader*)':
src/s57chart.cpp:4746: warning: format '%d' expects type 'int', but
argument 3 has type 'size_t'
src/tri.c:125: warning: 'cvsid_aw' defined but not used
src/myiso8211/ddfrecord.cpp: In member function 'int DDFRecord::Write()':
src/myiso8211/ddfrecord.cpp:285: warning: format '%05d' expects type
'int', but argument 3 has type 'size_t'
src/myiso8211/ddfrecord.cpp:289: warning: format '%05d' expects type
'int', but argument 3 has type 'size_t'



Upstream is using CVS, you might want to consider suggesting that they
switch to a more modern VCS like git, bzr or SVN.


In addition, I found some spam on the upstream website:

http://opencpn.org/node/67
http://opencpn.org/node/73
http://opencpn.org/node/72
http://opencpn.org/node/69
http://opencpn.org/node/65
http://opencpn.org/node/68

Some former spam pages still show up in menus:

http://opencpn.org/node/70
http://opencpn.org/node/71
http://opencpn.org/node/66


You have enough work above, but have you also considered packaging
wxTide, CapCode and other FLOSS nautical apps?

-- 
bye,
pabs

http://wiki.debian.org/PaulWise


Reply to: