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 plugin loader (#2052) #2053

Closed
wants to merge 1 commit into from
Closed

Conversation

berolinux
Copy link

The lxqt-plugin binary needs to export symbols that should be visible to plugins being loaded with QPluginLoader or dlopen.

Fixes #2052

The lxqt-plugin binary needs to export symbols that should be visible to
plugins being loaded with QPluginLoader or dlopen.
@tsujan tsujan requested review from luis-pereira and palinek April 20, 2024 20:31
@yan12125
Copy link
Member

Thanks for the fix, I can confirm it works. Just an issue: https://cmake.org/cmake/help/latest/variable/CMAKE_EXECUTABLE_ENABLE_EXPORTS.html says CMAKE_EXECUTABLE_ENABLE_EXPORTS is added in CMake 3.27. I'm not sure if it's too new for Linux/BSD/... distributions.

@stefonarch
Copy link
Member

Indeed it's on the edge:
https://repology.org/project/cmake/versions

Copy link
Member

@luis-pereira luis-pereira left a comment

Choose a reason for hiding this comment

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

CMAKE_EXECUTABLE_ENABLE_EXPORTS was introduces in version 3.27 released in Jul 2023. Too recent IMO.

We can use the ENABLE_EXPORTS property:

set_property(TARGET ${PROJECT} PROPERTY ENABLE_EXPORTS TRUE)

It fixes the problem and it's available in the CMake versions we support.

@tsujan
Copy link
Member

tsujan commented May 6, 2024

This may deserve a point release. Please decide what should be done. @luis-pereira, @yan12125?

@yan12125
Copy link
Member

yan12125 commented May 6, 2024

We can use the ENABLE_EXPORTS property:

This approach looks good (not tested yet)

@luis-pereira
Copy link
Member

This approach looks good (not tested yet)

Pls test. It worked on my test.

@luis-pereira
Copy link
Member

This may deserve a point release. Please decide what should be done. @luis-pereira, @yan12125?

If ENABLE_EXPORTS is found to work, we should use it, IMO.
It deserves a point release, IMO.

@Chiitoo
Copy link
Contributor

Chiitoo commented May 6, 2024

This approach looks good (not tested yet)

Pls test. It worked on my test.

For what it is worth, worked for me.

Thank you!

@tsujan
Copy link
Member

tsujan commented May 6, 2024

@luis-pereira
Would you please make a PR?

luis-pereira added a commit that referenced this pull request May 6, 2024
The plugins must export the symbols to allow loading by the panel
executable.
CMAKE_EXECUTABLE_ENABLE_EXPORTS is too new for us.

Supersedes #2053.
Fixes #2052.
@luis-pereira
Copy link
Member

@tsujan Done here
@berolinux Thanks for your work.

tsujan pushed a commit that referenced this pull request May 7, 2024
The plugins must export the symbols to allow loading by the panel
executable.
CMAKE_EXECUTABLE_ENABLE_EXPORTS is too new for us.

Supersedes #2053.
Fixes #2052.
@yan12125
Copy link
Member

yan12125 commented May 7, 2024

Superseded by #2061

@yan12125 yan12125 closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants