lxqt / lxqt-qtplugin

LXQt Qt platform integration plugin
https://lxqt.github.io
GNU Lesser General Public License v2.1
24 stars 14 forks source link

Load versioned libfm-qt #47

Closed agaida closed 5 years ago

agaida commented 5 years ago

We really don't want libfm-qt.so which is clearly a development file refs lxqt/lxqt/issues/391

tsujan commented 5 years ago

Oh, that QLatin1String(FMQT_SO_VERSION) was needed.

GTM.

palinek commented 5 years ago

This is a quick & dirty solution. Would it be feasible to create a private helper library (with the name *.so) which will directly link to libfm-qt? Then lxqt-qtplugin will load this private .so (which in turn will automatically load the right "versioned" libfm-qt library). But I'm not sure about the symbols resolution in this case....

In this scenario libfm-qt will be build dependency of lxqt-qtpugin. But for runtime the libfm-qt can still be just a recommend (loading/resolution will fail in runtime if the "versioned" libfm-qt is not found). And we don't need to manually update the FMQT_SO_VERSION in cmake.

agaida commented 5 years ago

erm - a bit overkill, isn't it? All that is needed is a hint in libfm-qt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 63702d7..f183cc9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -24,6 +24,8 @@ list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake")
 # We use "(current-age).age.revision" as the cmake version.
 # current: 6, revision: 0, age: 0 => version: 6.0.0
 set(LIBFM_QT_ABI_VERSION "6.1.0")
+# IF YOU REALLY THINK THAT BUMPING THE SOVERSION IS A GOOD IDEA
+# PLEASE CHANGE IT IN LXQT-QTPLUGIN TO
 set(LIBFM_QT_SOVERSION "6")

 set(GLIB_MINIMUM_VERSION "2.50.0")

EDIT: Ok, it should be LIBFM_QT_SOVERSION in lxqt-qtplugin too.

EDIT 2: Second thing would be a simple optional dependency on libfm-qt - if found fine, if not - who cares, fallback to native Qt

tsujan commented 5 years ago

IMHO, we need to be fast here because, as we've found out recently, many LXQt users many still not know that LXQt has a feature-rich file dialog.

tsujan commented 5 years ago

if not - who cares, fallback to native Qt

Not an option, IMO.

luis-pereira commented 5 years ago

@agaida Edit 2 is a good solution. Less error prone than this.

luis-pereira commented 5 years ago

@tsujan if libfm-qt is not found there isn't other solution. If it was found, use the version we found.

tsujan commented 5 years ago

@luis-pereira I agree with anything that works; don't want this to be postponed because of probable disagreements.

agaida commented 5 years ago

@luis-pereira - is there some cmake magic for the soversion?

Edit: More verbose -the problem is twofold

@tsujan - this should address your concerns about "So what, we don't care" - in that case we have build time support for the libfm-qt and don't have to care if the libfm-qt isn't available for what reasons ever, user and maintainer might decide strange things.

tsujan commented 5 years ago

... in that case we have build time support for the libfm-qt and don't have to care if the libfm-qt isn't available for what reasons ever, user and maintainer might decide strange things.

Understood.

tsujan commented 5 years ago

BTW, I think it wasn't a bug. Theoretically, QLibrary could find the latest version and load it in Linux but it doesn't do so; hence this workaround.

agaida commented 5 years ago

After thinking of it again my guess is that hard wire the libfm-qt-soversion in CMakeList.txt is the easiest thing for now. It seems impossible for cmake to read out the soversion with find_package. And a further guess: A comment in libfm-qt CMakeLists should be enough for now. It is just not worth the effort.

agaida commented 5 years ago

a bit further - we can merge it like it is - in future (before the next release) we can have a module for lxqt-build-tools that will find a lib and extracts the soname from. Should be easy to implement (a bit shellscripting, grep and standard tools). So with this module we can detect the lib at build time, the rest remain the same.

tsujan commented 5 years ago

we can merge it like it is

I'd say, do it! The fix is too important to be delayed.

agaida commented 5 years ago

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=927984 - the solution in debian is a bit easier :grinning:

tsujan commented 5 years ago

Good :)

luis-pereira commented 5 years ago

@tsujan There is no disagreement here. @agaida Yes, some CMake "magic" can be done. At lxqt-qtplugin we can do:

find_package(fmqt)

# extract so version or soname and pass it to the C++ code

Currently away from my dev machine. Will write a patch next week.

agaida commented 5 years ago

@luis-pereira i had such thing in mind

find_package 
  find_file
    objdump $file 
       done

That would be cool.

luis-pereira commented 5 years ago

@agaida CMake IMPORTED targets features provides the answer cleanly.

tsujan commented 5 years ago

That would be great; it would prevent future problems.

agaida commented 5 years ago

@luis-pereira nice, i'm not that deep into cmake how i should be.