Please bump SONAME following API breakage #333
Comments
|
Thanks for your suggestion and explanation. Indeed, I am to blame for messing this up. The idea was to follow semver but my understanding of that was obviously lacking. It seems the right step would have been to release 2.0 instead of 1.3. Thinking about it for a minute makes me believe the 'right' way to go forward is actually complying with the semver rules (in the medium/long term). But how to get there? a) immediately release 2.0 and hope that people like you skip packaging 1.3 altogether? Could I go for b) and then a little later a) so that both patched 1.3 and 2.0 have SONAME 2 (provided there are no further breaking changes between 1.3 and 2.0)? What is your suggestion about how to best move forward? |
|
I believe a) b) c) d) all make sense. If the source code between already-released 1.3 and to-be-released 2.0 would not introduce any break, I would suggest to go with b), otherwise d) might be better. I am not in a hurry of packaging the new zxing-cpp version, and may as well wait for 2.0 if 2.0 will be released reasonably soon (e.g., within 3 months). |
|
I think I prefer d) and introduce more breaking changes to 'get it over with' and don't end up having to increase the major version very soon again. I added a hint to the release page discouraging the packaging of 1.3.0 until this is resolved. I also just stumbled upon this interesting list, so maybe is it already too late for that... |
|
I'd like to bring your attention to the e-mail conversation we had about 2 years ago regarding the following subjects:
Back then I considered them nice to have, now I would strongly prefer to apply the discussed changes. |
|
Sure, thanks for the hint!
|
|
About the naming currently used in Experimental: I'd like to see the following changes if possible:
In this context I'm also considering to rename the library from EDIT: I just realized that the two binaries |
|
@axxel The library package name follows the actual shared library name. For now the built filename is like Library renaming is solely at your discretion (as the upstream). If you have determined to rename the built shared library filename to be something like For the binary tools: I will take a look into them. |
|
I see. I'm not convinced the hassle of renaming and library to About the SONAME 2: I'm currently trying to get as much ABI breaking stuff done and then first release a 1.4 with more depredations and then a 2.0 with those removed and hopefully something that lasts a while. So I hope that experimental package will not trickle to testing before that is done, otherwise I'd need to directly jump to 3.0 to get in sync again. |
|
@hosiet I just released v1.4.0, see release notes for details. My next step would be releasing 2.0 with all the deprecated API removed. I might make other ABI breaking changes hiding the implementation details of |
|
But this is not how SONAME bumping works. As soon as you've made a release that makes ABI breaking changes in the shared library by removing symbols from it, you need to bump the SONAME. As soon as you've made a release, the code is out in the wild and will be linked against by other software. This software needs to know which version it can expect to provide the symbols it uses. You already performed one of these breaks with the 1.3 release and that's why we bumped SONAME to 2 in Debian. If the 2.0 release breaks ABI again, we need to bump SONAME to 3. Remember that the SONAME is decoupled from the version you give your software. So releasing version 2.0 with SONAME 3 is not wrong. Is there any plan to release 2.0 soon? The gstreamer 1.22 release (supposed to happen within the next few weeks) requires zxing-cpp 1.4 and i'm currently working on better suppor for 1.4 in Debian but maybe the 2.0 is coming out soon? |
|
Obviously I've completely missed my own goal of releasing 2.0 in the summer. The mentioned changes (PIMPL idiom for Regarding the SONAME: my understanding is that
|
If we are only talking about Debian, then the answer is "maybe" (more on that below). But you probably do not know all of the places where zxing-cpp is used and these other distributions might've already done a "release" of 1.3 or 1.4. But this is only a hypothetical argument so lets stick to Debian for the rest of this message: Yes, as far as Debian is concerned, no "release" of 1.4 happened yet. It's still sitting in "experimental" where we are free to upload even broken stuff. The problem is, that other packages like gstreamer 1.22 or mediastreamer2 5.2 need at least zxing-cpp 1.4 and are not compatible with earlier versions anymore (this hints at some other distribution already having released 1.4 or otherwise these projects probably wouldn't have bumped their build requirements?). So if we want to update these packages, then we need to upload zxing-cpp 1.4 or later to "unstable" where it will then become "released". And then there is also the freeze date 2023-01-12 before which all transitions have to be done. As far as I see we have the following options:
|
|
zxing-cpp 1.4 is now "released" by being in Debian testing and thus becoming part of the next stable release: https://packages.debian.org/source/testing/zxing-cpp Since the 1.4 release breaks ABI, we patch zxing-cpp to have SONAME2, hence the binary package is called libzxing2 with version 1.4.0-3. Thus, it would help if you could bump SONAME to 3 for the 2.0 release. Thanks! |
|
That is most unfortunate. I was putting in a couple hours the last days to move forward with the 2.0 release. You mentioned the 12.1. as the deadline (not today). I was checking the mentioned gstreamer and mediastreamer2 code bases yesterday to make sure they both work with 2.0 as is and they do. I'd very much prefer if you could work towards "releasing" 2.0 with SONAME 2. I can tag that today. |
|
Thank you for working on the 2.0 release! Why are you so adamant on making the 2.0 release SONAME 2? It is completely fine to let it have SONAME 3. Do you want to have the SONAME version be the same as the major version? There are a lot of popular libraries where SONAME versioning is independent of library versioning. It's completely fine to have those two versions not match at all. If you really want the SONAME to be the same as the version, you could skip a 2.0 release and go 3.0 directly. Yes, the deadline is at the 12th but by that time, transitions need to be finished and a lot of other transitions are intermingled with libzxing because software that uses libzxing (mainly libreoffice) depend on lots of other stuff (like qt) which is transitioning these days as well. Is the ABI changing again between 1.4 and 2.0? If yes, you need to bump SONAME. If you don't, then we'll have to patch that in Debian. |
Since I was made aware of the fact that my version naming was flawed (see above) I was working (too slowly, apparently) towards a 'proper' SemVer fix and feeling a final push from your message 4 days ago and investing time to make this happen, it is just a bit disappointing that I failed to prevent this situation. I don't know, yet, how I will move forward. From a technical perspective, I can't disagree with your assessment. Regarding LibreOffice: does this mean the 2 day old patch is about to make it into both the next libreoffice release as well as the next debian release? impressive.
Yes, and contrasting to the practically irrelevant ABI changes between 1.1 and 1.4, these are actually 'relevant' (meaning all 1.1 based client code will require changes to even compile with 2.0). |
|
Thank you for your swift replies! I'm sorry that you are troubled by the current situation. Honestly, I did not think t hat your 2.0 release would really be happening today because the last time you said you will do a release soon, it also took a while. This is no problem. You are probably doing this as a hobby just as I? Sometimes other life stuff just gets in the way. But if other code requires changes to even compile then there is no way that 2.0 will manage to make it into Debian testing before the 12th of January because then we need to change all source packages that require libzxing and depending on the maintainer that can take a lot longer because then it's not only about libreoffice (i was surprised by how quickly that got patched as well). |
|
Looking at https://salsa.debian.org/debian/zxing-cpp/-/blob/master/debian/control I have 2 improvement suggestions:
Questions:
|
Thank you! Fixed!
That is correct, but we want to ship them in the future, see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993012 So yes, you are correct that they are not shipped yet but I'm working on distributing ZXingQtReader and ZXingOpenCV as well, see this commit: https://salsa.debian.org/debian/zxing-cpp/-/commit/79b9757c8442a804bff4a267c8990d1ce3942676
True. But since zxing-cpp-tools and python3-zxing-cpp are binary packages we produce, its dependencies have to be put into the Build-Depends field. As you are looking into the packaging, maybe the list of patches we currently carry in Debian is of interest to you (in the
It does not. Where do you see that?
Yes. But you need to have the tools for parsing Debian dependencies like dose3 installed and then feed them the Packages and Sources files that you downloaded from Debian. Would you like to see a list? |
Nice. In case my EDIT above was a bit too sneaky: please also fix the metadata
I see. I actually would like to question this move, though: The two "examples"
Yes, I noticed the list. I'd be interested to understand why 0001 and part of 0002 is required. Also 0003 seems unnecessary as long as the default build-options are used.
https://salsa.debian.org/debian/zxing-cpp/-/blob/master/debian/changelog#L4
I would be interested to see how people are using the code. E.g. I offered some suggestions to the mediastreamer2 people. |
Whoops, fixed now as well. Thanks!
I see. I just asked the submitter of #993012 why they need these tools.
It is not needed. I tried removing it and the output is bit-by-bit identical minus the gcc build-id. Thank you for bringing this up!
If I drop 0002, then
Without that patch, the build will fail with: The reason is because we removed
That changelog entry talks about the removal of the breaks which was added wrongly before.
The current list of source packages is:
And while we are abusing this issue for packaging questions, I'd also like to ask if the naming of the python module is deliberate? Should its name not be zxing_cpp? It's currently named zxingcpp and this clashes with its package name. I'm tempted to rename python3-zxing-cpp to python3-zxingcpp so that the package name matches the import module name if this library naming was deliberate. Thanks a lot for all your feedback! |
I understand that the
That means
Starting with 1.4 I removed them from the source tarball (because of their size, mostly). I thought debian packages are built starting from that (and not from cloning a git repo, right)?
Thanks, then I know all of them.
Yes, see this and the following comments. The python package (on PyPi) is definitely not going to change its name Thanks a lot for your feedback! :) |
|
Thanks for the ping!
The python module still builds successfully without these lines but it will build a different module compared to having these lines added. I didn't write them. Maybe @hosiet knows more?
in
But some of the tests that are enabled by BUILD_BLACKBOX_TESTS=ON do not need the sample images, right?
We do not clone the git repo but we download the tarball from https://github.com/zxing-cpp/zxing-cpp/tags The problem with using the github release page is that the tarball links are only visible after some javascript has run. So our automated tools that check for new upstream versions cannot spot new releases anymore. See https://lists.debian.org/20220919205036.6400ff9d9767af3991c263e6@iijmio-mail.jp Another reason why we often avoid the releases and download a tarball from a git tag is, that oftentimes projects will remove source from the release and only publish the created binary, like autoconf artifacts. In Debian we want to make sure to build from source and using git tags is one way to achieve that. |
looking forward to his/her input.
ah, how could I have missed that... This comment got it backwards: the unit tests run without the samples, the blackbox tests need them. Please flip that and remove the 0003 patch.
Understood, just as I thought. The images are not part of the source tarball ever since 1.4 anymore. (see here) |
Aha! Nice find! I patched it like this: diff --git a/debian/rules b/debian/rules
index 18792db..e7d3424 100755
--- a/debian/rules
+++ b/debian/rules
@@ -19,13 +19,12 @@ export QT_SELECT := 5
# Disabled flags:
#
-# BLACKBOX_TESTS: see #1004653
-# UNIT_TESTS: requires tests/samples/* files with unclear license
+# BLACKBOX_TESTS: requires tests/samples/* files with unclear license
# PYTHON_EXECUTABLE: force select default python version
override_dh_auto_configure:
dh_auto_configure -- \
- -DBUILD_BLACKBOX_TESTS=ON \
- -DBUILD_UNIT_TESTS=OFF \
+ -DBUILD_BLACKBOX_TESTS=OFF \
+ -DBUILD_UNIT_TESTS=ON \
-DBUILD_PYTHON_MODULE=ON \
-DBUILD_SYSTEM_DEPS=ALWAYS \
-DPYTHON_EXECUTABLE:FILEPATH=$(shell command -v $$(py3versions -d))and dropped |
Arghh... that is unfortunate. The test ran just fine for 1.4 in our own CI. A quick guess is that is has something to do with the locale. The current (2.0) code does not contain anything dependent on the locale anymore, so this is most certainly fixed in 2.0. For your 1.4 based release, I see two options:
From a project maintainer perspective it would certainly be of interest to leverage the wide platform support of Debian as some kind of external CI system but from a package maintainer and also end-user perspective I would question the cost/benefit ratio of adding another build dependency and paying for the test execution. |
|
The following patch fixes it: --- a/test/unit/TextUtfEncodingTest.cpp
+++ b/test/unit/TextUtfEncodingTest.cpp
@@ -26,8 +26,8 @@ TEST(TextUtfEncodingTest, ToUtf8AngleEsc
EXPECT_EQ(ToUtf8(L"\u2602", angleEscape), "<U+2602>");
#ifndef _WIN32
- std::setlocale(LC_CTYPE, "en_US.UTF-8");
- EXPECT_STREQ(std::setlocale(LC_CTYPE, NULL), "en_US.UTF-8");
+ std::setlocale(LC_CTYPE, "C.UTF-8");
+ EXPECT_STREQ(std::setlocale(LC_CTYPE, NULL), "C.UTF-8");
#else
std::setlocale(LC_CTYPE, ".utf8");
EXPECT_TRUE(std::string(std::setlocale(LC_CTYPE, NULL)).find("utf8") != std::string::npos); |
|
fair enough. even though the locale stuff is gone in 2.0 your report still helped improve it: I just removed two stale |
|
@josch I just noticed one thing: your above list of source packages depending on zxing-cpp actually invalidates the following reason of yours why a 2.0 can't be part of the next Debian release
As that software is going to compile with 2.0 as is (to the best of my knowledge). Just wondering whether that makes any difference to you. It would for me, if I could then get my desired SemVer fix in place. |
|
They are going to compile without changes? Because above you said:
Assuming all projects would compile as-is, i'd have to contact the Debian release managers first because this would require a transition and rebuilds without SONAME bump. |
As I said: "to the best of my knowledge" and that came from
Now that you questioned my statement again, I also had a look at the current Kaidan code base and learned that the "living at head" assumption was indeed a little bit overoptimistic. They are using a function that produces a deprecation warning when compiled with 1.4 and is removed in 2.0. I just assumed that looking at this warning for 6 months must have been annoying enough to simply do the transition. Looks like I was wrong. :-( Thanks for considering the extra hassle just for me, though! |
Kaidan isn't something I'm particularly involved with, but I can look into that if needed. The rest of our stuff seems fine with 2.0. |
Ahh, so much for my memory... In my mind there was a simple equation @vkrause == KDE ;). Thanks for looking into this. It might be just one problematic line: https://invent.kde.org/network/kaidan/-/blob/master/src/QrCodeDecoder.cpp#L93. |
|
FTR: The reason I requested ZXingQtReader and ZXingOpenCV be packaged
is that zxing-cpp isn't well integrated into Linux desktops (at least
not GNOME, but I see there is some KDE integration) so there aren't
many GUI tools that provide its functionality to users and the Qt and
OpenCV zxing-cpp tools, while minimal, do provide useful functionality
to a subset of the potential users. Even if GNOME integration existed,
the tools would be useful in bare-bones window managers like i3 or
sway, or on Linux based mobile devices like the PinePhone, which often
run bare-bones desktops.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
Thanks for the explanation. As I explained above I see the gain in having a live video app to get some interactive results. That means either the |
|
Okay, if |
|
Regarding the heaviness of dependencies: I would personally find it annoying if I wanted to get access to the "main" tools |
|
Ah that you mean. I misunderstood. Yes, that is a valid comment. We'd ship these two tools in their own binary package called |
|
If ZXingOpenCV or ZXingQtCamReader can extract barcodes within images
(rather than live camera feeds) then ZXingQtReader isn't needed, but
otherwise it definitely is needed, most of my zxing-cpp usage is on
images that have come from the web or a 2D scanner.
I also don't know what the interfaces of these tools look like and if
there is value in having a simpler interface in some cases.
I already have both Qt and OpenCV installed due to other programs, on
Linux I expect lots of folks have both so either/both deps are fine.
I suggest shipping the GUI tools in a zxing-cpp-tools-gui package,
it is more descriptive than -extras.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
I respectfully disagree. The tool to use on image files is
I'm confused. That sounds like you have not looked at / tested any of these? How then would you know that having them shipped with Debian is useful to you? |
|
The value of ZXingQtReader/etc over ZXingReader is that they are GUI
tools. Not everyone can handle using a command-line tool of course, so
GUI tools are needed too, for people who cannot use a command-line.
Personally I'm fine with command-line, so the request for having the
GUI tools isn't for me personally but for users who need a GUI.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
That is not correct. ZXingQtReader is a cli tool (just like ZXingReader is, only way less capable). |
|
I see, then yes it definitely isn't useful.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
|
@josch it looks like the work on the Debian package has stalled about a week ago and e.g. the ZXingQtReader binary will indeed end up in the |
|
No, sorry I forgot to push my local commit into the repo. This commit reverts my last changes: https://salsa.debian.org/debian/zxing-cpp/-/commit/d2cf32308c983c850274b994bb4caeb31b6b35d8 So ZXingQtReader will not get installed. And yes, work on the Debian package has stalled because right now, the packaging repository (as far as I'm concerned) does not include any essential functionality that absolutely must make it into the next Debian stable release. Thus, my plan was, to leave the packaging git repo in a good state and leave it up to @hosiet to do the next upload. |
|
Thanks for the info. Sounds like a plan. 2.0 is waiting to get into Debian anytime :). If new problems with that should show up, please open new github issue. I declare this one as retired now ;) |
Hi,
I was working on packaging zxing-cpp in Debian/Ubuntu. As I read in https://github.com/nu-book/zxing-cpp/releases/tag/v1.3.0 , v1.3.0 introduces API breakage by removing deprecated APIs. Essentially we should bump library SONAME to avoid older binaries that dynamically linked to
libzxing.so.1from crashing due to missing symbols. Ideally this should be done before each version release that introduces API/ABI breakage.May I suggest decoupling
SONAMEwith{PROJECT_VERSION_MAJOR}and bumpSONAMEto 2, given that we are obviously not following semver? The following patch should do the job. Let me know if you prefer me to submit a Pull Request instead.This would greatly help downstream Linux distributions to track library API/ABI changes, and ensure that consumers of zxing-cpp would transition to a new library version seamlessly.
Please let me know if you have any questions or concerns. Thanks!
The text was updated successfully, but these errors were encountered: