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

fix the linker error on ubuntu 24.04 #3559

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

georgeliao
Copy link
Contributor

@georgeliao georgeliao commented Jul 3, 2024

close #3558

A minimalistic example to reproduce this is to have an executable in Multipass CMake environment links to Qt6::Network and ssh where ssh target is built by the git submodule library libssh of multipass.

The problem here is that the executable links to libQt6Network.so.6.4.2->libcurl-gnutls.so.4->libssh.so.4 and 3rd-party libssh. So there are two libssh shared libraries linked to the same executable. Symbol versioning is a mechanism used in shared libraries to manage changes in the library API over time. In this case, the system libssh has the symbol version that linker is looking for but 3rd-party one does not have. Besides, the linker apparently is looking into the 3rd-party libssh for the functions he needs as opposed to the system libssh.

So one possible fix for this is to disambiguate the double libssh libraries by changing the 3rd-party libssh library from shared library to static library.

Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.85%. Comparing base (9a9bcf8) to head (d2d4a82).
Report is 88 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3559      +/-   ##
==========================================
- Coverage   88.88%   88.85%   -0.03%     
==========================================
  Files         254      254              
  Lines       14222    14261      +39     
==========================================
+ Hits        12641    12672      +31     
- Misses       1581     1589       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@georgeliao georgeliao force-pushed the fix_linker_error_ubuntu_24.04 branch from 23dbd39 to cbafad3 Compare July 9, 2024 10:54
@andrei-toterman andrei-toterman marked this pull request as draft September 10, 2024 11:01
@andrei-toterman andrei-toterman marked this pull request as ready for review September 10, 2024 11:01
Copy link
Contributor

@andrei-toterman andrei-toterman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to introduce packaging issues on macos. But macos does not really need this fix anyway, so could you only set those options on Linux?

@georgeliao
Copy link
Contributor Author

This seems to introduce packaging issues on macos. But macos does not really need this fix anyway, so could you only set those options on Linux?

Indeed, it is better to localize the change if windows and macos are fine with the original cmake script, and also I thought that macos build failure was spurious.

@andrei-toterman
Copy link
Contributor

Indeed, it could be that the macos failure was not caused by this. That was just my quick assumption based on the failing script. It failed on the install-ssl-libs.sh script which is looking for libssh.dylib. And since this no longer builds the shared lib, that's why I assumed it failed because of this change.

@townsend2010
Copy link
Contributor

townsend2010 commented Sep 10, 2024

Hey All :)

There is a reason we had to make libssh a dynamic library- it's GPLv2. If you do this, you will need to make all of the source available including Windows and macOS code. Since GPLv2 says you must be able to use another version of the code and if you statically link this, the only way to accomplish that is to make all of the code available.

By making it dynamic, you can much more easily replace the library without touching the Multipass code.

@townsend2010
Copy link
Contributor

Oh, nevermind, the last commit takes care of that. Sorry for the noise, but something to keep in mind if it ever comes up.

@ricab
Copy link
Collaborator

ricab commented Sep 10, 2024

Thanks anyway, nice surprise to see you around 🙂

@townsend2010
Copy link
Contributor

townsend2010 commented Sep 10, 2024

Hey @ricab,

Yeah, there is definitely some history here where I don't want you guys to get into a pickle about. This is the clause that I'm thinking of:
https://www.gnu.org/licenses/gpl-faq.html#LGPLStaticVsDynamic

libssh is LGPLv2, so if you statically link to it, you will need to consider (1).

I still follow you guys and interested in what you are doing, hence me piping up when I see a potential pitfall:)

@andrei-toterman andrei-toterman added this pull request to the merge queue Sep 10, 2024
Merged via the queue into main with commit fa2b290 Sep 10, 2024
13 of 14 checks passed
@andrei-toterman andrei-toterman deleted the fix_linker_error_ubuntu_24.04 branch September 10, 2024 20:49
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

Successfully merging this pull request may close these issues.

Linker error on ubuntu 24.04, it does not occur on ubuntu 23.10
4 participants