D9446: WIP: Allow to autogenerate and install categories file

[David Faure]

dfaure added a comment.


  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.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Ralf Habacker]

habacker added a comment.


  In D9446#284822 , @dfaure wrote:
  
  > 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 _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
  
  or by an additional function
  
ecm_qt_logging_category_config(
 COLLECT_HEADER # 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 # store category entries for 
kdebugsettings in this filename
)
  
  BTW: The parameter names are just proposals - there may be better names

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[David Faure]

dfaure added a comment.


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

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Ralf Habacker]

habacker added a comment.


  In D9446#284185 , @ltoscano wrote:
  
  > 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

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Ralf Habacker]

habacker added a comment.


  - doc in file header is missing
  - test case is missing

INLINE COMMENTS

> ECMQtDeclareLoggingCategory.cmake:68
>  set(options)
> -set(oneValueArgs HEADER IDENTIFIER CATEGORY_NAME DEFAULT_SEVERITY)
> +set(oneValueArgs HEADER IDENTIFIER CATEGORY_NAME DEFAULT_SEVERITY 
> CATEGORY_INSTALL_FILENAME CATEGORY_DESCRIPTION)
>  set(multiValueArgs)

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

> ECMQtDeclareLoggingCategory.cmake:138
> +if (ARG_CATEGORY_INSTALL_FILENAME)
> +set(CAT_DESCRIPTION)
> +if (ARG_CATEGORY_DESCRIPTION) 

Using ${PROJECT_NAME} if empty ?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Ralf Habacker]

habacker added a comment.


  In D9446#284272 , @habacker wrote:
  
  > In D9446#181785 , @mlaurent wrote:
  >
  > > 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()

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Luigi Toscano]

ltoscano added a comment.


  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...

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Christophe Giboudeaux]

cgiboudeaux added a comment.


  In D9446#284272 , @habacker wrote:
  
  > In D9446#181785 , @mlaurent wrote:
  >
  > > 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

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Ralf Habacker]

habacker added a comment.


  In D9446#181785 , @mlaurent wrote:
  
  > 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?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Luigi Toscano]

ltoscano added a comment.


  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.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: ltoscano, kde-frameworks-devel, kde-buildsystem, habacker, michaelh, 
ngraham, bruns

D9446: WIP: Allow to autogenerate and install categories file

[Ralf Habacker]

habacker added a comment.
Restricted Application edited subscribers, added: kde-buildsystem, 
kde-frameworks-devel; removed: Frameworks, Build System.


  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.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: kde-frameworks-devel, kde-buildsystem, habacker, michaelh, ngraham, bruns, 
#frameworks, #build_system

D9446: WIP: Allow to autogenerate and install categories file

[Laurent Montel]

mlaurent abandoned this revision.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: #frameworks, #build_system, michaelh, ngraham

D9446: WIP: Allow to autogenerate and install categories file

[Fabian Vogt]

fvogt added a comment.


  `${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.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: #frameworks, #build_system

D9446: WIP: Allow to autogenerate and install categories file

[Luca Beltrame]

lbeltrame added a comment.


  For those who wonder why this would be necessary, see 
https://mail.kde.org/pipermail/kde-frameworks-devel/2017-December/054139.html.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: #frameworks, #build_system

D9446: WIP: Allow to autogenerate and install categories file

[Luca Beltrame]

lbeltrame added a reviewer: fvogt.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure, fvogt
Cc: #frameworks, #build_system

D9446: WIP: Allow to autogenerate and install categories file

[Laurent Montel]

mlaurent added reviewers: kfunk, lbeltrame, cgiboudeaux, dfaure.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent, kfunk, lbeltrame, cgiboudeaux, dfaure
Cc: #frameworks, #build_system

D9446: WIP: Allow to autogenerate and install categories file

[Laurent Montel]

mlaurent added a comment.


  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.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D9446

To: mlaurent
Cc: #frameworks, #build_system

D9446: WIP: Allow to autogenerate and install categories file

[Laurent Montel]

mlaurent created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: Build System, Frameworks.

TEST PLAN
  Generate kmail categories file

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  autogenerate_categories_file

REVISION DETAIL
  https://phabricator.kde.org/D9446

AFFECTED FILES
  modules/ECMQtDeclareLoggingCategory.cmake

To: mlaurent
Cc: #frameworks, #build_system