[MIR] lerc

Review for Package: src:lerc

[Summary]
https://www.osgeo.org/projects/lerc-limited-error-raster-compression/

LERC is an open-source image or raster format which supports rapid encoding
and decoding for any pixel type (not just RGB or Byte). The codebase seems
mature and didn't see many updates recently. It does provide an internal
test-suite that is being run at build time, using the just build binaries and
during autopkgtest using the installed library version.

MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: liblerc3, python3-lerc
Specific binary packages built, but NOT to be promoted to main: liblerc-dev

Notes:
#0 Big endian architectures (s390x) are not supported upstream.
#1 The -dev package contains a static build of the library.
   It should therefore be excluded from the promotion
#2 Requesting security review because it's parsing image/TIFF data

Required TODOs:
#3 d/rules uses "-c0" in dpkg-gensymbols/dh_makeshlibs, ignoring the symbols
   check. Why is this? Please explain, or fix this behavior. It should fail
   the build as usual, if symbols change.

Recommended TODOs:
#4 Lintian: lerc source: package-uses-old-debhelper-compat-version 12
   dh-compat should be updated or at least a bug should be reported against
   the Debian package
#5 SetuptoolsDeprecationWarning: setup.py install is deprecated.
   This deprecation warning should be fixed or at least a bug should be reported
   against the Debian package

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - SRCPKG checked with `check-mir`
  - all dependencies can be found in `seeded-in-ubuntu` (already in main)
  - none of the (potentially auto-generated) dependencies (Depends
    and Recommends) that are present after build are not in main
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems:
- exclude liblerc-dev, as it includes a statically linked library

[Embedded sources and static linking]
OK:
- no embedded source present
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard

Problems:
- statically linked library inside the -dev package, built in parallel to the
  dynamic library in "build-static" directory from debian/rules

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does parse data formats (image/TIFF data)

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build up...

This can enter security queue while Desktop considers the remaining open required todos.
Assigning.

Also setting the milestone matching the requested 22.10 feature freeze deadline in August.

#1 ack

#2 thanks!

#3 asked about the symbol to Debian on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013146 which they closed

'No, because C++ libraries have frequent disappearing symbols that are not ABI breaks. See:

 https://wiki.debian.org/UsingSymbolsFiles#C.2B-.2B-_libraries

The package uses pkgkde-symbolshelper that has better support for C++:

 https://qt-kde-team.pages.debian.net/symbolfiles.html

We don't want the build to fail during transitions just because some template was optimized out for example.

Kind Regards, '

#4 compat level report to Debian, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013147 which they responded by

'
Compat level 12 is recommended for packages maintained in the Debian GIS team:

 https://debian-gis-team.pages.debian.net/policy/policy.html#debhelper

We don't track the latest to ease backports for UbuntuGIS and OSGeoLive.

See:

 https://lists.debian.org/debian-gis/2021/09/msg00009.html'

Would you want us to carry a Delta in Ubuntu for it? (I would prefer not)

#5 reported deprecation warning to Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013148

Which got that reply

'This is not a problem.

Kind Regards,

Bas

PS. Please stop filing bugreports for minor cosmetic issues. '

:-(

#1 & #2 -> OK!
#4 & #5 -> Those are only recommended TODOs and won't block the MIR, thanks for pestering the DM about it, though. Those will most probably come back to us eventually, but it's fine for now!

#3: Thanks, I wasn't aware of pkgkde-symbolshelper!
So at least they are using some advanced tools to track their (C++) symbols.
The problem I see with this is that during testing it does not detect an API/ABI breakage, that I introduced on purpose. E.g. if I rename the "lerc_encodeForVersion" API to "lerc_encodeForVersionX", dpkg-buildpackage still passes the build, displaying a warning at least:
dh_makeshlibs -- -v3.0 -c0
dpkg-gensymbols: warning: some new symbols appeared in the symbols file: see diff output below
dpkg-gensymbols: warning: debian/liblerc3/DEBIAN/symbols doesn't match completely debian/liblerc3.symbols
--- debian/liblerc3.symbols (liblerc3_3.0_amd64)
+++ dpkg-gensymbolsaQREWf 2022-06-21 15:22:43.440908065 +0000
@@ -382,4 +382,5 @@
  lerc_decodeToDouble@Base 3.0
  lerc_encode@Base 3.0
  lerc_encodeForVersion@Base 3.0
+ lerc_encodeForVersionX@Base 3.0
  lerc_getBlobInfo@Base 3.0

But "lerc_encodeForVersion" is still there as a symbol, even though I removed/renamed it from the public header and implementation and the build doesn't fail (which is what I would expect in such case)... So I wonder if those tools are being used correctly.

Re-reading the MIR rules, we only ask for "symbols tracking is in place", not that it needs to fail the build... so this could be a tentative MIR team ACK.

But let me check back with the other MIR team members (while waiting for security-review anyway), just to be sure that this is what was intended here.

PS: C++ libraries seem to be a special case anyway, here's an interesting read:
https://wiki.debian.org/UsingSymbolsFiles#C.2B-.2B-_libraries

After consultation with fellow MIR team members, we agreed upon requiring the build to fail on API/ABI changes and that mere "tracking" of symbols is not enough.

Seb, could you please make sure changed/removed symbols are correctly detected during the lerc build and make the build fail if this happens? Currently, removing (renaming) a symbol doesn't seem to be detected at all, see example given in the previous comment.

IMO this could either be done by dropping the "pkgkde_symbolshelper" and the "dh_makeshlibs" override. Or maybe pkgkde_symbolshelper can be proberly configured and combined with a dh_makeshlibs -c1 (or bigger) setting to make it work.

Security-review can still go on in parallel.

Thanks.

We removed the dpkg-gensymbols override in https://launchpad.net/ubuntu/+source/lerc/3.0+ds-1ubuntu1

I reviewed lerc 3.0+ds-1ubuntu1 as checked into kinetic as well as upstream's 4.0.0. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

Please note that LERC 4.0.0 was released after MIR process began (2022-06-15).

> LERC is an open-source image or raster format which supports rapid encoding and decoding for any pixel type (not just RGB or Byte). Users set the maximum compression error per pixel while encoding, so the precision of the original input image is preserved (within user defined error bounds).

- CVE History:
  - none
- Build-Depends?
  - C/C++
    - ld-linux-x86-64.so.2
    - libgcc_s.so.1
    - libm.so.6
    - libstdc++.so.6
    - linux-vdso.so.1
  - Python
    - numpy
    - ctypes
    - timeit
    - platform
    - os
- pre/post inst/rm scripts?
  - yes, generated by dh_python3
  - postint: compiles python3-lerc
  - prerm: attempts to remove python3-lerc
  - tested install and removal
- init scripts?
  - none
- systemd units?
  - none
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - none
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - has build tests and autopkgtests
- cron jobs?
  - none
- Build logs:
  - only setup.py being deprecated as previously mentioned

- Processes spawned?
  - none
- Memory management?
  - 86 memcpy calls, 1 malloc call
  - memcpy and malloc use looks good
  - defensive memory management tools are provided by functions of this library
    - see lerc_computeCompressedSize to help memory allocation
- File IO?
  - only for tests
- Logging?
  - none for C/C++, some commented out print lines
  - Python has some error messages and a verbose flag
- Environment variable usage?
  - none
- Use of privileged functions?
  - none
- Use of cryptography / random number sources etc?
  - none
- Use of temp files?
  - none
- Use of networking?
  - no, only js files not including in MIR
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - js results ignored
  - a minor bitshift error edgecase in private method
    - bitshift never errored as written
    - upstream addressed concern immediately with https://github.com/Esri/lerc/pull/224
- Any significant shellcheck results?
  - none
- Any significant bandit results?
  - none

LercTest/main.cpp and _lerc.py contain "TestLegacyData" which includes unshared files and a specific directory setup ('D:/GitHub/LercOpenSource_v2.5/testData/'). It would be nice if upstream moved this to ./testData and our test suite was expanded.

Upstream can make optional or remove timer() from _lerc.py when debug/performance information is not needed. https://github.com/Esri/lerc/issues/221

@seb128, thanks for reporting @slyon's #5. Deprecating setup.py would be nice to see.

Security team ACK for promoting lerc to main liblerc3 and python3-lerc. Packages using ./OtherLanguages/CSharp/ and ./OtherLanguages/js/ are not acknowledged for main and require a separate MIR.

The version of libtiff5 in kinetic-proposed (4.4.0-4ubuntu2) depends on liblerc3.

ricotz mentioned yesterday on IRC that an i386 build is needed.

it's building on i386 now

which ended up depwaiting on python3-numpy, we will build libtiff with lerc on i386 instead

Updated tiff: https://launchpad.net/ubuntu/+source/tiff/4.4.0-4ubuntu3

And I forwarded our request to Debian to not build tiff with lerc on i386
https://bugs.debian.org/1017958

All required TODOs have been deal with. The situation of i386 is clear now too. +1 from the MIR team as discussed on IRC.

Override component to main
lerc 3.0+ds-1ubuntu1 in kinetic: universe/misc -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic amd64: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic arm64: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic armhf: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic ppc64el: universe/libdevel/optional/100% -> main
liblerc-dev 3.0+ds-1ubuntu1 in kinetic riscv64: universe/libdevel/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic amd64: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic arm64: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic armhf: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic ppc64el: universe/libs/optional/100% -> main
liblerc3 3.0+ds-1ubuntu1 in kinetic riscv64: universe/libs/optional/100% -> main
Override [y|N]? y