Include fixx11.h to fix the build with Qt 5.14
Needs ReviewPublic

Authored by heikobecker on Thu, Oct 31, 4:40 PM.

Details

Reviewers
pino
whiting
Summary

Trying to build kmousetool against Qt 5.14 otherwise results in an
error:

"... kmousetool/main.cpp
In file included from /usr/x86_64-pc-linux-gnu/include/X11/Xlib.h:44,

from /usr/x86_64-pc-linux-gnu/include/X11/Intrinsic.h:53,
from /var/tmp/paludis/build/kde-kmousetool-19.08.2/work/kmousetool-19.08.2/kmousetool/kmousetool.cpp:29:

/usr/x86_64-pc-linux-gnu/include/qt5/QtWidgets/qactiongroup.h:64:9: error: expected identifier before numeric constant

64 |         None,
   |         ^~~~

..."

The "QActionGroup::None" enum, which is new in Qt 5.14 [1], collides
with some X header.
Also move an Phonon include, which apparently was involved in a
similar problem, back to where it was before.

[1] https://doc-snapshots.qt.io/qt5-5.14/qactiongroup.html#ExclusionPolicy-enum

Test Plan

Builds and runs fine

Diff Detail

Repository
R429 KMouseTool
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18399
Build 18417: arc lint + arc unit
heikobecker requested review of this revision.Thu, Oct 31, 4:40 PM
heikobecker created this revision.
pino added a comment.Thu, Oct 31, 4:55 PM

IMHO:

  • moving includes like this is more an hack than a real solution, as future changes in other Qt includes may cause this issue again -- see also the comment right before the phonon include...
  • the real solution is to #include <fixx11h.h> right after the X11 includes -- see the KWindowSystem framework

Oh, I was looking for something else but I didn't found anything. Thanks!
Included fixx11.h

heikobecker retitled this revision from Include QMenu before X includes to fix build with Qt 5.14 to Include fixx11.h to fix the build with Qt 5.14.Thu, Oct 31, 5:18 PM
heikobecker edited the summary of this revision. (Show Details)

Not sure if that makes it worth to depend on KWindowSystem, or if it's maybe better to copy the header over.

pino added a comment.Thu, Oct 31, 6:03 PM

Not sure if that makes it worth to depend on KWindowSystem, or if it's maybe better to copy the header over.

Strictly speaking, we just need the includes from KWindowSystem, as the library is not actually used (and when linking with as-needed KWindowSystem will not be required).

In D25095#557350, @pino wrote:

Strictly speaking, we just need the includes from KWindowSystem, as the library is not actually used (and when linking with as-needed KWindowSystem will not be required).

Yeah, I know. Still, it's an additional build time dependency.