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

kdePackages: add qtwayland to ensure wayland platform support #341028

Closed
wants to merge 1 commit into from

Conversation

eclairevoyant
Copy link
Contributor

Description of changes

Fixes #305959, couldn't find any other similar issues for some reason

Tested this with kdePackages.kruler, it now starts with QT_QPA_PLATFORM=wayland

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

I'd rather have it in wrapQtAppsHook, as previously discussed.

@eclairevoyant
Copy link
Contributor Author

Where was this previously discussed?
And I strongly disagree, propagation is causing us more problems than it solves. #333012/#269674 was the wrong approach.

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

Why is propagation the wrong approach here?

@eclairevoyant
Copy link
Contributor Author

#333012 (comment)

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

I don't follow. Propagated inputs follow the same offsets as normal, so they should not be causing issues on cross. Even if they are, the hook could simply add qtwayland to QT_PLUGIN_PATH without propagating it.

@Aleksanaa
Copy link
Member

My understanding is that there's some black magic inside Qt toolkit that no one has enough patience and time to figure it out and workaround it correctly

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

Qt cross is currently broken, but I don't think propagating qtwayland would make it any more broken.

@eclairevoyant
Copy link
Contributor Author

We're taking a step backwards with propagation, why should we actively make cross worse?
Surely we have to fix things one step at a time if no one's fixing it all at once.

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

You're still assuming propagation makes cross worse. It doesn't.

@eclairevoyant
Copy link
Contributor Author

How is picking the wrong arch's setup hook better for cross? Please explain that.

@Aleksanaa
Copy link
Member

But it's not efficient to always tell people "hey you should include qtwayland now" and adding qtwayland to plugin paths will bring incompatibility between Qt versions. We just end up with a very broken user experience with nearly all Qt6 programs now, not only kde6 ones

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

How would this be picking the wrong setup hook?

@eclairevoyant
Copy link
Contributor Author

How would this be picking the wrong setup hook?

If I knew the answer to "why"/"how", I'd simply fix it and we wouldn't be having this discussion.
All I can observe is that it happens, without knowing the cause, and it took me hours to even figure that part out.

After searching around a bit, it seems that someone else has observed this as well (#269756 (comment)) and they've tried to fix in qt5 here: #267311

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

This is Qt5, which is an entirely different build system, with an entirely different set of problems.

@eclairevoyant
Copy link
Contributor Author

Well according to https://doc.qt.io/qt-6/qmake-environment-reference.html hardcoding QMAKE_XSPEC is still a thing, it looks pretty identical to the qt5 page so I'm not sure why you say it's an entirely different system

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

Because Qt6 builds with CMake, not qmake.

@eclairevoyant
Copy link
Contributor Author

Okay, please feel free to create a PR that better reflects the Qt maintainer intentions then. I'd be happy to take a look and learn.

@ilya-fedin
Copy link
Contributor

qtwayland isn't the only plugin useful for a wide range of apps. For instance, kimageformats is such. And there are likely many others.

IMO, the right way would be to have Qt plugins configurable via a NixOS module, to have global set of plugins for any application, just like Qt expects it to be.

Also, #333012 doesn't look like a real bug: Qt is supposed to fallback to xcb. What happened perhaps is that the user sets QT_QPA_PLATFORM=wayland that force disables the fallback, yet doesn't add qtwayland to systemPackages. That's a user fault IMO.

@NickCao
Copy link
Member

NickCao commented Sep 10, 2024

IMO, the right way would be to have Qt plugins configurable via a NixOS module, to have global set of plugins for any application, just like Qt expects it to be.

@K900 aren't we already doing this with KDE (indirectly via systemPackages and our search for qt plugins from a path derived from PATH patch), can we expand that approach to all desktop envs?

@K900
Copy link
Contributor

K900 commented Sep 11, 2024

We are already doing that for everything, really, you can just qt.enable = true; and add whatever plugins to environment.systemPackages.

@ilya-fedin
Copy link
Contributor

Maybe the point is to make it more discoverable, e.g. have some qt.wayland = true; that would add qtwayland 5 and 6 packages to systemPackages

@K900
Copy link
Contributor

K900 commented Sep 11, 2024

We could add options like that pretty easily, if there's interest.

@SuperSandro2000
Copy link
Member

Do we even need an option? Just add it by default here

environment.systemPackages =

@K900
Copy link
Contributor

K900 commented Sep 11, 2024

Actually, now that I think about it, one issue with this approach is that we end up breaking applications with mismatched Qt versions.

@ilya-fedin
Copy link
Contributor

Qt 5 and Qt 6 have a different NIXPKGS_QT_PLUGIN_PREFIX. In fact this makes adding plugins to systemPackages safer than setting QT_PLUGIN_PATH.

@K900
Copy link
Contributor

K900 commented Sep 11, 2024

I don't mean a different major Qt version, I mean different minor Qt versions.

@ilya-fedin
Copy link
Contributor

Then you can just add full version to the prefix?

@K900
Copy link
Contributor

K900 commented Sep 11, 2024

But how would we provide plugins for older Qt versions?

@ilya-fedin
Copy link
Contributor

Just add it by default here

The point is to prevent reports from people who don't know how Qt works (that they need a package in first place) and using a non-Qt DE/WM. A qt.wayland option adding qtwayland to systemPackages and setting QT_QPA_PLATFORM=wayland;xcb for Qt 5 would prevent the most annoying issues (people manually setting wrong QT_QPA_PLATFORM and not installing the package).

@ilya-fedin
Copy link
Contributor

But how would we provide plugins for older Qt versions?

The same way older Qt packages are provided? Not loading plugins of other version is a better UX than a crash IMO.

@K900
Copy link
Contributor

K900 commented Sep 11, 2024

There is no crash, Qt will ignore mismatched plugins. The question is that maybe we want at least qtwayland by default, so applications from other nixpkgs don't just break on Wayland-only.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Sep 11, 2024

This is actually upstream question, IMO. One has to install qtwayland manually on any user-centric distro (such as Arch or Debian).

QT_QPA_DEFAULT_PLATFORM is still xcb and changing it to wayland would likely require moving the platform plugin into qtbase so maybe it's worth to ask about changing QT_QPA_DEFAULT_PLATFORM upstream?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

skanpage: failing to start on sway
6 participants