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

Re: Packaging lerc



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.

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.

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.

Kind Regards,

Bas

--
 GPG Key ID: 4096R/6750F10AE88D4AF1
Fingerprint: 8182 DE41 7056 408D 6146  50D1 6750 F10A E88D 4AF1


Reply to: