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

Re: Packaging lerc



Dear Sebastiaan,

Il 01/11/21 18:52, Sebastiaan Couwenberg ha scritto:
On 11/1/21 18:27, Antonio Valentino wrote:
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?

That sounds similar to proj which got a lot more C++ symbols since 6.0.0, it switched to pkgkde-symbols since 6.0.0~rc1 for that reason.

As long as the public API doesn't change, you likely won't have to worry about C++ symbols going missing.

understood

Why is the list of architectures restricted in debian/control?


LERC does not support bigendian architectures

Might be better to use Architecture: any and just have the build fail on those architectures, any new LE architectures won't need changes to the architecture list.

My problem is that currently in the list of supported architectures there is at least one that is BE (s390x). As far as I can understand lerc will never be able to migrate to testing if we use Architecture: any.


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

You may want to start with SOVERSION 0, that's more common.

If upstream is unsure when to bump the SOVERSION, having it match the major version is reasonable assuming they adhere to semver.

Uploading the package now and having to deal with a lower SOVERSION when upstream gets its act together won't be fun.

Yes, I agree.
Probably it is better to wait few days to see ig there is some feedback form upstream.

Otherwise I will use soversion 0 for the debian package, just to be on the safe side.


kind regards
--
Antonio Valentino


Reply to: