v3.2.5...v3.2.6 ABI breaking? (Issue #25173)
Naser Mahfouz
mahf708 created an issue (wxWidgets/wxWidgets#25173)Even though the release notes at https://www.wxwidgets.org/news/2024/09/wxwidgets-3.2.6-released/ claim 3.2.6 was ABI compatible, it appears it is not.
This release is API and ABI-compatible with the previous 3.2.x releases, so the existing applications don’t even need to be rebuilt to profit from all the fixes above if they use shared/dynamic libraries. And if they do need to be recompiled, this can be done without any changes to the code.
If one looks at release diff v3.2.5...v3.2.6 and find the Makefile.in file, do the changes there keep ABI compatibility? Quoted below for ease:
diff --git a/Makefile.in b/Makefile.in index 93fbb2328d86..d6c26128855e 100644 --- a/Makefile.in +++ b/Makefile.in @@ -82,7 +82,7 @@ wxCFLAGS_C99 = @wxCFLAGS_C99@ DESTDIR = WX_RELEASE = 3.2 WX_RELEASE_NODOT = 32 -WX_VERSION = $(WX_RELEASE).5 +WX_VERSION = $(WX_RELEASE).6 LIBDIRNAME = $(wx_top_builddir)/lib WXREGEX_CFLAGS = $(WX_CPPFLAGS) -DNDEBUG \ -I$(wx_top_builddir)/3rdparty/pcre/src -D__WX__ -DHAVE_CONFIG_H \ @@ -2215,7 +2215,7 @@ LOCALE_LINGUAS = ca cs da de el es fi fr hu id it ja nl pl ru sl sv tr uk zh zh_ @COND_DEPS_TRACKING_1@CXXC = $(BK_DEPS) $(CXX) @COND_USE_PLUGINS_0@PLUGIN_ADV_EXTRALIBS = $(EXTRALIBS_SDL) @COND_PLATFORM_MACOSX_1@WXMACVERSION_CMD = \ -@COND_PLATFORM_MACOSX_1@ -compatibility_version 3.0 -current_version 3.3 +@COND_PLATFORM_MACOSX_1@ -compatibility_version 3.0 -current_version 3.4 @COND_USE_GUI_0@PORTNAME = base @COND_USE_GUI_1@PORTNAME = $(TOOLKIT_LOWERCASE)$(TOOLKIT_VERSION) @COND_TOOLKIT_MAC@WXBASEPORT = _carbon @@ -13977,9 +13977,9 @@ COND_MONOLITHIC_0_SHARED_1_USE_GUI_1_USE_HTML_1___htmldll_library_link_LIBR_0 \ @COND_USE_SOVERSION_0@dll___targetsuf2 = .$(SO_SUFFIX) @COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@dll___targetsuf3 \ @COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@ = \ -@COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@ .$(SO_SUFFIX).0.2.3 +@COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@ .$(SO_SUFFIX).0.2.4 @COND_PLATFORM_MACOSX_1_USE_SOVERSION_1@dll___targetsuf3 \ -@COND_PLATFORM_MACOSX_1_USE_SOVERSION_1@ = .0.2.3.$(SO_SUFFIX) +@COND_PLATFORM_MACOSX_1_USE_SOVERSION_1@ = .0.2.4.$(SO_SUFFIX) @COND_USE_SOVERCYGWIN_1_USE_SOVERSION_1@dll___targetsuf3 = -0.$(SO_SUFFIX) @COND_USE_SOVERSION_0@dll___targetsuf3 = .$(SO_SUFFIX) @COND_USE_SOVERSION_1_USE_SOVERSOLARIS_1@dll___targetsuf3 = .$(SO_SUFFIX).0
From d7f6a7e8d2522e740af0899418c22af19fdb4aeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= <vac...@slavik.io> Date: Sat, 10 Feb 2024 11:46:13 +0100 Subject: [PATCH 38/73] Fix AddAvailableCatalog() API Knowing msgIdLanguage is necessary for correct evaluation of preferred languages order; without it, it would be possible to load undesirable translation file. See 2af36e9560c694a750b6aa835d32d12a0393fb83 for details. --- Makefile.in | 6 +++--- build/bakefiles/version.bkl | 6 +++--- include/wx/translation.h | 12 ++++++++++++ interface/wx/translation.h | 15 +++++++++++++-- src/common/translation.cpp | 31 ++++++++++++++++--------------- version-script.in | 7 +++++++ 6 files changed, 54 insertions(+), 23 deletions(-) diff --git a/Makefile.in b/Makefile.in index d6c26128855e..86948bf57673 100644 --- a/Makefile.in +++ b/Makefile.in @@ -2215,7 +2215,7 @@ LOCALE_LINGUAS = ca cs da de el es fi fr hu id it ja nl pl ru sl sv tr uk zh zh_ @COND_DEPS_TRACKING_1@CXXC = $(BK_DEPS) $(CXX) @COND_USE_PLUGINS_0@PLUGIN_ADV_EXTRALIBS = $(EXTRALIBS_SDL) @COND_PLATFORM_MACOSX_1@WXMACVERSION_CMD = \ -@COND_PLATFORM_MACOSX_1@ -compatibility_version 3.0 -current_version 3.4 +@COND_PLATFORM_MACOSX_1@ -compatibility_version 4.0 -current_version 4.0 @COND_USE_GUI_0@PORTNAME = base @COND_USE_GUI_1@PORTNAME = $(TOOLKIT_LOWERCASE)$(TOOLKIT_VERSION) @COND_TOOLKIT_MAC@WXBASEPORT = _carbon @@ -13977,9 +13977,9 @@ COND_MONOLITHIC_0_SHARED_1_USE_GUI_1_USE_HTML_1___htmldll_library_link_LIBR_0 \ @COND_USE_SOVERSION_0@dll___targetsuf2 = .$(SO_SUFFIX) @COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@dll___targetsuf3 \ @COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@ = \ -@COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@ .$(SO_SUFFIX).0.2.4 +@COND_PLATFORM_MACOSX_0_USE_SOVERCYGWIN_0_USE_SOVERSION_1@ .$(SO_SUFFIX).0.3.0 @COND_PLATFORM_MACOSX_1_USE_SOVERSION_1@dll___targetsuf3 \ -@COND_PLATFORM_MACOSX_1_USE_SOVERSION_1@ = .0.2.4.$(SO_SUFFIX) +@COND_PLATFORM_MACOSX_1_USE_SOVERSION_1@ = .0.3.0.$(SO_SUFFIX) @COND_USE_SOVERCYGWIN_1_USE_SOVERSION_1@dll___targetsuf3 = -0.$(SO_SUFFIX) @COND_USE_SOVERSION_0@dll___targetsuf3 = .$(SO_SUFFIX) @COND_USE_SOVERSION_1_USE_SOVERSOLARIS_1@dll___targetsuf3 = .$(SO_SUFFIX).0
The quoted diffs above result in the following runtime error, reported in conda-forge/ncvis-climate-feedstock#15:
dyld[14285]: Library not loaded: @rpath/libwx_osx_cocoau_xrc-3.2.0.2.3.dylib
Referenced from: <76C4EB57-0906-3946-AD85-6F87A61B37FA> /Users/zender/anaconda/bin/ncvis
Reason: tried: '/Users/zender/anaconda/bin/../lib/libwx_osx_cocoau_xrc-3.2.0.2.3.dylib' (no such file), '/Users/zender/anaconda/bin/../lib/libwx_osx_cocoau_xrc-3.2.0.2.3.dylib' (no such file), '/usr/local/lib/libwx_osx_cocoau_xrc-3.2.0.2.3.dylib' (no such file), '/usr/lib/libwx_osx_cocoau_xrc-3.2.0.2.3.dylib' (no such file, not in dyld cache)
Abort trap: 6
zender@spectral:~/anaconda/bin$ ls -l /Users/zender/anaconda/bin/../lib/libwx_osx_cocoau_xrc*
-rwxr-xr-x 2 zender staff 903648 Dec 13 10:24 /Users/zender/anaconda/bin/../lib/libwx_osx_cocoau_xrc-3.2.0.3.0.dylib*
lrwxr-xr-x 1 zender staff 36 Feb 8 15:47 /Users/zender/anaconda/bin/../lib/libwx_osx_cocoau_xrc-3.2.0.dylib -> libwx_osx_cocoau_xrc-3.2.0.3.0.dylib*
lrwxr-xr-x 1 zender staff 36 Feb 8 15:47 /Users/zender/anaconda/bin/../lib/libwx_osx_cocoau_xrc-3.2.dylib -> libwx_osx_cocoau_xrc-3.2.0.3.0.dylib*
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)Oops, thanks for noticing this, this is definitely unexpected and I'm afraid we got macOS compatibility/current version stuff wrong, finally, see #24699 :-(
I'm not sure what is the best way forward, i.e. what version should be used for 3.2.7. Should we just document the breakage and live with it or is there some way of salvaging compatibility here, i.e. would reverting to 0.2.4.dylib make things better or worse (by making 3.2.7 incompatible with the programs linked against 3.2.6)?
cc @vslavik
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Naser Mahfouz
mahf708 left a comment (wxWidgets/wxWidgets#25173)I'm not sure what is the best way forward, i.e. what version should be used for 3.2.7. Should we just document the breakage and live with it or is there some way of salvaging compatibility here, i.e. would reverting to
0.2.4.dylibmake things better or worse (by making 3.2.7 incompatible with the programs linked against 3.2.6)?
I am not sure as well.
Do you usually strive to keep things ABI compatible across 3.2.x releases? If so, it may be worth reverting to reestablish compatibility, and so leaving v3.2.6 as the odd incompatible one out (potentially with a patch if someone desires to ensure it is compatible if they're building from source)? But if that has undesirable side effects (I assume this change was made for a reason to begin with), then we can consider documenting it and living with it.
I'd like to invite @hmaarrfk to weigh in because he's the maintainer of distributing the wxWidgets package in the conda-forge ecosystem.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Harfouche
hmaarrfk left a comment (wxWidgets/wxWidgets#25173)I would rather you document the breakage, and fix things as is.
i.e. what version should be used for 3.2.7.
IMO it should use the 3.2.6 ABI.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Václav Slavík
vslavik left a comment (wxWidgets/wxWidgets#25173)Oops, thanks for noticing this, this is definitely unexpected and I'm afraid we got macOS compatibility/current version stuff wrong, finally,
...
I'm not sure what is the best way forward, i.e. what version should be used for 3.2.7
Unless I'm missing something, I don't think this is a matter of one-time screw-up, but rather of macOS ABI compatibility handling being broken.
I followed the ABI instructions correctly, I think: a new function was added, so I followed step 2 and bumped current+age:
--- a/build/bakefiles/version.bkl +++ b/build/bakefiles/version.bkl @@ -22,9 +22,9 @@ 3. Else, i.e. if there were no changes at all to API but only internal changes, change C:R:A to C:R+1:A --> - <set var="WX_CURRENT">2</set> - <set var="WX_REVISION">4</set> - <set var="WX_AGE">2</set> + <set var="WX_CURRENT">3</set> + <set var="WX_REVISION">0</set> + <set var="WX_AGE">3</set>
This presumably generated correct sonames (there was no outcry from Linux package maintainers and their automated checks). Consequently, there's no easy fix for macOS side — the WX_* values are not wrong. The mapping to macOS compatibility handling is.
Reviewing my homebrew-installed wx instance on macOS, I at first had no idea how compatibility was supposed to work, if it changed at some point or was like this forever. It looks to me that the intent was that user binaries will link against (e.g.) libwx_baseu-3.2.dylib name, or perhaps libwx_baseu-3.2.0.dylib, but definitely not to the fully-specced libwx_baseu-3.2.0.3.0.dylib — which is what happens.
And that seems to be due to -- or in any case somewhat related to -- fa2fb36 from #24524 (May 2024, also explaining why it didn't pop up earlier). That commit intentionally skips symlinks when invoking install_name_tool during make install, but the correct filename that should be used is precisely one of those symlinks and not the fully-specced one (the only actual file)! That's how LC_ID_DYLIB ends up being wrong in the dylib and being referenced when linking user binaries. Assuming I'm right, v3.2.5 is the earliest version affected.
Finally, I can only see 3 ways to address this:
- Go back to pre-3.2.5 correct naming and document this as macOS-specific issue in 3.2.7's relnotes; or
- On 3.2 branch only, add (somehow...) a hack to keep the "bad" filename from 3.2.6 regardless of any future current:revision:age evolution; or
- Do option 1 and then, on 3.2 branch only, always create symlinks for "bad" filenames (
3.2.0.3.0.dylib,3.2.0.2.4.dylib) back to the correct file. I am not 100% sure this will work with the macOS linker, it may also be checkingLC_ID_DYLIB, but that's easy to test. @mahf708 does creating such a symlink help?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)Thanks a lot for taking time to look at this, Vaclav!
I am almost certain that the intention was indeed to only link against the libwx_baseu-3.2.dylib, after wx-config --libs returns -lwx_baseu-3.2, but I'm still not sure if we don't have another problem with the current/compatibility versions because of this:
% otool -l lib/libwx_baseu-3.2.0.2.3.dylib | sed -ne '/LC_ID_DYLIB/,/^Load/p' cmd LC_ID_DYLIB cmdsize 88 name /Users/zeitlin/build/wx/32/lib/libwx_baseu-3.2.0.2.3.dylib (offset 24) time stamp 1 Thu Jan 1 01:00:01 1970 current version 3.3.0 compatibility version 3.0.0 Load command 5 % otool -l lib/libwx_baseu-3.2.0.3.1.dylib | sed -ne '/LC_ID_DYLIB/,/^Load/p' cmd LC_ID_DYLIB cmdsize 88 name /Users/zeitlin/build/wx/32/lib/libwx_baseu-3.2.0.3.1.dylib (offset 24) time stamp 1 Thu Jan 1 01:00:01 1970 current version 4.1.0 compatibility version 4.0.0 Load command 5
i.e. the compatibility version has changed from 3 to 4 and this doesn't seem right, or do I misunderstand how this works (which is perfectly possible)?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)P.S. If the information here is correct, we should be using
current_version = "{}.{}".format(current + 1, revision)
compatibility_version = current + 1
while we use
current_version = "{}.{}".format(age + 1, revision)
compatibility_version = age + 1
Using age here really seems to be wrong, doesn't it?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Václav Slavík
vslavik left a comment (wxWidgets/wxWidgets#25173)I am almost certain that the intention was indeed to only link against the libwx_baseu-3.2.dylib, after wx-config --libs returns -lwx_baseu-3.2,
That's not a valid argument: You link with -lfoo on Linux too and the runtime linker will use e.g. libfoo.so.0, not libfoo.so (which is devel-only symlink). So I think the expectation was to use libwx_baseu-3.2.0 at runtime. In wx's case, the difference is only theoretical, of course, because the soname is always going to be .0 by wx's ABI and naming rules.
P.S. If the mesonbuild/meson#1451 is correct,
I was initially suspicious about it too, wrote something similar, then deleted it when I convinced myself that it is correct (IIRC it was modelled after libtool in bakefile back then). I think that link got it backwards in the initial description about how to set the compat. version, and compatibility_version is interpreted counter-intuitively by dyld:
Specifies the compatibility version number of the library. When a library is loaded by dyld, the compatibility version is checked and if the program's version is greater that the library's version, it is an error.
That is, if the program says it requires compatibility_version 4.0 of the library, running it against 3.4 is an error. Program's library list says "I require at least compatilibity_version 4.0" and the library says "I am good for compatibility up to 4.0".
See also https://stackoverflow.com/questions/67055770/usage-of-current-version-and-compatibility-version-on-macos where it is also mentioned than non-ancient versions of macOS just ignore all of this anyway and don't check 😳
Using age here really seems to be wrong, doesn't it?
Thanks for another brain-teaser :) I think it's correct after all:
- Increasing current alone (rule 1) never happens in wx. That is, when it happens, the library name itself changes (from foo-3.2 to foo-3.3; this is not part of the versioning suffix of the filename, but our chosen name).
- Therefore, current is modified in lockstep with age. Therefore, it doesn't matter.
- But removing APIs or changing their ABIs in backward incompatible way (rule 1) requires that you change the library filename (soname on Linux). Otherwise the files couldn't coexist. So that is where current is encoded: in the filename. By using age for compatibility and current versions within the dylib headers, you start the versioning from 0 when the filename changes and don't carry over baggage from ancient history.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)Thanks again for the explanations!
I've checked and LC_ID_DYLIB in 3.2.4 uses libwx_baseu-3.2.dylib, so I've created #25182 to do the same thing in v3.2.7. It touches only a couple of lines but changes quite a few things, so please let me know if you see something wrong with it.
@mahf708 and @hmaarrfk: Could you please check if the changes in this PR work for you? Also, would it be useful to create the links from 3.2.0.x.y.dylib to 3.2.0.dylib for compatibility with the programs linked against 3.2.5 and 3.2.6, i.e. to allow them work, without relinking, with 3.2.7 libraries, as suggested above?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Harfouche
hmaarrfk left a comment (wxWidgets/wxWidgets#25173)ah ok, so as i understand it, the ABI is compatible, but the libraries were named badly. I won't have time to backport it but maybe @mahf708 can to conda-forge to test it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)Yes, as far as we can see, there is no real ABI breakage, "just" the fact that the executables linked with 3.2.5 encoded wrong dependency name.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Naser Mahfouz
mahf708 left a comment (wxWidgets/wxWidgets#25173)Yes, that was my understanding too (links), but I wasn't sure if there would be side effects. I will test what you're asking and report back.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Naser Mahfouz
mahf708 left a comment (wxWidgets/wxWidgets#25173)So, lrwxr-xr-x 1 mahf708 staff 36 Feb 19 13:17 /Users/mahf708/.micromamba/envs/test_ncvis/bin/../lib/libwx_osx_cocoau_xrc-3.2.dylib -> libwx_osx_cocoau_xrc-3.2.0.3.0.dylib (likewise for 3.2.0.dylib) which means the the linking is correct, in that it is linking the existing versioned libraries as expected. The problem is, the binaries built previously are looking for the explicitly versioned library, dyld[88842]: Library not loaded: @rpath/libwx_osx_cocoau_xrc-3.2.0.2.3.dylib, which of course doesn't exist. We could potentially symlink libwx_osx_cocoau_xrc-3.2.0.3.0.dylib to libwx_osx_cocoau_xrc-3.2.0.2.3.dylib (and repeat as approriate) but I don't know if that's desirable.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Naser Mahfouz
mahf708 left a comment (wxWidgets/wxWidgets#25173)I do think your PR #25182 will likely fix the root of the issue, but it is a future-looking PR and I don't think applying it in retrospect will be desirable.
For our needs in conda-forge, we have implemented a solution (requiring packages needed wxwidgest to be rebuilt with every release, like we do with packages not guaranteeing ABI stability) and so everything is set there for now. In the future, once you have a release, we can restore wxwidgets on conda-forge side to be such that it ABI stability is guaranteed. For us there though, it doesn't make much of a difference (there are only 4 packages right now using wxwidgets that are impacted, and only on osx-, so nbd)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)but I don't know if that's desirable.
Well, if it would help you, we could do it. If it doesn't matter, it's simpler not to do anything, of course.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Naser Mahfouz
mahf708 left a comment (wxWidgets/wxWidgets#25173)As I said, on the conda-forge side, we are now good to go. I think I agree with Mark's comment above: #25173 (comment).
I would rather you document the breakage, and fix things as is.
i.e. what version should be used for 3.2.7.
IMO it should use the 3.2.6 ABI.
So, do what's best for your package and we will adapt
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)OK, thanks, if @vslavik doesn't have any comments/objections to this neither, I'll just merge this then.
Please note that that this means that 3.2.7 libraries still won't be compatible with 3.2.6 ones, but 3.2.8 will be compatible with 3.2.7 ones again.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)Fixed by the commit above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #25173 as completed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25173)Could you please check that #26200 doesn't break this again? AFAICS it should not, but I've made so many mistakes with this already that I'd appreciate an extra check. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Mark Harfouche
hmaarrfk left a comment (wxWidgets/wxWidgets#25173)I’ve really stopped using wx. So I won’t be super helpful. Sorry
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.