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

Add CMake option to disable Wayland support code #1155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sideeffect42
Copy link

I added a CMake option to disable Wayland support code requiring LayerShellQt.

The dependency on LayerShellQt generates quite a few extra dependencies which are not used for people running QTerminal only on X11.

@tsujan
Copy link
Member

tsujan commented Jul 19, 2024

IMO, this isn't acceptable anywhere in LXQt. You may dislike Wayland, but it's unavoidable in the long rum.

LayerShellQt is just a small dependency, whose removal doesn't justify adding conditions that could easily complicate development and maintenance.

@sideeffect42
Copy link
Author

This PR is not about whether or not I like Wayland. I'm agnostic, actually.
It's just that my window manager is X11 and so I don't see a reason to clobber the system with Wayland-only libraries which will never be executed.

I've seen other software having options to enable or disable Wayland/X11 support at compile-time (including Qt itself).
Is wrapping Wayland-specific (or X11-specific for that matter) code in an #ifdef really so complicated?
Currently it's a dozen of lines in total and I would go so far as to say that QTerminal (in an ideal world) shouldn't require any display server specific code at all.

While LayerShellQt may be a small dependency, it comes with a rat tail of other requirements.
On my system this "small dependency" triggers changes in at least 6 other packages:

[ebuild  N     ] x11-terms/qterminal-2.0.1-r1::gentoo  USE="-test" 225 KiB
[ebuild  N     ]  kde-plasma/layer-shell-qt-6.1.3:6::gentoo  USE="-debug" 36 KiB
[nomerge       ] kde-plasma/layer-shell-qt-6.1.3:6::gentoo  USE="-debug" 
[ebuild  NS    ]  kde-frameworks/kf-env-6:6::gentoo [5:5::gentoo] 0 KiB
[nomerge       ] kde-plasma/layer-shell-qt-6.1.3:6::gentoo  USE="-debug" 
[ebuild  N     ]  dev-qt/qtwayland-6.7.2-r3:6/6.7.2::gentoo  USE="vulkan -accessibility -compositor -qml -test" 1,097 KiB
[ebuild     U  ]   dev-qt/qtbase-6.7.2-r1:6/6.7.2::gentoo [6.7.2:6/6.7.2::gentoo] USE="X concurrent cups dbus gles2-only gtk gui icu libinput libproxy network nls opengl sql sqlite ssl udev vulkan wayland* widgets xml (zstd) -accessibility -brotli -eglfs -evdev -gssapi -mysql (-oci8) -odbc -postgres (-renderdoc) -sctp -test -tslib" 0 KiB
[ebuild  N     ]  dev-libs/wayland-protocols-1.36::gentoo  USE="-test" 94 KiB
[ebuild  N     ]  dev-libs/wayland-1.23.0::gentoo  USE="-doc -test" 233 KiB
[ebuild  N     ]   dev-util/wayland-scanner-1.23.0::gentoo  0 KiB

LXQt is about lightness, isn't it?

@tsujan
Copy link
Member

tsujan commented Jul 19, 2024

On Arch, the package of layer-shell-qt (33 KiB) depends on qt6-wayland (1.1 MiB) that depends on wayland (137 KiB). Their installation takes < 9 MiB of disk space.

Anyway, that's not the point.

LXQt is about lightness, isn't it?

Yes, but it isn't a bare-bones DE either.

@sideeffect42
Copy link
Author

On Arch, the package of layer-shell-qt (33 KiB) depends on qt6-wayland (1.1 MiB) that depends on wayland (137 KiB). Their installation takes < 9 MiB of disk space.

Anyway, that's not the point.

Correct, the point is not installed binary size.
On Gentoo the build dependencies also need to be installed, that's why it's a few more.

In any case: are you suggesting me to recompile Qt with Wayland support, compile the Qt Wayland module and the Wayland libraries which I won't be able to use because my window manager does not support Wayland?
Just to avoid #ifdefing 5 lines of code?

Or is QTerminal just not meant to be used outside of the LXQt desktop environment and I should be looking for an alternative instead?

@stefonarch
Copy link
Member

I'm half-half on this, it's not a invasive change as there are a few lines only to disable a small block. The point is if we accept it in QTerminal why not in pcmanfm-qt or in the other components? We could accept it in apps but not in the DE?

@sideeffect42
Copy link
Author

@stefonarch pcmanfm-qt would've been my next target, indeed.

@tsujan
Copy link
Member

tsujan commented Jul 19, 2024

I'm half-half on this

I'm against it for the reason I clearly mentioned in my first comment. Also, I don't have time for reports like this:

— It doesn't work on LabWC/Wayfire/....
— Have you compiled it with LayerShellQt?
— What's LayerShellQt?
....

@tsujan
Copy link
Member

tsujan commented Jul 19, 2024

Or is QTerminal just not meant to be used outside of the LXQt desktop environment and I should be looking for an alternative instead?

This was irrelevant. Of course QTerminal can be used outside LXQt or without Wayland.

But if you want a bare-bones system, you'll have to install bare-bones programs.

@sideeffect42
Copy link
Author

sideeffect42 commented Jul 19, 2024

I don't think this change will be the source of many reports.

First of all, the option is default on, so you have to actively disable it.
And most people won't be compiling from source. Binary distros will enable the option anyway because they likely also ship Wayland-enabled DEs and so will want it.

And it will print an error message on start-up when running on Wayland with -DENABLE_WAYLAND=OFF

@stefonarch
Copy link
Member

For me for the applications only this option would be fine. If it will generate timeconsuming issues we can still revert it but I don't think so as most distros are shipping wayland.

For the DE it's a not necessary because we depend on kwindowsystem/kscreen which depends among other on layer-shell-qt anyway.

Let's hear others maybe.

@tsujan
Copy link
Member

tsujan commented Jul 19, 2024

Also see tsujan/Kvantum#948. It's as "valid" as this.

I think we should avoid changing our codes because someone wants to compile Qt without X11 or Wayland. Of course, they have right to do so, but they also have to accept the consequences.

Last but not least, as the new maintainer (or half-Maintainer) of QTerminal, and considering that I also maintain libfm-qt, pcmanfm-qt, lximage-qt,..., I should manage my time efficiently. That requires having an approach different from that of the previous maintainer. We lack manpower for these things.

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.

3 participants