diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.07k stars 795 forks source link

[Issue Report]: Manjaro does not have libsodium version 23 #6793

Closed coreybruce closed 1 year ago

coreybruce commented 1 year ago

Operating System

Linux x64

DevilutionX version

1.5.1

Describe

Unable to find libsodium.so.23 library which doesn't exist in my system but I have libsodium.sos, libsodium.so.26 and libsodium.so.26.1.0

I can work around the issue by doing sudo ln -s /usr/lib/libsodium.so.26.1.0 /usr/lib/libsodium.so.23 but it should be finding the library file on it's own.

I am on Manjaro x64

To Reproduce

Run game

Expected Behavior

finds library file in /usr/lib and works

Additional context

No response

StephenCWills commented 1 year ago

I did it because Arch/Manjaro x64 have the library updated and Manjaro Arm64 hasn't has this update yet and to fix the issue, I don't want to have to recompile on x64, Arm64 and i386 hardware everytime this issue randomly occurs and be told by users that there is a missing dependency. I will remove the fix when the next release comes out also there hasn't been any proof of any unknown side effects.

Wait, hang on. If you are repackaging our software on Manjaro, and you have users who depend on that package to receive the software, then you absolutely should not use that workaround. Rebuild DevilutionX against the new version of libsodium and update your package to reference the new version of libsodium. That is the correct way to handle this. When Arm64 gets the updated version, do the same over there.

It sounds to me like you're just trying to take shortcuts because the package maintainers for libsodium couldn't roll out the update for all platforms all at once. I do not like that idea even a little bit.

In all of my time compiling software or repackaging them I have never ran into this issue except for here

So what happens in your other software when a dependency gets updated? Does that software use static linking or dynamic linking? Does it compile using CMake? Do those developers specify version numbers in their Makefiles?

If you're going to point to other projects and say we're doing it wrong, at least tell us what they are. These details matter.

AJenbo commented 1 year ago

I don't want to have to recompile on x64, Arm64 and i386 hardware everytime this issue randomly occurs and be told by users that there is a missing dependency.

Then for the love of god just set the lib to be statically linked, it's already an option, you just have to select it. Why insist on making the same mistake and then doing a hack on top of it instead of just doing it the right way.

I tested that, it doesn't work

Looks like you tested with an empty file.

there hasn't been any proof of any unknown side effects.

I for one trust the Debian/Manjaro maintainers know what they are doing when they decide that a breaking change is needed.

I only mentioned a workaround and suggestions of changes in the makefile itself to resolve a issue for when it gets compiled and reporting a issue so we can work together to create a better solution.

It's very hard to see how we can work together when you are not receptive to input. Like I stated two days ago we are already aware of the mistake that was made during the release, and there already is a proper solution. static linking, yet you keep arguing over it to no end.

coreybruce commented 1 year ago

I obviously have to because of the issue at hand and since you know about the issue and have a solution this is only a temporary solution until next release with the fix which I replied to 20 hours with the fix so it's not permanent

Looks like you tested with an empty file.

Is that what you meant by dummy file exactly or did you mean in a different way? again I was only doing a quick test on this and wasn't anything serious lol

I for one trust the Debian/Manjaro maintainers know what they are doing when they decide that a breaking change is needed.

I assume they just updated the dependency as it was the newest release not knowing that other projects would run into some issues.

It's very hard to see how we can work together when you are not receptive to input. Like I stated two days ago we are already aware of the mistake that was made during the release, and there already is a proper solution. static linking, yet you keep arguing over it to no end.

Yes and I said 20 hours ago "Awesome thanks for letting me know πŸ‘" and you kept continuing the conversation as if I didn't see that

AJenbo commented 1 year ago

I obviously have to because of the issue at hand

No you don't, use static linking, there is no need to wait for the next release, the support is already there.

Yes and I said 20 hours ago "Awesome thanks for letting me know πŸ‘" and you kept continuing the conversation as if I didn't see that

The problem is not you not seeing it, I know you did, the problem is you not understanding what it means and arguing against us when we try to explain it to you.

coreybruce commented 1 year ago

I obviously have to because of the issue at hand

No you don't, use static linking, there is no need to wait for the next release, the support is already there.

The libsodium.so file is a symlink to libsodium.so.26 that was done by the package developers and will always be symlinked to the proper lib file libsodium.so.26 which was how the developer packaged it, how exactly would this cause unknown issues if this is by the developers design? this isn't a random symlink that is created by me or to a random newer version of the library. the is doing exactly as your suggesting and will link to the current version of the library on the system without the issue of the llibrary ink breaking when the package does get updated.From what we talked about before do you guys just not know if the game works with newer versions of libsodium and afraid of something breaking even tho there hasn't been any evidence of that being the case? I get you guys want to control the libraries but trying to stick to a certain lib version is only going to cause a lot of potential issues, breakage and a lot of recompiling compared to just using the lib and then if anything does come up do some fixes for newer versions if that does come up which would be less likely but of course I am not saying it wouldn't ever happen but that isn't the case here.

image

image

StephenCWills commented 1 year ago

I get you guys want to control the libraries...

Then you don't get it at all. We are not trying to control libraries. When I said we don't have control over "it", I meant that we don't have control over what version we link against. I showed you that we're not referencing any specific version. CMake is our build system, and it generates the Makefiles. It chooses the appropriate library file on the system to link against.

AJenbo commented 1 year ago

how exactly would this cause unknown issues if this is by the developers design

Because the there where several ABI breakage between libsodium.so.23 and libsodium.so.26. We don't know what they are since we are not maintainers of that lib, but they are telling us not to use the other version because there are parts that are incompatible between them and doing so will lead to unexpected behavior that we cannot easily identify. Trying to investigate what exactly has changed it moot since the right way to solve it across system updates it so statically link it.

If you are truly interested in what issues there are with using libsodium.so.26 in applications build against libsodium.so.23 then I suggest you go talk to the developers or package maintainers they are the experts on this, we are not.

coreybruce commented 1 year ago

I get you guys want to control the libraries...

Then you don't get it at all. We are not trying to control libraries. When I said we don't have control over "it", I meant that we don't have control over what version we link against. I showed you that we're not referencing any specific version. CMake is our build system, and it generates the Makefiles. It chooses the appropriate library file on the system to link against.

You aren't understanding what I am saying, I am not specially saying you are controlling the library or have control over it, I am saying that yes you don't tell it look for a specific version yet when it does look for the library isn't using the symlink lib provided by the libsodium developer and instead picks libsodium.so.23 when it was built or the lib at the time of being built which causes issues.

coreybruce commented 1 year ago

Because the there where several ABI breakage between libsodium.so.23 and libsodium.so.26. We don't know what they are since we are not maintainers of that lib, but they are telling us not to use the other version because there are parts that are incompatible between them and doing so will lead to unexpected behavior that we cannot easily identify. Trying to investigate what exactly has changed it moot since the right way to solve it across system updates it so statically link it.

Can you show/tell me what api changes break devilutionx exactly or am I assuming no because you don't know yet?

I will leave the conversion of this issue post here for now, thank you for your replies

AJenbo commented 1 year ago

it does look for the library isn't using the symlink lib provided by the libsodium developer and instead picks libsodium.so.23 when it was built or the lib at the time of being built which causes issues.

This simply sounds like guess work, please try to listen to what we are saying or look up the actual documentation.

Can you show/tell me what api changes break devilutionx exactly or am I assuming no because you don't know yet?

In the very same message you quoted I told you that you should ask the libsodium maintainers if you want to know exactly what ABI breakage exists between the two version. Please break out of fight mode and try to collaborate.

stick to a certain lib version is only going to cause a lot of potential issues, breakage and a lot of recompiling compared to just using the lib and then if anything does come up do some fixes for newer versions

None of these problems exist with static linking. Why are you ignoring this?

StephenCWills commented 1 year ago

You aren't understanding what I am saying, I am not specially saying you are controlling the library or have control over it, I am saying that yes you don't tell it look for a specific version yet when it does look for the library isn't using the symlink lib provided by the libsodium developer and instead picks libsodium.so.23 when it was built or the lib at the time of being built which causes issues.

There is not a single reference to libsodium.so.23 in any of the CMake variables. Actually, it is explicitly referencing libsodium.so.

image

And yet the binary still references libsodium.so.23 after linking.

$ readelf -d devilutionx

Dynamic section at offset 0x75fc90 contains 35 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libz.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libbz2.so.1.0]
 0x0000000000000001 (NEEDED)             Shared library: [libSDL2_image-2.0.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libsodium.so.23]
 0x0000000000000001 (NEEDED)             Shared library: [libSDL2-2.0.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]

I don't know what you want us to do about this. You don't know what you want us to do about this. You just want to complain that I'm not understanding you. You are the one who is not understanding. There is nothing that I can do for you.

StephenCWills commented 1 year ago

Actually, it seems there is one thing I can do for you. I was wrong about the fact that CMake picks the file we link against. AJenbo's claim was more accurate that it's the linker, but I guess even that is a bit misleading. It comes from the SONAME of the libsodium.so library itself.

https://discourse.cmake.org/t/linking-against-non-versioned-or-major-library/3931/2

The version that will be written to the NEEDED section depends on the SONAME of the linked library. That means the linker reads it from the file your /usr/lib/x86_64-linux-gnu/libnova.so ultimately points to. This has nothing to do with CMake, it’s how the linking works.

$ objdump -p /usr/lib/x86_64-linux-gnu/libsodium.so | grep SONAME
  SONAME               libsodium.so.23
AJenbo commented 1 year ago

As you might notice most other libs are very conservative with breaking there ABI and thus has a very low numbers (0-6) compared to libsodium which has a very unstable ABI and has already reached 23 despite being much younger. This might be why have seen this issue with programs that only depend on libs with a stable ABIs (again I encurage you to play around with ldd/readelf). This is also why it should been statically linked if the intent is for the binary to be unaffected by system changes. But there is no need for you to wait for the next release, you can simply statically link it yourself.

if anything does come up do some fixes for newer versions

You likely wouldn't need to, there API is stable so you simply build the applikation again for it to link against the new ABI.

coreybruce commented 1 year ago

Ah I see

You likely wouldn't need to, there API is stable so you simply build the applikation again for it to link against the new ABI.

Oh I see, how would you go doing that?

AJenbo commented 1 year ago

Just do this:

cmake -S. -Bbuild -DCMAKE_BUILD_TYPE=Release -DDEVILUTIONX_SYSTEM_LIBSODIUM=OFF -DDEVILUTIONX_STATIC_LIBSODIUM=ON
cmake --build build -j $(getconf _NPROCESSORS_ONLN)
coreybruce commented 1 year ago

Ah I see, well thanks for letting me know :+1: