WIP: Allow to autogenerate and install categories file
AbandonedPublic

Authored by mlaurent on Dec 21 2017, 7:56 AM.

Details

Test Plan

Generate kmail categories file

Diff Detail

Repository
R240 Extra CMake Modules
Branch
autogenerate_categories_file
Lint
No Linters Available
Unit
No Unit Test Coverage
mlaurent created this revision.Dec 21 2017, 7:56 AM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptDec 21 2017, 7:56 AM
Restricted Application added subscribers: Build System, Frameworks. · View Herald Transcript
mlaurent requested review of this revision.Dec 21 2017, 7:56 AM

IT's a WIP as I use file(APPEND...) because I want to generate several categories in one file.
But it doesn't work as I don't have idea how to reset file when cmake is started...

Do you have an idea ?

I will implement autotest and co if we have a solution for it.

fvogt added a comment.Dec 21 2017, 9:23 AM

${KDE_INSTALL_CONFDIR} should be changed to somewhere else (but maybe in a different patch)

IMO file(APPEND ...) is the wrong approach, it should collect all categories in a variable and only write it out once at the end (with configure_file). I'm not fluent in CMake, I'm not sure whether that's actually possible that way.

mlaurent abandoned this revision.Feb 9 2018, 5:53 AM

Having support for defining multiple debug categories has an additional issue as shown with the following example. As far as I can see there should be only one header file for all categories in a repo eg. kcoreaddons_debug.h. To define all available debug categories for kcoreaddons the following cmake code is required.

ecm_qt_declare_logging_category(libkcoreaddons_SRCS
                                HEADER kcoreaddons_debug.h
                                IDENTIFIER KCOREADDONS_DEBUG
                                CATEGORY_NAME org.kde.kcoreaddons
                                CATEGORY_INSTALL_FILENAME kcoreaddons
                                CATEGORY_DESCRIPTION kcoreaddons)

ecm_qt_declare_logging_category(libkcoreaddons_SRCS
                                HEADER kcoreaddons_debug.h
                                IDENTIFIER MIGRATOR
                                CATEGORY_NAME kf5.kcoreaddons.kdelibs4configmigrator
                                DEFAULT_SEVERITY Warning
                                CATEGORY_INSTALL_FILENAME kcoreaddons
                                CATEGORY_DESCRIPTION Kdelibs4ConfigMigrator)

ecm_qt_declare_logging_category(libkcoreaddons_SRCS
                                HEADER kcoreaddons_debug.h
                                IDENTIFIER KDIRWATCH
                                CATEGORY_NAME kf5.kcoreaddons.kdirwatch
                                DEFAULT_SEVERITY Warning
                                CATEGORY_INSTALL_FILENAME kcoreaddons
                                CATEGORY_DESCRIPTION KDirWatch)

if(BUILDING_DESKTOPTOJSON_TOOL)
    set(Level Debug)
else()
    set(Level Warning)
endif()

ecm_qt_declare_logging_category(libkcoreaddons_SRCS
                                HEADER kcoreaddons_debug.h
                                IDENTIFIER DESKTOPPARSER
                                CATEGORY_NAME kf5.kcoreaddons.desktopparser
                                DEFAULT_SEVERITY ${LEVEL}
                                CATEGORY_INSTALL_FILENAME kcoreaddons
                                CATEGORY_DESCRIPTION KDesktopParser)

ecm_qt_declare_logging_category(libkcoreaddons_SRCS
                                HEADER kcoreaddons_debug.h
                                IDENTIFIER KABOUTDATA
                                CATEGORY_NAME kf5.kcoreaddons.kaboutdata
                                DEFAULT_SEVERITY Warning
                                CATEGORY_INSTALL_FILENAME kcoreaddons
                                CATEGORY_DESCRIPTION KAboutData)

Unfortunally this creates kcoreaddons_debug.h only containing the definition for KABOUTDATA. The alternative to specify different files forces developers to know exactly which file to include for specifing a dedicated debug category.

Restricted Application edited subscribers, added: kde-buildsystem, kde-frameworks-devel; removed: Frameworks, Build System. · View Herald TranscriptJun 27 2018, 10:12 PM

There can be more headers file for the same repository, and it has not been a problem in practice because the categories usually maps to different groups of files in different directories.
See for example kopete, wacomtablet o kgraphviewer.

IT's a WIP as I use file(APPEND...) because I want to generate several categories in one file.
But it doesn't work as I don't have idea how to reset file when cmake is started...

Do you have an idea ?

No cmake experts on the KF5 team like it was at KDE4?

krop added a comment.Jun 28 2018, 8:16 AM

IT's a WIP as I use file(APPEND...) because I want to generate several categories in one file.
But it doesn't work as I don't have idea how to reset file when cmake is started...

Do you have an idea ?

No cmake experts on the KF5 team like it was at KDE4?

I didn't forget this task. Just didn't have time.

There are several issues that need fixes:

  • Installation dir for categories, /etc/xdg on linux has always been wrong. categories are not config files
  • Fix the issue Laurent raised
In D9446#284288, @cgiboudeaux wrote:

There are several issues that need fixes:

  • Installation dir for categories, /etc/xdg on linux has always been wrong. categories are not config files

That's something that should be addressed separately and it requires support in kdebugsettings for few cycles before switching, otherwise people will not be able to use the new categories file generated. At this point it could be probably a TODO KF6...

IT's a WIP as I use file(APPEND...) because I want to generate several categories in one file.
But it doesn't work as I don't have idea how to reset file when cmake is started...

Do you have an idea ?

No cmake experts on the KF5 team like it was at KDE4?

I tried this, which seems to work

#.rst:
# ECMQtDeclareLoggingCategory
...
set_property(GLOBAL PROPERTY _ecm_qtdlc_counter "0")
...
function(ecm_qt_declare_logging_category sources_var)
  ...
  if (ARG_CATEGORY_INSTALL_FILENAME)
      set(cat_file ${CMAKE_BINARY_DIR}/${ARG_CATEGORY_INSTALL_FILENAME}.categories)
      get_property(counter GLOBAL PROPERTY _ecm_qtdlc_counter)
      if(counter STREQUAL "0")
          file(WRITE ${cat_file} "")
      endif()

      set(CAT_DESCRIPTION)
      if (ARG_CATEGORY_DESCRIPTION)
          set(CAT_DESCRIPTION ${ARG_CATEGORY_DESCRIPTION})
      endif()
      file(APPEND ${cat_file} "${ARG_CATEGORY_NAME} ${CAT_DESCRIPTION}\n")
      MESSAGE(STATUS "${cat_file} ${CAT_DESCRIPTION}")
      install( FILES ${cat_file} DESTINATION ${KDE_INSTALL_CONFDIR} )
      math(EXPR counter "${counter} + 1")
      set_property(GLOBAL PROPERTY _ecm_qtdlc_counter "${counter}")
  endif()
  • doc in file header is missing
  • test case is missing
modules/ECMQtDeclareLoggingCategory.cmake
68

may be better using shorter names like INSTALL_FILENAME, DESTINATION or FILENAME
and DESCRIPTION instead of CATEGORY_DESCRIPTION. because we are declaring a logging category

138

Using ${PROJECT_NAME} if empty ?

There can be more headers file for the same repository, and it has not been a problem in practice because the categories usually maps to different groups of files in different directories.
See for example kopete, wacomtablet o kgraphviewer.

I made a query to the kde git repos about the use of ecm_qt_declare_logging_category and got

61 kdevelop
57 kdepim-addons
39 kdepim
29 kdepim-runtime
23 plasma-workspace
20 kwin
13 kstars
11 messagelib
11 ark
11 akonadi-import-wizard
10 akonadi
....

Sure, it works, but is it efficient to have 61 files, for example in kdevelop, just to declare debug categories and always have to remember which ones to include?
In Umbrello, for example, I have only one header file that contains debug support, regardless of part of the application.

To have such support, ecm_qt_declare_logging_category only needs to bre able to append code fragments to the related header or cpp file

On the other hand it means that when you add a new category, you get to recompile *everything* :-)

On the other hand it means that when you add a new category, you get to recompile *everything* :-)

I understand what you mean, what can be solved by an additional include file that includes the header file for the created categories e.g.

#include <projectname>_debug.h

where the file content is something like

#include "log1.h"
#include "subdir2/log2.h"

may be with an additional parameter

COLLECT_HEADER        # uses ${PROJECT_NAME}_debug.h

or

COLLECT_HEADER    <filename>

or by an additional function

ecm_qt_logging_category_config(
     COLLECT_HEADER <filename>    # collect all defined category headers into this file
)

Also an additional function has the advantage to be able to define a common destination file for *.categories files

ecm_qt_logging_category_config(
     CATEGORY_INSTALL               # store category entries for kdebugsettings in ${PROJECT_NAME}.categories
     CATEGORY_INSTALL <filename>    # store category entries for kdebugsettings in this filename
)

BTW: The parameter names are just proposals - there may be better names

dfaure added a comment.Jul 2 2018, 8:01 AM

I don't understand what difference that would make. Any user of project_debug.h would get recompiled every time that file (or one of the files it includes) is modified (i.e. every time a category is added, modified or removed). So if every piece of code should include log1.h or log2.h directly instead, project_debug.h would be completely useless.