Skip to content

qtwayland: propagate wayland#359559

Closed
FliegendeWurst wants to merge 1 commit intoNixOS:stagingfrom
FliegendeWurst:telegram-fix
Closed

qtwayland: propagate wayland#359559
FliegendeWurst wants to merge 1 commit intoNixOS:stagingfrom
FliegendeWurst:telegram-fix

Conversation

@FliegendeWurst
Copy link
Member

@FliegendeWurst FliegendeWurst commented Nov 27, 2024 •
edited
Loading

Some dependencies like telegram-desktop require it.

Things done

  • Built on platform(s)
    • x86_64-linux (on staging where GCC 14 is the default)
  • Fits CONTRIBUTING.md.

(previously this PR contained a patch to telegram-desktop)

@Aleksanaa Aleksanaa requested a review from NickCao November 27, 2024 13:18
@NickCao
Copy link
Member

NickCao commented Nov 27, 2024

tdesktop releases very frequently, can we wait for the next release?

@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Nov 27, 2024

tdesktop releases very frequently, can we wait for the next release?

Sure. It will take a while before the GCC bump arrives on master anyhow.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 28, 2024
@Mindavi Mindavi marked this pull request as draft November 29, 2024 21:56
@ilya-fedin
Copy link
Contributor

ilya-fedin commented Dec 2, 2024 •
edited
Loading

If this is happening, it means the deps needed for web bots support on Wayland are missing. The real fix would likely be to add qtdeclarative (how did it work before?).

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Dec 2, 2024

If anything, I would rather think it's caused by some change in propagated deps (perhaps qtwayland's?) rather than gcc bump

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Dec 2, 2024

Although looking at staging, qtwayland still propagates qtdeclarative. It could be only any of those three:

  1. QtQuickWidgets is not accessible by the build
  2. QtWaylandCompositor is not accessible by the build
  3. qtwayland is built without wayland_compositor_quick feature

@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Dec 2, 2024 •
edited
Loading

If anything, I would rather think it's caused by some change in propagated deps (perhaps qtwayland's?) rather than gcc bump

desktop-app/lib_webview#117 fixed a real issue.

qtwayland is built without wayland_compositor_quick feature

No, the issue is that qtwayland apparently does not propagate wayland. I have added it here and it now builds successfully.

@FliegendeWurst FliegendeWurst marked this pull request as ready for review December 2, 2024 11:07
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Dec 2, 2024
@ilya-fedin
Copy link
Contributor

ilya-fedin commented Dec 2, 2024

Are you sure? The code doesn't use libwayland. Maybe you had a temporary problem on your end and niw it builds without any changes too?

@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Dec 2, 2024

The build sure fails without it:

In file included from /nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/qsharedpointer.h:12,
                 from /nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/qpointer.h:8,
                 from /nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/QPointer:1,
                 from /build/source/Telegram/lib_base/base/unique_qptr.h:11,
                 from /build/source/Telegram/lib_webview/webview/platform/linux/webview_linux_webkitgtk.cpp:14:
/nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/qsharedpointer_impl.h:932:1: note: candidate: 'template<class X, class T> QSharedPointer<typename QtSharedPointer::RemovePointer<T>::Type> qobject_cast(const QSharedPointer<T>
  932 | qobject_cast(const QSharedPointer<T> &src)
      | ^~~~~~~~~~~~
/nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/qsharedpointer_impl.h:932:1: note:   template argument deduction/substitution failed:
/build/source/Telegram/lib_webview/webview/platform/linux/webview_linux_webkitgtk.cpp:733:68: error: template argument 1 is invalid
  733 |                 if (const auto widget = qobject_cast<QQuickWidget*>(_widget.get())) {
      |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
/nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/qsharedpointer_impl.h:938:1: note: candidate: 'template<class X, class T> QSharedPointer<typename QtSharedPointer::RemovePointer<T>::Type> qobject_cast(const QWeakPointer<T>&>
  938 | qobject_cast(const QWeakPointer<T> &src)
      | ^~~~~~~~~~~~
/nix/store/ibsn29ly4v1vjkbc959yb2z9fvyrvpnx-qtbase-6.8.0/include/QtCore/qsharedpointer_impl.h:938:1: note:   template argument deduction/substitution failed:
/build/source/Telegram/lib_webview/webview/platform/linux/webview_linux_webkitgtk.cpp:733:68: error: template argument 1 is invalid
  733 |                 if (const auto widget = qobject_cast<QQuickWidget*>(_widget.get())) {
      |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~

@FliegendeWurst
Copy link
Member Author

FliegendeWurst commented Dec 2, 2024

The configure log indicates Wayland is required: (this is without my change)

-- Could NOT find Wayland_Client (missing: Wayland_Client_LIBRARY Wayland_Client_INCLUDE_DIR) (found version "")
-- Could NOT find Wayland_Server (missing: Wayland_Server_LIBRARY Wayland_Server_INCLUDE_DIR) (found version "")
-- Could NOT find Wayland_Cursor (missing: Wayland_Cursor_LIBRARY Wayland_Cursor_INCLUDE_DIR) (found version "")
-- Could NOT find Wayland_Egl (missing: Wayland_Egl_LIBRARY Wayland_Egl_INCLUDE_DIR Wayland_Client_FOUND) (found version "")
-- Could NOT find Wayland_Client (missing: Wayland_Client_LIBRARY Wayland_Client_INCLUDE_DIR) (found version "")
-- Could NOT find Wayland_Server (missing: Wayland_Server_LIBRARY Wayland_Server_INCLUDE_DIR) (found version "")
-- Could NOT find Wayland_Cursor (missing: Wayland_Cursor_LIBRARY Wayland_Cursor_INCLUDE_DIR) (found version "")
-- Could NOT find Wayland_Egl (missing: Wayland_Egl_LIBRARY Wayland_Egl_INCLUDE_DIR Wayland_Client_FOUND) (found version "")

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Dec 2, 2024 •
edited
Loading

This means that qtwayland's cmake config requires libwayland to be installed (as it marks QtWaylandCompositor as not found due to those errors) and the right fix would be to move wayland from buildInputs to propagatedBuildInpus of qtwayland.

@FliegendeWurst FliegendeWurst force-pushed the telegram-fix branch 2 times, most recently from 0167fdd to 1269e74 Compare December 2, 2024 18:09
@FliegendeWurst FliegendeWurst changed the title telegram-desktop: fix build with GCC 14 qtwayland: propagate wayland Dec 2, 2024
@github-actions github-actions bot added 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 2, 2024
@FliegendeWurst FliegendeWurst changed the base branch from master to staging December 2, 2024 18:17
@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Dec 2, 2024
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: vim Advanced text editor 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 6.topic: xen-project Issues and PRs related to the Xen Project Hypervisor. labels Dec 2, 2024
@FliegendeWurst
qt6Packages.qtwayland: propagate wayland
c50f540 
This is required for e.g. telegram-desktop.
@github-actions github-actions bot removed 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: vim Advanced text editor 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 6.topic: xen-project Issues and PRs related to the Xen Project Hypervisor. labels Dec 2, 2024
@NickCao NickCao mentioned this pull request Dec 2, 2024
Merged
13 tasks
K900
K900 reviewed Dec 2, 2024
View reviewed changes
pkgs/development/libraries/qt-6/modules/qtwayland.nix
# (for the wayland-scanner binary) and host (for the
# actual wayland.xml protocol definition)
propagatedBuildInputs = [ qtbase qtdeclarative wayland-scanner ];
propagatedNativeBuildInputs = [ wayland wayland-scanner ];
Copy link
Contributor

@K900 K900 Dec 2, 2024

Choose a reason for hiding this comment

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

Wayland needs to still be in propagatedNativeBuildInputs.

Copy link
Member Author

@FliegendeWurst FliegendeWurst Dec 2, 2024

Choose a reason for hiding this comment

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

Are you sure? There are no binaries in wayland.

Copy link
Contributor

@K900 K900 Dec 2, 2024

Choose a reason for hiding this comment

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

Yes, it has to do with cmake weirdness and the wayland.xml protocol definition.

@K900
Copy link
Contributor

K900 commented Dec 2, 2024

Included in #361047.

@K900 K900 closed this Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@K900 K900 K900 left review comments

@NickCao NickCao Awaiting requested review from NickCao

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

@ttuegel ttuegel Awaiting requested review from ttuegel

Assignees

No one assigned

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@FliegendeWurst @NickCao @ilya-fedin @K900