Bug#538067: RFS: opencpn
Hi,
an update on ITP progress for the OpenCPN software (opencpn.org).
Before Anton formally requests the next review, I though I'd take care of
commenting on and clearing out as much of the old business as I can.
the debian/ dir can be viewed here:
http://anonscm.debian.org/viewvc/pkg-grass/packages/opencpn/trunk/
the .orig tarball with the dll and exe ms-windows port binary stuff removed
can be found here:
http://anonscm.debian.org/viewvc/pkg-grass/packages/opencpn/tarballs/
On Feb 8, 2011, Paul Wise wrote:
> > macsercomm.cpp looks slightly suspect. I'd like to know its lineage,
> > copyright and license. Same for macutils.c
[+]
> > sercomm.cpp macsercomm.h macutils.h
> > sercomm.h
Hamish replied:
> src/about.cpp lists seriallib as being GPL.
> We can request better header comments.
requested, waiting.
http://www.opencpn.org/flyspray/index.php?do=details&task_id=442
Hamish:
> Convincing OpenCPN, Xtide, ZyGrib, and GMT, ..? to all consolidate on the
> same world vector coastline data remains a TODO (I'd humbly suggest
> focusing on GMT's version(s) of GSHHS as the standard), but remains beyond
> the scope of this [OpenCPN] packaging effort.
I notice others have the same idea,
http://wiki.debian.org/DebianScience/Meteorology (bottom of the page)
http://wiki.debian.org/DebianScienceGshhsMaps
see also earlier discussions on the debian-gis mailing list (pkg-grass@alioth actually)
for now OpenCPN depends on the xtide-coastline package for its needs.
> "dpkg-shlibdeps: warning: dependency ... needlessly depends on ..."
we suspect these are false positives from WxWidgets/cmake.
On May 1, 2011, Paul Wise wrote:
> Generally one should build and test using the suite you are uploading
> to. Most uploads go to sid so they should be built and tested there.
I now have installed a sid VM for that.
Frankie encountered an assembly build error in a previous beta but it
builds without fault for me. Dave suggested that __INTEL__ might trigger
it; but I only have AMD to test on so I can't confirm.
> > The cmake rules are patched to use the system's zlib, bzip2 if UNIX.
...
> Fair enough. Please remove the embedded code copies before
> debian/rules build (or configure) so that the Debian package can never
> build against it by accident.
mmph, IMO .orig should mean "orig", and the .diff should take care of
the cleanup needed for Debianization. (the exception being dfsg
incompatible stuff) Lintian will alert us if a change in the cmake rules
re-embed the libs. (which are supplied for building the MS-Windows/Mac ports)
> Please ask the OpenCPN folks to get their GDAL changes merged
> upstream, maybe it will become suitable for OpenCPN use as a result.
AFAIU, the changes are not considered useful/desired for others, so
intentionally not pushed upstream to GDAL.
> > The tide and current harmonic data [...]
> Would it be possible to merge those changes into the xtide version?
It would be nice, but probably not. They have their own ideas and goals,
and have implemented them. AFAIU OpenCPN has decided something else is
needed, and to take it on their own path with that.
> Lintian has improved a lot since squeeze, always use the
> sid/experimental/backports version.
ok, sid's lintian now reports:
E: opencpn-data: helper-templates-in-copyright
E: opencpn: embedded-library usr/bin/opencpn: tinyxml
E: opencpn: helper-templates-in-copyright
E: opencpn-doc: helper-templates-in-copyright
tinyxml: not sure, exists to be embedded? suggestions to fix that welcome.
helper-templates-in-copyright x3: I don't see what it's talking about, the
debian/copyright files are custom crafted.
compiler warnings:
upstream supplied with latest sid build log, nearly almost all squashed.
valgrind analysis:
status unknown. (personal ignorance on my part of what's been done)
> Hamish, you should add yourself to the Uploaders, or use `dch --team`
> to indicate a GIS team upload.
done, but this is still Anton's to lead, working under the guise of the
DebianGIS team (the control file's maintainer of note).
We can worry about that in the final hours before formal submission to the
FTP masters.
> override_dh_installman shouldn't be needed, best create a
> debian/opencpn.manpages file listing the manual page. It should
> probably also reference the HTML documentation in the SEE ALSO
> section.
done, done, & done.
> I personally like to wrap the Build-Depends line one item per line
> so that changes to them are easier to diff.
In general I heartily agree, and if it wrapped onto a third line I'd do
it. But the current two <80 column lines satisfy the easy-clarity req's
IMO and so I've left it as-is.
> "cross-platform" isn't relevant in the context of Debian package descriptions.
ok, gone.
> The package descriptions are duplicates of one another.
ok, reduced.
> Why are you depending on libgps19 manually? Dependencies on libraries
> should only be done via the shlibs mechanism.
yeah I know. It's there as an explicit reminder that there'll likely be
trouble when the upcoming libgps20 is transitioned into sid.
> Your Standards-Version is out of date, please read the upgrading
> checklist from the version of debian-policy in sid and do any required
> changes: /usr/share/doc/debian-policy/upgrading-checklist.txt*
-> TODO
(want to take that one Anton?)
> You can remove the extra whitespace lines at the end of debian/control.
> No need for the blank line in debian/watch.
ok, both gone.
> The watch file doesn't work, please replace it with this:
done.
> I'm not sure what the letters mean in the upstream version numbers but
> if they mean alpha then I suggest this instead since Debian mainly
> prefers upstream blessed as stable versions:
currently we are packaging late betas for the upcoming (this month?) stable
release. After that we'll track the stable releases for official packages.
> Please write a get-orig-source debian/rules target since the watch
> file does not find the tarball for the version listed in
> debian/changelog.
An .orig tarball with the dll and exe ms-windows port binary stuff removed
can now be found here:
http://anonscm.debian.org/viewvc/pkg-grass/packages/opencpn/tarballs/
It is likely this area can be further automated.
I would still aim the watch file at the official SourceForge site.
Props to Frankie for helping with this.
> debian/changelog does not close the ITP.
I consider that a last-minute task for when we have passed review and are
ready to submit for upload.
> The upstream README file contains information about building and
> dependencies, which is not useful to people installing the binary
> package. Please get upstream to split that out into README.install so
> that it can be shipped in the source package but not the binary
> package. Some of the path names may need tweaking for Debian too (/usr
> vs /usr/local etc).
presumably if debian users are reading that, they already have it built
and the non-conforming info is irrelevant to them.
I agree that splitting the file into INSTALL and README would be a good idea.
> Please add DEP-3 compliant headers to the patches:
> http://dep.debian.net/deps/dep3/
done.
> Please alter the patches until they are acceptable for and included upstream.
I think the only one left which isn't wholly about conforming to Debianisms
is the location of the plugins lib dir. Suggested on the upstream forums but
no feedback to report yet --anyway, low priority and the difference is noted
in the README.Debian file.
> plugins/grib_pi/CMakeLists.txt seems to include Windows line endings, eww.
reported upstream:
http://www.opencpn.org/flyspray/index.php?do=details&task_id=420
> I personally use this in my ~/.quiltrc to generate nice looking
> patches with less cruft:
> QUILT_REFRESH_ARGS="-pab --no-timestamps --no-index"
I'm a big fan of as many breadcrumbs/much metadata as humanly possible,
and abhor throwing away data which may help to work backwards later on, as usually I need all the clues that I can get.
> I personally like to wrap debian/menu one item per line to make diffs
> easier to read.
agreed, done.
> debian/README.harmonics seems to duplicate data/doc/README.harmonics,
> please merge the extra info in the debian one upstream.
It replaces it, see the last line of the rules file.
The debian/ version goes into a little more history/detail to explain
how it fits in with other debian packages.
> Please add --parallel to the invocation of dh so that opencpn can be
> built using parallelism.
I tried that on my 6-core Squeeze box, but it did not change build time
at all, so I've left it "clean". It would be nice to have working though.
??
> You can drop lines 2-9 from debian/rules (the comments) and the extra
> whitespace.
boilerplace stuff removed.
> The xpm file can be installed by dh_install, just edit
> debian/opencpn.install to add this
done.
> It would be nice if override_dh_install/override_dh_installdocs wasn't
> nessecary,
done/removed.
> When building the package I noticed that the gcc command-lines had
> both -O2 and -O3.
-> TODO
(Dave?)
> When building twice in a row a new patch is added
> (debian-changes-2.4.423-1), looks like this is caused by
> include/version.h not being cleaned up by debian/rules clean, please
> fix the upstream build system so it does that
done upstream.
> Why is the MacOS X icon completely different to the normal OpenCPN logo?
no idea, but now it's gone in our +dfsg.orig tarball.
> Please remove the prebuilt Windows executables from the source tarball
done. (now we make a stripped +dfsg.orig.tar.gz in alioth svn)
> and git repository.
suggested in upstream forums.
> data/doc/images/print.html seems to be a 404 page, please remove it.
filed upstream:
http://www.opencpn.org/flyspray/index.php?do=details&task_id=538
> data/doc/css/ can probably be used since nothing in the package uses it.
> Please remove data/doc/js/. Nothing in the package uses it and it is a
> embedded code copy that is minified and therefore is missing the
> source code.
I was under the assumption that it was being used.
(Dave?)
> A lot of the source code contains CVS $Id lines while the package is
> in git, I would suggest to clean those up, especially the CPL_CVSID
> ones.
Filed upstream as FS#539.
> The upstream README file (and many other files) has the executable bit
> set, why is that?
Filed upstream as FS#420 (see above about DOS newlines)
> The following scripts look like they should be run during the build
> process so that any changes to the SVG files are reflected in the
> bitmaps. Alternatively they could be merged into the CMake build
> system.
>
> plugins/grib_pi/src/icons.sh
> plugins/dashboard_pi/src/icons.sh
> src/bitmaps/32x32_svg_src/cursor/create_all_32x32.sh
> src/bitmaps/32x32_svg_src/ribbon/create_all_32x32.sh
> src/bitmaps/13xX_svg_src/create_all_13xX.sh
> src/bitmaps/28x28_svg_src/create_all_28x28.sh
> src/bitmaps/optimize_png.sh
> src/bitmaps/other_svg_src/create_opencpn_main_icon.sh
> src/bitmaps/other_svg_src/create_ship.sh
> src/bitmaps/16x16_svg_src/create_all_16x16.sh
> src/bitmaps/create_all.sh
src/bitmaps/README.TXT says:
"""
1. Run create_all.sh
Creation scripts require:
ImageMagick
Inkscape
Optipng
Icoutils
png2wx.pl
"""
avoiding those as build-deps seems to outweigh the possibility of stale
xpm/png files. FWIW all generated files seem younger than 8 weeks, so I
suspect are kept up to date with regularity.
> What is the license for src/bitmaps/paypal_donate.xpm?
-> TODO
I don't know, but we need to find out as a priority.
> Lintian complaints:
> dpkg-shlibdeps warnings:
> Some gcc and ld warnings:
see comments earlier in this email.
thanks & regards,
Hamish
Reply to: