lxqt / lxqt-config

Tools to configure LXQt and the underlying operating system
https://lxqt.github.io
GNU Lesser General Public License v2.1
83 stars 61 forks source link

Missing kscreen includes cause build failures. #903

Closed Chiitoo closed 1 year ago

Chiitoo commented 1 year ago
Expected Behavior

(I'd rather make a pull request but I haven't had the time to prepare it right... so I'll create this issue at least while I can!)

I'm not sure this is in any release version of KScreen yet, but there are some new requirements for headers (possibly related to 38113506382 [1]).

  1. https://invent.kde.org/plasma/kscreen/-/commit/38113506382d28f7cb44a8ecd9911817649e6cde
Current Behavior

One example of a point of failure:

[...]/lxqt-config-monitor/monitorwidget.cpp:111:25: error: invalid use of incomplete type ‘class KScreen::Mode’
  111 |                 if( mode->size() == output->currentMode()->size() )
      |
Possible Solution

Add #include <kscreen/mode.h> (and friends) in monitorwidget.cpp and friends.

Steps to Reproduce (for bugs)
  1. Build 'lxqt-config' with 'kscreen' from 'git master'.
Context

Unable to build 'lxqt-config' without patching it.

System Information

Thank you!

tsujan commented 1 year ago

... with 'kscreen' from 'git master'.

A wild guess: they may be preparing kscreen for Qt6. If that's the case, the required changes will be in the Qt6 port of lxqt-config.

rikmills commented 1 year ago

In a Ubuntu build against the Plasma 5.27 released beta tars:

In file included from /usr/include/KF5/KScreen/KScreen/Config:1,
                 from /<<PKGBUILDDIR>>/obj-x86_64-linux-gnu/lxqt-config-monitor/lxqt-config-monitor_autogen/EWIEGA46WW/../../../../lxqt-config-monitor/fastmenu.h:28,
                 from /<<PKGBUILDDIR>>/obj-x86_64-linux-gnu/lxqt-config-monitor/lxqt-config-monitor_autogen/EWIEGA46WW/moc_fastmenu.cpp:10,
                 from /<<PKGBUILDDIR>>/obj-x86_64-linux-gnu/lxqt-config-monitor/lxqt-config-monitor_autogen/mocs_compilation.cpp:2:
/usr/include/KF5/KScreen/kscreen/config.h:197:32: error: ‘std::optional’ has not been declared
  197 |     void adjustPriorities(std::optional<OutputPtr> keep = std::nullopt);
      |                                ^~~~~~~~
/usr/include/KF5/KScreen/kscreen/config.h:197:40: error: expected ‘,’ or ‘...’ before ‘<’ token
  197 |     void adjustPriorities(std::optional<OutputPtr> keep = std::nullopt);
      |                                        ^
make[3]: *** [lxqt-config-monitor/CMakeFiles/lxqt-config-monitor.dir/build.make:231: lxqt-config-monitor/CMakeFiles/lxqt-config-monitor.dir/lxqt-config-monitor_autogen/mocs_compilation.cpp.o] Error 1
make[3]: Leaving directory '/<<PKGBUILDDIR>>/obj-x86_64-linux-gnu'
make[2]: *** [CMakeFiles/Makefile2:639: lxqt-config-monitor/CMakeFiles/lxqt-config-monitor.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....
[ 66%] Building CXX object liblxqt-config-cur
palinek commented 1 year ago

/usr/include/KF5/KScreen/kscreen/config.h:197:32: error: ‘std::optional’ has not been declared 197 | void adjustPriorities(std::optional keep = std::nullopt);

This is to be reported to KScreen. The header should include everything needed to not make the compilation fail. This is probably caused by some upstream clean up (e.g. std lib).

Chiitoo commented 1 year ago

The 'optional' issue is related to C++17 standard I believe, or at least adding some '::experimental::' bits will help with that one here.

If I remember right, 'lxqt-config' is building with C++14, but I haven't had the time to look into if that can cause it here (probably not (or maybe yes)).

nicolasfella commented 1 year ago

config.h correctly includes <optional>, the problem here it that libkscreen requires C++17, so you have to build it accordingly

nicolasfella commented 1 year ago

A wild guess: they may be preparing kscreen for Qt6. If that's the case, the required changes will be in the Qt6 port of lxqt-config.

kscreen does not (yet) use Qt6. The problem here is simply a missing include, so the suggested solution is correct

mtasaka commented 1 year ago

So currently two issue is here, not one.

  1. By https://github.com/KDE/libkscreen/commit/68a389c717ad97d2f57f534040ecdf8458f8e8b0 , with libkscreen 5.26.90, using #include <KScreen/Config> now uses #include <optional> and std::optional, which needs at least std=c++17. This is the report of https://github.com/lxqt/lxqt-config/issues/903#issuecomment-1403442877 . This should be handled by https://github.com/lxqt/lxqt-build-tools/pull/83 .

  2. Even if https://github.com/lxqt/lxqt-build-tools/pull/83 is applied, due to https://github.com/KDE/libkscreen/commit/94f330959b0eda775418aef7faee80ce69144e63 , #include <KScreen/Output> no longer includes "mode.h" implicitly. This is the report of https://github.com/lxqt/lxqt-config/issues/903#issue-1508533950 . So lxqt-config source using class KScreen::Mode should include #include <KScreen/Mode> explicitly, which should be handed by https://github.com/lxqt/lxqt-config/pull/915 .

tsimonq2 commented 1 year ago

Should be solved now, with those two aforementioned patches.