Created attachment 106763 [details] patch to fix Qt includes I wonder how anyone could compile libreoffice with KDE4/Qt4 support on an up-to-date installation. I had several compile errors because some Qt header files (like QString) were not found, which was quite obvious as the proper include paths were not set during compilation and the directory prefixes were not used (example: use <QtCore/QString> instead of <QString>). Additionally there was an include with wrong quotation (#include "QString" instead of #include <QString>) and also some old (deprecated) include file names. Fixing was quite simple, all affected files are under libreoffice-4.3.1.2/shell/source/backends/kde4be/*, please find the patch in the attachment... my system: - Gentoo Linux - dev-qt/qtcore-4.8.5-r2 - kde-base/kdelibs-4.14.1 - sys-devel/gcc-4.8.3
Lubos: I thought you might be interested in this one. Moreover, reporter suggests a patch.
LO gets Qt include paths from Qt's pkgconfig files. If your build does not use e.g .the QtCore include path, your Qt is presumably not installed properly.
Created attachment 114571 [details] fix bad Qt include directives
Sorry, but the problem described is still present in recent versions (at least in 4.4.1.2) and makes compilation fail. Maybe I did not explain clear enough... The problem is not in the include paths that are passed as compiler options, these are fine. The point is that the code uses deprecated and sometimes also wrong include directives - the errors are in the source code, not in the build environment! Somtimes the quotation is wrong, sometimes the component is missing (which is recommended by Qt since some years), somethimes the old fashioned (and also deprecated) file names are used. Here some examples: old/wrong: => should be: #include "QString" => #include <QtCore/QString> #include <qclipboard.h> => #include <QtGui/QClipboard> #include <QHash> => #include <QtCore/QHash> I attached a patch to fix these things...
Jan-Marek: can you review the patch?
1. Using "" instead of <> for includes Well - actually this is implementation specific. For gcc see: https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html If you send a patch to Gerrit, I'll merge it. The C standard, section 6.10.2, paragraphs 2 to 4 states, that "" falls back to <>, if the search doesn't succeed. 2. Include Qt components in #include <> I was told there is no deprecation in #qt on IRC. Both include styles are correct. You have an independent build problem.
to 1. yes, the theory says that it does not matter, but my experience says that this is also depends on the environment/distribution you are using, and their compiler flags. It is common habit to use "" for project local includes and <> for system includes. This can be enforced by CFLAGS. At least one handle do that in one unique way within one project, and not a wild mixture. That's just a matter of programming style. to 2. maybe you asked the wrong person...? I am sure that the Qt-4 include paths heavily depend on the distribution and in my case (Gentoo Linux) there is only one common qt4 include directive, so that the directory prefix is needed. This was different in Qt-3 and it will change (again) in Qt-5, but for Qt-4 you only are on the safe side if you use it the way I patched it. AFAIR this was the *recommended* way when porting from Qt-3 to Qt-4. Using the old way "might" work, but it definitely did not here. So as a conclusion: what kind of risk do you see in fixing this? BTW: my pkgconf and Qt setup *is* correct, definitely! I have hundreds of Qt/KDE applications that compile fine.
just for your reference, please read here: https://techbase.kde.org/Policies/Library_Code_Policy#Getting_.23includes_right
Thomas: have you confirmed with Qt devs that your build setup is *not* broken? The risk with merging your patch is that builds will break for others.
As I said above: I am working with a Gentoo Linux machine here, so I have to compile all kinds of Qt and KDE applications on my own and that works fine (except for unpatched LibreOffice). I am also maintaining a KDE application for more than 15 years, so I know about this topic from porting from KDE-3 to KDE-4. If you want some "external reference", please read here for the topic "library prefix": http://wiki.qt.io/Coding_Conventions ---------- citation begin ---------- Including headers In public header files, always use this form to include Qt headers: #include <QtCore/qwhatever.h>. The library prefix is neccessary for Mac OS X frameworks and is very convenient for non-qmake projects. ---------- citation end ---------- The lower case file names like "qwhatever.h" seem to be ok, but most people use the class name like "QWhatEver". You can go to gitweb.kde.org, choose a KDE4 based application and look how they handle this, so just as a reference, you may want to take a look here for an example: https://quickgit.kde.org/?p=kdelibs.git&a=blob&f=kdecore%2Fkernel%2Fkglobal.cpp
Jan-Marek Glogowski committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=d9bceb34310f05acce886fe28e9e42433fd488c2 tdf#84263 KDE4: unify includes It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Dear Bug Submitter, This bug has been in NEEDINFO status with no change for at least 6 months. Please provide the requested information as soon as possible and mark the bug as UNCONFIRMED. Due to regular bug tracker maintenance, if the bug is still in NEEDINFO status with no change in 30 days the QA team will close the bug as INVALID due to lack of needed information. For more information about our NEEDINFO policy please read the wiki located here: https://wiki.documentfoundation.org/QA/Bugzilla/Fields/Status/NEEDINFO If you have already provided the requested information, please mark the bug as UNCONFIRMED so that the QA team knows that the bug is ready to be confirmed. Thank you for helping us make LibreOffice even better for everyone! Warm Regards, QA Team Sun, 11 Sep 2016 21:43:24 +0200
As stated in comment #11 a patch for this has already been submitted, and it looks good now. IMHO you can mark this bug as RESOLVED.
Let's put this one to FIXED then.