Create specific directory for Qt logging categories file
ClosedPublic

Authored by mlaurent on May 21 2019, 5:48 AM.

Details

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mlaurent created this revision.May 21 2019, 5:48 AM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptMay 21 2019, 5:48 AM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
mlaurent requested review of this revision.May 21 2019, 5:48 AM
lbeltrame added a subscriber: cgiboudeaux.
lbeltrame removed a subscriber: cgiboudeaux.
mlaurent updated this revision to Diff 58390.May 21 2019, 6:14 AM

Forgot to commit before creating diff

cgiboudeaux added inline comments.May 21 2019, 6:51 AM
kde-modules/KDEInstallDirs.cmake
132
#     kdebugsettings categories files directory (``DATAROOTDIR/debug-categories``) Since 5.59.0.
586–588

copy-paste issue?

mlaurent updated this revision to Diff 58391.May 21 2019, 7:08 AM

Fix comment reported by Christophe Giboudeaux

cgiboudeaux added inline comments.May 21 2019, 7:16 AM
kde-modules/KDEInstallDirs.cmake
583–584

KXMLGUI_INSTALL_DIR) came from this line :)

Looks mostly fine safe for the directory name that needs the major version to avoid conflicts when libfoo.categories built using KF5 will have to coexist with KF6.
It needs a '5' in its name.

I'm also unsure about 'KDEBUGSETTINGSDIR'. kdebugsettings is the 'tool to change the debug messages visibility' and it makes it sound like we're still using kdebug :)
What about 'DEBUGCATEGORIESDIR' instead?

DEBUGCATEGORIESDIR is fine for me too

debug-categories5 ?

mlaurent updated this revision to Diff 58454.May 22 2019, 4:55 AM

Use DEBUGCATEGORIESDIR and debug-categories5

Ping ?

You didn't add "KXMLGUI_INSTALL_DIR)" back to where it belongs afaics.

If noone has a better suggestion, we'll go for debug-categories5

mlaurent updated this revision to Diff 58604.May 24 2019, 2:35 PM

Oops indeed I forgot to readd KXMLGUI_INSTALL_DIR

Thanks for pointing me it :)

kossebau added inline comments.
kde-modules/KDEInstallDirs.cmake
131

Might LOGGINGCATEGORIESDIR or perhaps short LOGCATDIR not be a better name, for one more following the naming pattern of Qt, but also more clearly pointing out what this is about? Debug categories could mean a lot, while this here is about logging only.

132

given this is adding a dir in the shared naming area, could this perhaps be more specific named, unless the plan is to make this a general-purpose dir shared among all kind of applications & toolkits?
For now this data is only for one program, kdebugsettings, so IMHO that should be reflected here.

LOGGINGCATEGORIESDIR indeed it's more specific +1 for me
debug-categories5 => indeed it's just using by kdebugsettings at the moment. => kdebugsettings-categories5 ?
no idea if it's better.

@dfaure do you have an idea ?:)

My suggestion is to call this qlogging-categories5.

The 'q' prefix is missing to make this qt-specific rather than cross-toolkit.
The "logging" instead of "debug" is because this affects all types of messages, and this is all about what's called "logging categories" in Qt.
The "5" will allow co-installation of the qt5 and qt6 versions of the same library (but it will require two versions of kdebugsettings, or that kdebugsettings looks in both places -- but that can be done later).

I wouldn't name it after the application using this, because well, it could change name, and there could be more apps using this (e.g. akonadiconsole, kdevelop...).

Thanks david.
Indeed qlogging-categories5 seems a good name :)

mlaurent updated this revision to Diff 58746.May 28 2019, 6:55 AM

Change as qlogging-categories5

cgiboudeaux accepted this revision.May 31 2019, 8:43 AM
cgiboudeaux added inline comments.
kde-modules/KDEInstallDirs.cmake
132

nitpick: s/qt/Qt/

This revision is now accepted and ready to land.May 31 2019, 8:43 AM

Maybe also update the commit message to 'Create specific directory for Qt logging categories file'

mlaurent retitled this revision from Create specific directory for kdebugsettings categories file to Create specific directory for Qt logging categories file.May 31 2019, 2:55 PM
This revision was automatically updated to reflect the committed changes.

This change appears to be responsible for all Android builds being broken.
See https://build.kde.org/view/Failing/

Could someone take a look please?

kossebau added subscribers: vkrause, apol.EditedJun 16 2019, 12:37 AM

This change appears to be responsible for all Android builds being broken.
See https://build.kde.org/view/Failing/

Could someone take a look please?

Seems the cause of the build failures is that with the Android builds not the ECM from the product builds or dep builds is picked up, but only the ECM as deployed as part of the SDK.
At least this is what looking at the age of the different builds on CI of the ECM product, the dep set and the SDK showed (where SDK was older than this change, everything else newer).
And triggering a fresh build of the SDK made the builds of the Framework products no longer fail.
So no issue with this patch, but the Android build setup it seems.

@apol @vkrause Perhaps something to check wit the CI or in general with the ECM Android toolchain to make sure the ECM of the host and the target are not accidentally mixed or both visible at the same time.

@apol @vkrause Perhaps something to check wit the CI or in general with the ECM Android toolchain to make sure the ECM of the host and the target are not accidentally mixed or both visible at the same time.

Right, I don't understand why it does that though. On binary factory we didn't have that problem, and AFAIK they use the same setup?

The Binary Factory uses the tooling shipped as part of the KDE SDK (which always builds everything from scratch, and I don't know if part of that includes ECM, hence why the issue doesn't show up there).

The CI system on the other hand uses it's own tooling exclusively on all platforms, however because the Android image ships with a limited number of libraries it is reliant on the KDE SDK to provide a number of non-KDE libraries, and thus has to include the SDK paths when performing builds. Unfortunately this means that it is possible on Android for CI builds to be contaminated by the SDK - the result of which we're seeing here.

Ideally the SDK would separate the base system (non-KDE) libraries from the KDE ones to avoid this issue (and the CI system could therefore rely on it's own build results exclusively)