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

Improve shared object versioning #155

Open
kmilos opened this issue Sep 23, 2024 · 7 comments
Open

Improve shared object versioning #155

kmilos opened this issue Sep 23, 2024 · 7 comments

Comments

@kmilos
Copy link

kmilos commented Sep 23, 2024

Currently because of

SOVERSION "${OPENJPH_VERSION_MAJOR}.${OPENJPH_VERSION_MINOR}"

it is necessary to rebuild all clients on every minor release, even if API/ABI has not changed, which is somewhat inconvenient (and wasteful).

Either

  1. Have soverison as major only (0, i.e. "unstable" for now) and let clients deal w/ potential breakage on a case by case basis depending on each release until you reach stable 1.x, 2.x etc., or
  2. Introduce a separate library version that's bumped only when needed (e.g. you could even fix it at 0.17 now and increase manually until you reach 1.x)
@aous72
Copy link
Owner

aous72 commented Sep 24, 2024

Thank you Milos. I will look into this.

0.17 has changes to both library API and apps.

@kmilos
Copy link
Author

kmilos commented Sep 24, 2024

Thanks. There was e.g. no such change between 0.15 and 0.16, it was "just" performance dynamically linked apps would get for free w/o a real need to rebuild... So, could've been a patch update only, but I get why the performance bump might be seen as a more significant one. Anyhow, I guess a separate API/ABI versioning might provide a way to manage releases in a more flexible fashion. Or just be more strict on the patch vs minor...

@hmaarrfk
Copy link

One of the hiccups I faced regarding this is that the "version" is included in the .lib extension on windows.

This is the first time I have seen a "Version identifier" in a lib file.

It would be nice if the .lib didn't have any version identifier.

For example, ffmpeg does:

Library/lib/avcodec.lib
Library/lib/avdevice.lib
Library/lib/avfilter.lib
Library/lib/avformat.lib
Library/lib/avutil.lib

but installs

Library/bin/avcodec-61.dll
Library/bin/avdevice-61.dll
Library/bin/avfilter-10.dll
Library/bin/avformat-61.dll
Library/bin/avutil-59.dll

which has the 'so version' in the dll, but not in the lib file.

https://conda-metadata-app.streamlit.app/?q=conda-forge%2Fwin-64%2Fffmpeg-7.1.0-gpl_hb26d62f_701.conda

In my release of openjph i'm patching out this behavior with conda-forge/openjph-feedstock#1 but I don't know if what you would prefer.

@kmilos
Copy link
Author

kmilos commented Oct 16, 2024

I don't see this problem building w/ MinGW - e.g. in the MSYS2 package one gets w/o any patching

bin/libopenjph-0.17.dll
lib/libopenjph.a
lib/libopenjph.dll.a

(.a is the true static lib built w/ -DBUILD_SHARED_LIBS=OFF and .dll.a is the implib that goes w/ versioned libopenjph-0.17.dll produced by -DBUILD_SHARED_LIBS=ON -DCMAKE_DLL_NAME_WITH_SOVERSION=ON)

Edit: Ah, there is a different MSVC path that could indeed be improved (or simply removed so people can handle it w/ CMake options/properties?)...

@VVD
Copy link

VVD commented Nov 11, 2024

If you change ABI then change (increase) major version number too: 0.16.0 => 1.0.0 instead of 0.17.0 and SONAME libopenjph.0 => libopenjph.1.

@aous72
Copy link
Owner

aous72 commented Nov 11, 2024

Thank you for your suggestion. I think you agree with Milos.

Yes, my worry is that I might need to change the ABI often (I know this is a bad idea, because it breaks compatibility with existing apps) -- then I will end be with a high number for the major.

I feel I am still far away from version 1.0 -- still a lot of work needs to be done.
I am also planning a major restructure starting around Dec/Jan, which will change the whole ABI.

I still need to think it through. Suggestions are welcome.

@hmaarrfk
Copy link

Generally for projects with version "0", the minor version is treated at the "major" lol!!!!

Great work on this, do what feels right for you tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants