Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run_exports doesn't match ABI #62

Closed
1 task done
minrk opened this issue Apr 11, 2024 · 17 comments
Closed
1 task done

run_exports doesn't match ABI #62

minrk opened this issue Apr 11, 2024 · 17 comments
Labels
bug Something isn't working

Comments

@minrk
Copy link
Member

minrk commented Apr 11, 2024

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

current installs of libadios are failing to find libblosc2.2. This is because libadios is linking to libblosc2.2.dylib (dependency: c-blosc2 >=2.11.2,<3.0a0), but c-blosc2 2.14 contains libblosc2.3.dylib, which will not work with any package built against c-blosc2 < 2.14). I believe this will affect all packages that linked c-blosc2 prior to the 2.14 release.

I'm not sure how best to indicate this ABI version, but it's probably worth including a pinning in run_exports that will work, and an assert in tests that libblosc2.${ABI_VERSION} is present.

Apparently c-blosc has ABI breakages in minor versions.

Installed packages

# packages in environment at /opt/conda/envs/test:
#
# Name                    Version                   Build  Channel
_openmp_mutex             4.5                       2_gnu    conda-forge
adios2                    2.9.2           nompi_py312h6656f7a_101    conda-forge
bzip2                     1.0.8                h31becfc_5    conda-forge
c-ares                    1.28.1               h31becfc_0    conda-forge
c-blosc2                  2.13.2               h1a8d543_0    conda-forge
ca-certificates           2024.2.2             hcefe29a_0    conda-forge
hdf5                      1.14.3          nompi_ha486f32_100    conda-forge
keyutils                  1.6.1                h4e544f5_0    conda-forge
krb5                      1.21.2               hc419048_0    conda-forge
ld_impl_linux-aarch64     2.40                 h2d8c526_0    conda-forge
libadios2                 2.9.2           nompi_hecd7c66_101    conda-forge
libaec                    1.1.3                h2f0025b_0    conda-forge
libblas                   3.9.0           22_linuxaarch64_openblas    conda-forge
libcblas                  3.9.0           22_linuxaarch64_openblas    conda-forge
libcurl                   8.7.1                h4e8248e_0    conda-forge
libedit                   3.1.20191231         he28a2e2_2    conda-forge
libev                     4.33                 h31becfc_2    conda-forge
libexpat                  2.6.2                h2f0025b_0    conda-forge
libffi                    3.4.2                h3557bc0_5    conda-forge
libgcc-ng                 13.2.0               hf8544c7_5    conda-forge
libgfortran-ng            13.2.0               he9431aa_5    conda-forge
libgfortran5              13.2.0               h582850c_5    conda-forge
libgomp                   13.2.0               hf8544c7_5    conda-forge
liblapack                 3.9.0           22_linuxaarch64_openblas    conda-forge
libnghttp2                1.58.0               hb0e430d_1    conda-forge
libnsl                    2.0.1                h31becfc_0    conda-forge
libopenblas               0.3.27          pthreads_h5a5ec62_0    conda-forge
libpng                    1.6.43               h194ca79_0    conda-forge
libsodium                 1.0.18               hb9de7d4_1    conda-forge
libsqlite                 3.45.2               h194ca79_0    conda-forge
libssh2                   1.11.0               h492db2e_0    conda-forge
libstdcxx-ng              13.2.0               h9a76618_5    conda-forge
libuuid                   2.38.1               hb4cce97_0    conda-forge
libxcrypt                 4.4.36               h31becfc_1    conda-forge
libzlib                   1.2.13               h31becfc_5    conda-forge
lz4-c                     1.9.4                hd600fc2_0    conda-forge
ncurses                   6.4.20240210         h0425590_0    conda-forge
numpy                     1.26.4          py312h470d778_0    conda-forge
openssl                   3.2.1                h31becfc_1    conda-forge
pip                       24.0               pyhd8ed1ab_0    conda-forge
python                    3.12.2          h43d1f9e_0_cpython    conda-forge
python_abi                3.12                    4_cp312    conda-forge
readline                  8.2                  h8fc344f_1    conda-forge
setuptools                69.2.0             pyhd8ed1ab_0    conda-forge
tk                        8.6.13               h194ca79_0    conda-forge
tzdata                    2024a                h0c530f3_0    conda-forge
wheel                     0.43.0             pyhd8ed1ab_1    conda-forge
xz                        5.2.6                h9cdd2b7_0    conda-forge
zeromq                    4.3.5                h2f0025b_1    conda-forge
zfp                       0.5.5                h01db608_8    conda-forge
zlib-ng                   2.0.7                hb4cce97_0    conda-forge
zstd                      1.5.5                h4c53e97_0    conda-forge

Environment info

active environment : base
    active env location : /opt/conda
            shell level : 1
       user config file : /root/.condarc
 populated config files : /opt/conda/.condarc
          conda version : 24.1.2
    conda-build version : not installed
         python version : 3.10.14.final.0
                 solver : libmamba (default)
       virtual packages : __archspec=1=m1
                          __conda=24.1.2=0
                          __glibc=2.31=0
                          __linux=6.6.16=0
                          __unix=0=0
       base environment : /opt/conda  (writable)
      conda av data dir : /opt/conda/etc/conda
  conda av metadata url : None
           channel URLs : https://conda.anaconda.org/conda-forge/linux-aarch64
                          https://conda.anaconda.org/conda-forge/noarch
          package cache : /opt/conda/pkgs
                          /root/.conda/pkgs
       envs directories : /opt/conda/envs
                          /root/.conda/envs
               platform : linux-aarch64
             user-agent : conda/24.1.2 requests/2.31.0 CPython/3.10.14 Linux/6.6.16-linuxkit ubuntu/20.04.6 glibc/2.31 solver/libmamba conda-libmamba-solver/24.1.0 libmambapy/1.5.7
                UID:GID : 0:0
             netrc file : None
           offline mode : False
@minrk minrk added the bug Something isn't working label Apr 11, 2024
@hmaarrfk
Copy link
Contributor

Sigh. Sorry about this.

We should just pin to minor versions.

Hoping for major pin level compatibility is alot for nascent projects.

So we should:

  1. Tighten the run exports for 2.14.
  2. Release a new 2.13 with tightened exports
  3. Add a repo data patch to tighten the 2.11 to 2.13 to have an upper bound of <2.14.0a.0
  4. Mark 2.14.0 through 2.14.3 as broken (them changing the SO name on a patch number is really strange)

From their release notes.
image

Truthfully, I don't really have the "time" to deal with this before april 20th, so your help if you can would be greatly appreciated.

I would support:

  1. Marking 2.14.0 through 2.14.4 as broken.
  2. Releasing 2.13.X with a stricter run_export.

Then we can take our time with the repodata patch.

@hmaarrfk
Copy link
Contributor

@conda-forge-admin please rerender

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I just wanted to let you know that I started rerendering the recipe in #63.

@minrk
Copy link
Member Author

minrk commented Apr 11, 2024

#64 is one possibility for ABI pinning going forward, by tracking the ABI version (and making sure SONAME matches or it won't build)

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Apr 11, 2024

with the API / ABI changes, I'm not sure how far we have to mark things broken as...

if you were built with 2.14.1, does it work with 2.13.2 or 2.14.2???

@hmaarrfk
Copy link
Contributor

maybe we don't mark the packages as broken, but pin exact for anything built between:

2.14.0 and 2.14.3

2.14.4 can work with 2.14.x

@FrancescAlted
Copy link
Contributor

Ok, upstream developer here. For context, the origin of the change is this: Blosc/c-blosc2#581 . Would you like the Blosc developers to invalidate 2.14.4 and release 2.15.0 for this SONAME change better?

We are sorry that we don't deeply understand the conventions for changing APIs in C libraries, so we are just following advices here and hoping for the best. Thanks for your patience.

@hmaarrfk
Copy link
Contributor

Thanks Francesc!

I should have for sure read the release notes of 2.14.4 and it would have flagged this issue days ago.

We are sorry that we don't deeply understand the conventions for changing APIs in C libraries, so we are just following advices here and hoping for the best. Thanks for your patience.

There sadly aren't very strict conventions. Some take "minor bump" as the API change, some think "only in major".
Then for ABI, there aren't really any hard rules.

It is convenient when ABI bumps are synched predictaly with a version number for packagers, but many upstream developers don't really care.

Would you like the Blosc developers to invalidate 2.14.4 and release 2.15.0 for this SONAME change better?

I'm not really sure it matters so much for this release. The main thing I would ask (as a packager) is that future SONAME changes sync up with minor release bumps.

@FrancescAlted my question to you:

  • For a package built with 2.14.1, is it compatible with 2.14.4 in your mind?

@minrk
Copy link
Member Author

minrk commented Apr 11, 2024

Add a repo data patch to tighten the 2.11 to 2.13 to have an upper bound of <2.14.0a.0

why specifically 2.11? There are builds accepting up to 3.0 back to 2.0.1, I think. What should be done with those?

@FrancescAlted
Copy link
Contributor

Thanks Francesc!

I should have for sure read the release notes of 2.14.4 and it would have flagged this issue days ago.

We are sorry that we don't deeply understand the conventions for changing APIs in C libraries, so we are just following advices here and hoping for the best. Thanks for your patience.

There sadly aren't very strict conventions. Some take "minor bump" as the API change, some think "only in major". Then for ABI, there aren't really any hard rules.

Ok, so in this case, as the API change had effect back in 2.13.0, we should have bumped SOVERSION by then right? Ok, I have added a reminder to do that in the future.

It is convenient when ABI bumps are synched predictaly with a version number for packagers, but many upstream developers don't really care.

I'm not sure what do you mean here. Are you suggesting to add SOVERSION bumps for every new version? Or only when ABI/API changes?

Would you like the Blosc developers to invalidate 2.14.4 and release 2.15.0 for this SONAME change better?

I'm not really sure it matters so much for this release. The main thing I would ask (as a packager) is that future SONAME changes sync up with minor release bumps.

You mean that a future 2.15.0 should bump SOVERSION too (and so with every minor version, like 2.16.0 and so on)? If so, what's the rational behind it?

@FrancescAlted my question to you:

  • For a package built with 2.14.1, is it compatible with 2.14.4 in your mind?

Yes, actually 2.14.4 should be compatible with all previous 2.14.x releases. For 2.14.4. we just were trying to acknowledge a previous API change in 2.13.0.

@hmaarrfk
Copy link
Contributor

You mean that a future 2.15.0 should bump SOVERSION too (and so with every minor version, like 2.16.0 and so on)? If so, what's the rational behind it?

Sorry, I mean "if you change the ABI", try to "synchronize it with" a minor bump, like 2.15 -> 2.16

Yes, actually 2.14.4 should be compatible with all previous 2.14.x releases. For 2.14.4. we just were trying to acknowledge a previous API change in 2.13.0.

Understood. I'll likely try to find a package that was built with it, and see what happens in the conda-forge context.

@FrancescAlted
Copy link
Contributor

You mean that a future 2.15.0 should bump SOVERSION too (and so with every minor version, like 2.16.0 and so on)? If so, what's the rational behind it?

Sorry, I mean "if you change the ABI", try to "synchronize it with" a minor bump, like 2.15 -> 2.16

Ah, that makes sense. Clarified in Blosc/c-blosc2@ce788ef

Yes, actually 2.14.4 should be compatible with all previous 2.14.x releases. For 2.14.4. we just were trying to acknowledge a previous API change in 2.13.0.

Understood. I'll likely try to find a package that was built with it, and see what happens in the conda-forge context.

Sounds good. Thanks for your cooperation!

@hmaarrfk
Copy link
Contributor

@FrancescAlted part of the problem is that conda(-forge) makes an assumption that the ABI and the version number have a relationship. Since there is no hard and fast rule, I just "assumed" that it was related to the "major" number of the library.

So it was my assumption that ultimately caused the problem.

@minrk tried to "remove this assumption" in #64 but I think that it is too complicated and the version number is quite useful to communicate to downstream users and packagers that things "change". So many times, it is not necessary.

Often the challenge is when the lines are blurred between application and library, such as with ffmpeg, where the SO number is for a developers, and a version is for a user of the cli or the GUI.

@minrk
Copy link
Member Author

minrk commented Apr 11, 2024

SOVERSION should be increased when a package built against the previous version may not work with the new version (typically removed or renamed symbols, or a struct changing in size or layout). That's the goal of what SOVERSION is to communicate. If you are only adding symbols, SOVERSION doesn't need to change.

This can be tricky for maintainers, because ABI changes and API changes don't always map 1:1, but for the most part:

  • remove or rename an API: bump SOVERSION
  • add api: leave SOVERSION alone

If I understood Blosc/c-blosc2#581 correctly, that means that 2.13 should have bumped the SOVERSION, but this was delayed until 2.14.4.

That would mean that the compatibility ranges are:

  • 2.0-2.12 (SOVERSION 2)
  • 2.13-2.14.3 (ABI change due to changed symbol, but still identifies as SOVERSION 2)
  • 2.14.4+ (SOVERSION 3)

That is, these transition lines cause breakages when the compile-time version doesn't match runtime:

  • compiled against 2.12 with 2.13+ at runtime produces: "can't find symbol: register_tuner_private" because the ABI changed
  • compiled against <=2.14.3, with 2.14.4 at runtime produces: Library not loaded: @rpath/libblosc2.2.dylib because the reported ABI changed

and ideally these two transitions (reported ABI and actual ABI) happen at the same time.

https://abi-laboratory.pro is a handy tool (not sure if it's really maintained anymore) that scans libraries to determine when their ABI changes. It might be useful to check with a tool like that when preparing releases to make sure you know whether to bump SOVERSION.

I'll try to update conda-forge/conda-forge-repodata-patches-feedstock#703 to reflect this

hmaarrfk added a commit to hmaarrfk/conda-forge-pinning-feedstock that referenced this issue Apr 11, 2024
Larger discussion in: conda-forge/c-blosc2-feedstock#62 (comment)

TLDR:

- Previous run export was to major version
- SO changed in between 2.13 and 2.14

Patching will be made for previous builds in conda-forge/conda-forge-repodata-patches-feedstock#703
@FrancescAlted
Copy link
Contributor

FrancescAlted commented Apr 11, 2024

SOVERSION should be increased when a package built against the previous version may not work with the new version (typically removed or renamed symbols, or a struct changing in size or layout). That's the goal of what SOVERSION is to communicate. If you are only adding symbols, SOVERSION doesn't need to change.

This can be tricky for maintainers, because ABI changes and API changes don't always map 1:1, but for the most part:

  • remove or rename an API: bump SOVERSION
  • add api: leave SOVERSION alone

If I understood Blosc/c-blosc2#581 correctly, that means that 2.13 should have bumped the SOVERSION, but this was delayed until 2.14.4.

That would mean that the compatibility ranges are:

  • 2.0-2.12 (SOVERSION 2)
  • 2.13-2.14.3 (ABI change due to changed symbol, but still identifies as SOVERSION 2)
  • 2.14.4+ (SOVERSION 3)

That is, these transition lines cause breakages when the compile-time version doesn't match runtime:

  • compiled against 2.12 with 2.13+ at runtime produces: "can't find symbol: register_tuner_private" because the ABI changed
  • compiled against <=2.14.3, with 2.14.4 at runtime produces: Library not loaded: @rpath/libblosc2.2.dylib because the reported ABI changed

and ideally these two transitions (reported ABI and actual ABI) happen at the same time.

https://abi-laboratory.pro is a handy tool (not sure if it's really maintained anymore) that scans libraries to determine when their ABI changes. It might be useful to check with a tool like that when preparing releases to make sure you know whether to bump SOVERSION.

I'll try to update conda-forge/conda-forge-repodata-patches-feedstock#703 to reflect this

Thanks for the detailed information @minrk! I have added a link to this note to our release procedure. Hope to make it better next time!

@hmaarrfk
Copy link
Contributor

Do we have anything else to do now that the migration is complete:
#62

@minrk
Copy link
Member Author

minrk commented Apr 23, 2024

I don't think so, thanks @hmaarrfk!

@minrk minrk closed this as completed Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants