Re: Packaging lerc
Dear Sebastiaan,
thanks for the review
Il 01/11/21 13:28, Sebastiaan Couwenberg ha scritto:
Some comments based on a fist look.
The code base is C++ and you use a symbols file, but not
pkgkde-symbolshelper.
Symbols for C++ are fragile and tend to change per platform and compiler
version. It's fine to not use symbols for C++ libraries, we do that for
laszip for example. With dh_makeshlibs using -V by default since
debhelper compat 12, the dependencies it generates are quite reasonable.
Using symbols for C++ libraries doesn't have to be very painful, we use
them for pdal for example. But you need take care of a few things, first
is using the upstream version in dependencies instead of the package
version. You need the following in d/rules for that:
include /usr/share/dpkg/pkg-info.mk
UPSTREAM_VERSION = $(shell echo $(DEB_VERSION_UPSTREAM) | sed -e
's/\+.*//')
override_dh_makeshlibs:
dh_makeshlibs -- -v$(UPSTREAM_VERSION) -c0
-c0 is also important to not have dh_makeshlibs fail when the symbols
change as some tend to disappear when built with a newer compiler.
Using the pkgkde_symbolshelper dh helper makes maintaining the symbols
file for all architectures much simpler, refer to the documentation for
details:
https://qt-kde-team.pages.debian.net/symbolfiles.html
When symbols files are used for C++ projects in the Debian GIS team,
this is what we use.
Thanks, I didn't know pkgkde-symbolshelper.
I will give a look to it.
Actually I was not 100% sure to use symbol versioning for this library.
The public API is a "C" API consisting in a very small number of functions.
Would it make sense to just care about those?
Why is the list of architectures restricted in debian/control?
LERC does not support bigendian architectures
debian/copyright doesn't use the standalone license paragraph for the
main Files section.
fixed
Files-Excluded is clearer when each pattern is on its own line.
fixed
SOVERSION patch is not forwarded upstream. Not using a SOVERSION makes
me question the quality of the upstream project.
Patch forwarded now: https://github.com/Esri/lerc/pull/188
kind regards
--
Antonio Valentino
Reply to: