raise skip count in t/001version.t

[gregoa]

In Debian we are currently applying the following patch to
Compress-Raw-Lzma.
We thought you might be interested in it too.

Description: raise skip count
 2c922d9 added an additional test in the block guarded by
 TEST_SKIP_VERSION_CHECK but didn't bump the number of skipped test when the
 variable is set which leads to
 .
 t/001version.t .......
 1..5
 ok 1 - use Compress::Raw::Lzma;
 ok 2 # skip TEST_SKIP_VERSION_CHECK is set
 ok 3 # skip Not github
 ok 4 # skip Not github
 Dubious, test returned 1 (wstat 256, 0x100)
 Failed 1/5 subtests
         (less 3 skipped subtests: 1 okay)
Origin: vendor
Author: gregor herrmann <gregoa@debian.org>
Last-Update: 2024-02-19

The patch is tracked in our Git repository at
https://salsa.debian.org/perl-team/modules/packages/libcompress-raw-lzma-perl/raw/master/debian/patches/skip-new-test.patch

Thanks for considering,
gregor herrmann,
Debian Perl Group

[pmqs]

Thanks @gregoa -- good catch. I'll create a new release tomorrow with the fix included so you don't need to apply a patch.

Out of interest, why do you need to set TEST_SKIP_VERSION_CHECK?

[pmqs]

Found some time to do it now.

Version 2.209 just uploaded to CPAN (there have bee issues with uploads for the last few days that mean the file should be available at cpan.org soon, but may take a while to pop out on metacpan). Also available on GH at https://github.com/pmqs/Compress-Raw-Lzma/releases/tag/v2.209

Not sure where you source the code from, so you have a few options.

Ping me if you need a hand.

[gregoa]

Thanks for taking the patch, and for releasing 2.209! I've now grabbed the tarball from MetaCPAN (which seems to be back on track) and uploaded it to Debian/unstable.

As for TEST_SKIP_VERSION_CHECK I think [0] for Compress-Raw-Lzma we do this as a precaution to avoid a potential version skew, if tests are ran again later (called autopkgtestsin Debian which are immensely useful e.g. for catching regressions caused by newer releases of required packages), against the built and installed packages, where the version of liblzma might have changed -- and the tests fail because the baked-in library version at build time and the detected version at runtime differ.

Ah, now I found an old bug report: https://bugs.debian.org/839987
At that time I patched out the version check, either TEST_SKIP_VERSION_CHECK didn't exist back then, or I hadn't found it yet :) (Later the patch was removed, and later I added TEST_SKIP_VERSION_CHECK to our build test and installed test environments.)

BTW: The situation is the same for Compress-Raw-Zlib, where we also use TEST_SKIP_VERSION_CHECK.
Additionally we're applying a patch there as it's not enough (which I've never forwarded as I assumed it doesn't affect anyone except Debian). Cf. https://bugs.debian.org/1024179 and https://salsa.debian.org/perl-team/modules/packages/libcompress-raw-zlib-perl/-/blob/master/debian/patches/tests-version-skew.patch

Cheers,
gregor

[0] my commit message is a bit terse: https://salsa.debian.org/perl-team/modules/packages/libcompress-raw-lzma-perl/-/commit/22e9c5e2bd17f87612623dd4aa51a526667ee0af

[pmqs]

Interesting that you allow skew in library versions between compile time & runtime - as an outsider who knows nothing about the the Debian processes it sounds like a problem waiting to happen but I'm sure it must be well controlled.

The reason I put the version check in originally was the real problem where I had folk build my code with library version x then run on version y and wonder whey things blew up because of a change in the interface to the library. If I remember correctly I asked the zlib folk to add the compile time & version symbol to allow me to police the check in my code.

Regarding the 02zlib.t change, I assume you are trying to get the version number at run time in the patch below.

--- a/t/02zlib.t.orig	2022-11-17 19:42:16.512726239 +0000
+++ b/t/02zlib.t	2022-11-17 19:45:06.421757489 +0000
@@ -12,7 +12,7 @@
 
 use Test::More  ;
 
-use constant ZLIB_1_2_12_0 => 0x12C0;
+use constant ZLIB_1_2_12_0 => '1.2.12';
 
 BEGIN
 {
@@ -489,7 +489,7 @@
 
     # Z_STREAM_END returned by 1.12.2, Z_DATA_ERROR for older zlib
     # ZLIB_NG has the fix for all versions
-    if (ZLIB_VERNUM >= ZLIB_1_2_12_0 ||  Compress::Raw::Zlib::is_zlibng)
+    if ($Zlib_ver gt ZLIB_1_2_12_0 ||  Compress::Raw::Zlib::is_zlibng)
     {
         cmp_ok $status, '==', Z_STREAM_END ;
     }
@@ -523,7 +523,7 @@
     $GOT = '';
     $status = $k->inflate($rest, $GOT);
     # Z_STREAM_END returned by 1.12.2, Z_DATA_ERROR for older zlib
-    if (ZLIB_VERNUM >= ZLIB_1_2_12_0 || Compress::Raw::Zlib::is_zlibng)
+    if ($Zlib_ver gt ZLIB_1_2_12_0 || Compress::Raw::Zlib::is_zlibng)
     {
         cmp_ok $status, '==', Z_STREAM_END ;
     }

Not sure your string compare is a safe way to do this test. Ideally you need a runtime version of ZLIB_VERNUM but I haven't created one (yet).

As you say it feels like a Debian-only issue. My code makes the assumption that compile time library version == runtime library version.

Hmmm, would a better option for you to just assume the zlib version is always newer than 1.2.12 in your patch. The issue with 1.2.12 is a few years old now. I assume you don't have ancient versions of zlib to contend with in Debian-land?

So the line would change from

if ($Zlib_ver gt ZLIB_1_2_12_0 || Compress::Raw::Zlib::is_zlibng)

to

if (1)

Anyway, ping if you want to discuss further.

BTW, the salsa links all end up in 500 errors.

[gregoa]

On Wed, 21 Feb 2024 14:00:41 -0800, Paul Marquess wrote: Interesting that you allow skew in library versions between compile time & runtime - as an outsider who knows nothing about the the Debian processes it sounds like a problem waiting to happen but I'm sure it must be well controlled.

Library transitions (i.e. API and/or ABI changes in libraries) are indeed common, well-understood, and managed (very rare accidents notwithstanding :)) Basically when e.g. a new zlib version changes ABI, there are some intermediate steps and then reverse dependencies like Compress::Raw::Zlib get rebuilt against the new zlib version and will depend on it. OTOH, if zlib just bumps its version without changing ABI, there's no need to do anything about Compress::Raw::Zlib; and that's how we get into the situation we're discussing here :)

The reason I put the version check in originally was the real problem where I had folk build my code with library version x then run on version y and wonder whey things blew up because of a change in the interface to the library.

Right, that was my assumption and that's a support nightmare, and it's totally understandable from an upstream POV to try and make sure that versions match. And with `TEST_SKIP_VERSION_CHECK` we can have both approaches (in most cases except this test):

Regarding the `02zlib.t` change, I assume you are trying to get the version number at run time in the patch below.

Ack.

Not sure your string compare is a safe way to do this test.

Right, I never felt good about this hack.

As you say it feels like a Debian-only issue. My code makes the assumption that compile time library version == runtime library version.

*nod*

Hmmm, would a better option for you to just assume the zlib version is always newer than `1.2.12` in your patch. The issue with 1.2.12 is a few years old now. I assume you don't have ancient versions of zlib to contend with in Debian-land?

Oh, indeed, that's a very good point, thanks! In the meantime enough time (and Debian releases) has (have) passed that we can ignore older zlibs.

So the line would change from ``` if ($Zlib_ver gt ZLIB_1_2_12_0 || Compress::Raw::Zlib::is_zlibng) ``` to ``` if (1) ```

Thanks, I'll use this!

BTW, the salsa links all end up in 500 errors.

Sorry, right after my message salsa had a maintenance window for deploying a Gitlab security update, which you seem to have hit :/ Thanks again, gregor

…

-- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe