D24641: Collect more information from version control systems

[Harald Sitter]

sitter accepted this revision.
sitter added a comment.
This revision is now accepted and ready to land.


  I guess nobody has further input then.
  LGTM. Ship it.

REPOSITORY
  R240 Extra CMake Modules

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

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24641: Collect more information from version control systems

[Harald Sitter]

sitter added a comment.


  The WARNING tags should be removed or changed to AUTHOR_WARNING I think. A 
stray call to one of the functions here isn't really something a user building 
a tarball, for example, can do anything about, so WARNING seems wrong.
  
  Other than that I am happy with this.
  Any input from the @kossebau or anyone else @kde-buildsystem

REPOSITORY
  R240 Extra CMake Modules

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

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24641: Collect more information from version control systems

[Harald Sitter]

sitter added a comment.


  This is starting to look really good. All functions will need documenting in 
the header of that file so they show up on api.kde.org, see other modules for 
examples.

INLINE COMMENTS

> ECMSourceVersionControl.cmake:61
> +
> +set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS FALSE)
> +set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC FALSE)

Do we really need this? It seems to me we could just spam it for every call, in 
the grand scheme of things it makes no difference but is less logic one has to 
worry about when extending this module.

> ECMSourceVersionControl.cmake:75
> +# Git
> +if(NOT _ECM_SOURCE_VERSION_GIT_EXECUTABLE)
> +find_program(_ECM_SOURCE_VERSION_GIT_EXECUTABLE

I'd move the exec detection and missing reporting into its own helper which 
gets called by all "public" functions. Currently the logic is duped in both 
functions.

> ECMSourceVersionControl.cmake:87
> +)
> +set(ECM_SOURCE_VERSION_CONTROL_REVISION 
> "${ECM_SOURCE_VERSION_CONTROL_REVISION}" PARENT_SCOPE)
> +else()

I think it's more idiomatic if you accept the variable name as a function 
argument instead of hardcoding one.

For example `find_program`, but really most helpers work like that off the top 
of my head.

REPOSITORY
  R240 Extra CMake Modules

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

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24641: Collect more information from version control systems

[Harald Sitter]

sitter added a comment.


  Hm, how about separate functions though? With a single stat any given build 
still needs N process forks even when they only want 1 value.
  
  In D24641#548394 , @thomasfischer 
wrote:
  
  > To clarify `ECM_SOURCE_VERSION_CONTROL_COMMIT_COUNT`: it counts the number 
of commits in direct line of succession from the repository's initialization to 
the current commit. It does not include commits in other branches. Basically 
the number of commits listed in a plain `git log`. The commit count gives a 
quick indication of the progress of a repository (or branch) without requiring 
to look up the repo's commit messages or hashes.
  
  
  I understand. This is super heavy though. I have just run it on all clones I 
have on this PC and for some repos that takes upwards of half a second to 
complete... on an SSD, so I also ran a quick check on our CI servers, which use 
HDDs and there it takes at least half a second with some repos (e.g. kdevelop) 
going up to 26 seconds (uncached)! I wouldn't mind terribly if the various 
things got split into their own functions (ecm_source_version_control_revison, 
ecm_source_version_control_branch, ecm_source_version_control_commit_count... 
or some names like that) but even then I have to question the use case behind 
the commit count information. So, what is the use case? What do you do with 
this information? It occurs to me that if you know the hash you could look up 
the commit count of that hash should you need it, but I struggle to imagine a 
scenario where that number is relevant.

INLINE COMMENTS

> ECMSourceVersionControl.cmake:70
> +message(STATUS "Source directory '${CMAKE_SOURCE_DIR}' is under 
> version control by Git.")
> +find_program(GIT_EXECUTABLE
> +NAMES git.bat git # for Windows, 'git.bat' must be found before 
> 'git'

I think you are leaking this variable into the parent scope. I am not super 
sure how to deal with this but I think I've seen `_`prefixes, or you use a 
function and explicitly forward into the PARENT_SCOPE 
(https://cmake.org/cmake/help/v3.0/command/set.html).

Alternatively with a multi-function approach I'd probably just make it 
ECM_SOURCE_VERSION_CONTROL_EXECUTABLE so it only needs finding on the first 
call.

REPOSITORY
  R240 Extra CMake Modules

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

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24568: Provide clang-format target with a KDE Frameworks style file

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> KDEClangFormat.cmake:53
> +# try to find clang-format in path
> +find_program(KDE_CLANG_FORMAT_EXECUTABLE clang-format)
> +

I'm pretty sure you need to check the version the exectuable. When I use 6.0 I 
get ctors smushed into one line.

REPOSITORY
  R240 Extra CMake Modules

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

To: cullmann, #frameworks, dfaure
Cc: sitter, mwolff, ochurlaud, nalvarez, kossebau, aacid, davidedmundson, 
dhaumann, apol, ognarb, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D24641: Collect more information from version control systems

[Harald Sitter]

sitter added a comment.


  Do we have a request for this outside kbibtex? My attitude towards adding 
things to ECM is always "is there more than one user".
  
  ECM_SOURCE_VERSION_CONTROL_COMMIT_COUNT seems very specific and opinionated 
(what's origin? does it count commits in merges? also the question you raise 
vis a vis SVN), not necessarily cheap to do based on VCS type and history size 
and to be honest I am not sure what the use is of knowing how many commits 
there are in total, specifically because for decentralized systems like git 
there may be any number of local commits in that count. So I would not add it.
  
  Branch and rev seems plenty useful so long as there's demand for it outside a 
single project. BZR and HG I would remove on account of being incomplete, 
possibly also SVN for lack of demand (?).
  
  Lastly, I think that all of the variables should perhaps be individual 
functions. This file is included by every single KDE software implicitly, so 
we'd run >= 2 $CMD forks every single time cmake is run but for all we know 99% 
of all projects do not use the variables we set. What do you think?

REPOSITORY
  R240 Extra CMake Modules

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

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24159: new module ECMSourceVersionControl

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:f3f4893b5bd2: new module ECMSourceVersionControl 
(authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24159?vs=4=67114

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

AFFECTED FILES
  docs/module/ECMSourceVersionControl.rst
  kde-modules/KDECompilerSettings.cmake
  modules/ECMSourceVersionControl.cmake

To: sitter, kde-buildsystem, dfaure
Cc: apol, kossebau, kde-frameworks-devel, LeGast00n, GB_2, bencreasy, michaelh, 
ngraham, bruns

D24159: new module ECMSourceVersionControl

[Harald Sitter]

sitter created this revision.
sitter added reviewers: kde-buildsystem, dfaure.
Herald added projects: Frameworks, Build System.
Herald added a subscriber: kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  simply sets a variable when the source is under version control. use it to
  auto-enable Debug builds. there are also plans to switch special assertion
  logic on in KIO when used from git, so there definitely is a more generic
  use case of wanting to control behavior based on whether it the source is
  likely used to make a development or production build.
  
  conceivably the module could be used in the future to get git rev-parse or
  the like, hence the generic name.

TEST PLAN
  with .git the var is true, without it is false

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  vcs

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

AFFECTED FILES
  docs/module/ECMSourceVersionControl.rst
  kde-modules/KDECompilerSettings.cmake
  modules/ECMSourceVersionControl.cmake

To: sitter, kde-buildsystem, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D23552: ECM: remove set_package_properties from FindCanberra

[Harald Sitter]

sitter added a comment.


  No objections from me. This was explicitly suggested during review though, so 
I'd like @cgiboudeaux to approve this.

INLINE COMMENTS

> FindCanberra.cmake:95
>  
>  include(FeatureSummary)
>  

Can be removed as well.

REPOSITORY
  R240 Extra CMake Modules

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

To: dfaure, cgiboudeaux, sitter
Cc: kde-buildsystem, kde-frameworks-devel, apol, aacid, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D23262: disable autouic again - it breaks builds

[Harald Sitter]

sitter added a comment.


  https://gitlab.kitware.com/cmake/cmake/issues/19615

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, apol, nicolasfella, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D23262: disable autouic again - it breaks builds

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:557e5d898bf9: disable autouic again - it breaks builds 
(authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23262?vs=64039=64042

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol, nicolasfella, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, bencreasy, michaelh, 
ngraham, bruns

D22805: set autorcc and autouic by default

[Harald Sitter]

sitter abandoned this revision.
sitter added a comment.


  D23262 

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, apol, nicolasfella
Cc: cullmann, nicolasfella, cgiboudeaux, vkrause, kossebau, apol, 
kde-frameworks-devel, kde-buildsystem, LeGast00n, bencreasy, michaelh, ngraham, 
bruns

D23262: disable autouic again - it breaks builds

[Harald Sitter]

sitter created this revision.
sitter added reviewers: apol, nicolasfella, cullmann.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  autouic (like automoc) assumes that every ui_*.h include statement it finds
  relates to a .ui file that needs generating. this is not always true.
  
  e.g. we have software which generates ui_debug.h which is simply
  a qloggingcategory header for the UI category of that software which would
  then trip up autouic because it would assume there's a .ui file when there
  really isn't one.
  
  unfortunately the ui_ assumption cannot be selectively disabled, so we
  can't have explicit listing of .ui in source lists
  
set(foo_SRCS foo.cpp foo.ui)
  
  without also getting the not particularly compatible include assumptions.
  this should be revisited for kf6 since there isn't a technical need for
  files to be called ui_*, they could just as well be *_ui or anything else
  so as to not clash with autouic assumption.
  
  autorcc does not suffer from this problem so we can leave it enabled, for
  now anyway.

TEST PLAN
  sonnet builds again

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol, nicolasfella, cullmann
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, bencreasy, michaelh, 
ngraham, bruns

D22805: set autorcc and autouic by default

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:60ca1a27e539: set autorcc and autouic by default 
(authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22805?vs=64033=64037

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol
Cc: cgiboudeaux, vkrause, kossebau, apol, kde-frameworks-devel, 
kde-buildsystem, LeGast00n, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc and autouic by default

[Harald Sitter]

sitter updated this revision to Diff 64033.
sitter added a comment.


  typo--

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22805?vs=64032=64033

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter
Cc: cgiboudeaux, vkrause, kossebau, apol, kde-frameworks-devel, 
kde-buildsystem, LeGast00n, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc and autouic by default

[Harald Sitter]

sitter updated this revision to Diff 64032.
sitter added a comment.


  bump version again

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22805?vs=62838=64032

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter
Cc: cgiboudeaux, vkrause, kossebau, apol, kde-frameworks-devel, 
kde-buildsystem, LeGast00n, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc and autouic by default

[Harald Sitter]

sitter retitled this revision from "set autorcc by default" to "set autorcc and 
autouic by default".
sitter edited the summary of this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter
Cc: vkrause, kossebau, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
sbergeron, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc by default

[Harald Sitter]

sitter updated this revision to Diff 62838.
sitter added a comment.


  document behavior and when it was introduced, also enable autouic

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22805?vs=62729=62838

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter
Cc: vkrause, kossebau, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
sbergeron, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc by default

[Harald Sitter]

sitter added a comment.


  Indeed, both good points.

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter
Cc: vkrause, kossebau, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
sbergeron, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc by default

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> kossebau wrote in KDECMakeSettings.cmake:243
> Not sure if anyone is using cmake < 3.0 these days, but if, this will be a 
> surprise box to them, as with some biulds things work (where cmake >= 3.0) 
> and with some builds not.
> 
> This needs explicit mentioning in the docs, so developers know what they have 
> to prepare for, also a mention since which ECM version one can rely on this 
> behaviour.

This is cmakes's own behavior. <3.0 has no autorcc to begin with.

That being said the only reason I put the if there is because ECM itself is 
compatible with 2.8.12, so the if seemed appropriate.

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter
Cc: kossebau, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, 
sbergeron, bencreasy, michaelh, ngraham, bruns

D22805: set autorcc by default

[Harald Sitter]

sitter edited the summary of this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, 
bencreasy, michaelh, ngraham, bruns

D22805: set autorcc by default

[Harald Sitter]

sitter created this revision.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  I couldn't find any pertinent discussion on the topic but some reviews I
  stumbled over did set it on some of our application repos and also wonder
  why we don't disable it by default.
  
  autorcc allows more idiomatic use of qrc as they may be used like any
  "ordinary" source file and cmake will know what to do with them (namely
  compile into relevant cpp for inclusion in target) without the developer
  having to worry about anything.

TEST PLAN
  .qrc files can be added to src list variables and will get automatically 
generated into cpp files in the binary dir and built into the target

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, bencreasy, 
michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:50e8dd7b2d00: new find module for Canberra (authored by 
sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=52042=52043

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter marked an inline comment as done.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter updated this revision to Diff 52042.
sitter added a comment.


  explicitly set FOUND_VAR so it is camelcase too

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=51976=52042

BRANCH
  master

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter updated this revision to Diff 51976.
sitter added a comment.


  fix bad copy paste in compat setup

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=51952=51976

BRANCH
  master

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter marked 5 inline comments as done.

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter updated this revision to Diff 51952.
sitter added a comment.


  - pkgconfig is now quiet
  - variables are now camelcase
- old variables are still set for compat
  - new imported target (also sets pkgconfig's cflags, which I presume is the 
sane thing to do)
  - set package description url & description

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=51576=51952

BRANCH
  master

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: aacid, apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter added a comment.


  Good point, I've also noticed that (all?) our finders now also create an 
IMPORTED target. Should we maybe add that too? If so I guess 
`Canberra::Canberra` would be the preferred target name?

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, cgiboudeaux
Cc: apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter updated this revision to Diff 51576.
sitter added a comment.


  document in rst syntax

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18952?vs=51491=51576

BRANCH
  master

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter, cgiboudeaux
Cc: apol, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D18952: new find module for Canberra

[Harald Sitter]

sitter created this revision.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  used by:
  
  - knotification
  - (possibly also knotifyconfig at some point)
  - plasma-pa
  - kmix

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  docs/find-module/FindCanberra.rst
  find-modules/FindCanberra.cmake

To: sitter
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D16221: style++

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:72aa6e1bf19c: style++ (authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16221?vs=43649=43650

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

AFFECTED FILES
  modules/ECMQmLoader.cpp.in

To: sitter, apol
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D16221: style++

[Harald Sitter]

sitter created this revision.
sitter added a reviewer: apol.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  easier to read with spaces
  
  SCM_SILENT

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMQmLoader.cpp.in

To: sitter, apol
Cc: kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

[Harald Sitter]

sitter accepted this revision.
sitter added a comment.
This revision is now accepted and ready to land.


  I currently can't do a testbuild, but the code looks good now.
  
  If we want to iterate on the concept further I think we should do it 
separately, no sense holding up this diff from landing over discussing 
potential additional features. The current use case seems perfectly reasonable 
in of itself.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  prefix

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

To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system

D9299: Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes

[Harald Sitter]

sitter requested changes to this revision.
sitter added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> KDEInstallDirs.cmake:699
> +if(INSTALL_PREFIX_SCRIPT)
> +file(WRITE ${CMAKE_BINARY_DIR}/prefix.sh "
> +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS

>From a style perspective, I'd suggest having the prefix.sh live somewhere in 
>the installed ECM tree and get copied, rather than maintained as a glorified 
>heredoc in the cmake code. That's just a suggestion though.

> KDEInstallDirs.cmake:700
> +file(WRITE ${CMAKE_BINARY_DIR}/prefix.sh "
> +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS
> +export XDG_CONFIG_DIRS=${KDE_INSTALL_FULL_CONFDIR}:$XDG_CONFIG_DIRS

This is not correct, the XDG_ vars are not necessarily set, so all code that 
sets them ought to ensure their default values are appended if necessary.

> If $XDG_DATA_DIRS is either not set or empty, a value equal to 
> /usr/local/share/:/usr/share/ should be used.

i.e.

  export 
XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:${XDG_DATA_DIRS:-/usr/local/share:/usr/share}

> KDEInstallDirs.cmake:701
> +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS
> +export XDG_CONFIG_DIRS=${KDE_INSTALL_FULL_CONFDIR}:$XDG_CONFIG_DIRS
> +export PATH=${KDE_INSTALL_FULL_BINDIR}:$PATH

Same as for XDG_DATA_DIRS

> If $XDG_CONFIG_DIRS is either not set or empty, a value equal to /etc/xdg 
> should be used."

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, sitter
Cc: sitter, cgiboudeaux, #build_system

Re: Frameworks and pkgconfig files

[Harald Sitter]

On Wed, Nov 29, 2017 at 9:43 AM, Christophe Giboudeaux
 wrote:
> Hello,
>
> in https://bugs.kde.org/386933, the bug reporter mentions the pkgconfig file
> name shall match the library one.

I cannot find any such requirement. It is *suggested* to have one
config per lib with the config name matching the lib name, I don't see
it required though.

> Do we have a policy about the pkgconfig stuff ?

Not that I know of (neither do we have one for cmake config packages mind you).

> Only a couple frameworks install such files with different scheme (Baloo
> (uppercase B), libKF5Attica (library name), phonon4qt5...) and the 'name' var
> doesn't always match the filename (Phonon4Qt5).

Library-based naming IMO is fairly unsuitable to begin with as you
search for a "package" not a library, and one package may have
multiple libraries in its link list. So given it makes no difference
anyway I would argue that ideally the pc files name should be the same
as the cmake config package names rather than the library . To that
end, the mkspecs for qmake maybe should also be consistent.

What I would suggest is `find_package(KF5Attica)` in cmake would be
equal to `PKG_CHECK_MODULES([ATTICA], [KF5Attica])` in autotools would
be equal to `QT += KF5Attica`.

Whether consistent naming is worth the fuss of establishing a policy I
am not entirely convinced though.

All this said, backwards compatibility prevents us from changing this
in KF5 anyway. So at best this is KF6 material.

HS

D7094: Include a module for finding qml imports as runtime dependencies

[Harald Sitter]

sitter accepted this revision.
sitter added a comment.
This revision is now accepted and ready to land.


  Oh it's gorgeous!

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: apol, #build_system, #frameworks, sitter
Cc: dfaure, aacid

D7094: Include a module for finding qml imports as runtime dependencies

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> apol wrote in ECMFindQMLModule.cmake.in:30
> Doing find_program for now. The right fix would be to change Qt to export the 
> `qmlplugindump` target.

You still need to do "something" if qmlplugindump isn't found. Print a 
`message(WARNING` and/or `add_feature_info` or something similar.
Right now the code would mark the plugin not found but not tell the user that 
the reason it was not found is that the helper is missing.

e.g. here kirigami is actually installed but qmlplugindump is not:

   10:45:09  /tmp/t/b 
  $ cmake ..
  -- The C compiler identification is GNU 5.4.0
  -- The CXX compiler identification is GNU 5.4.0
  -- Check for working C compiler: /usr/lib/icecc/bin/gcc
  -- Check for working C compiler: /usr/lib/icecc/bin/gcc -- works
  -- Detecting C compiler ABI info
  -- Detecting C compiler ABI info - done
  -- Detecting C compile features
  -- Detecting C compile features - done
  -- Check for working CXX compiler: /usr/lib/icecc/bin/g++
  -- Check for working CXX compiler: /usr/lib/icecc/bin/g++ -- works
  -- Detecting CXX compiler ABI info
  -- Detecting CXX compiler ABI info - done
  -- Detecting CXX compile features
  -- Detecting CXX compile features - done
  -- qmlplugindump failed for org.kde.kirigami.
  -- Could NOT find org.kde.kirigami-QMLModule (missing:  
org.kde.kirigami-QMLModule_FOUND) 
  -- Configuring done
  -- Generating done
  -- Build files have been written to: /tmp/t/b
  
   10:45:12  /tmp/t/b 
  $ cat ../CMakeLists.txt 
  project(test)
  cmake_minimum_required(VERSION 3.5)
  
  find_package(ECM 1.3.0 REQUIRED NO_MODULE)
  set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${ECM_MODULE_PATH})
  
  include(ECMQMLModules)
  ecm_find_qmlmodule(org.kde.kirigami 2.1)

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #build_system, #frameworks, sitter
Cc: dfaure, aacid

D6604: add a metainfo.yaml to make ECM a proper framework

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:5f6aa58b8067: add a metainfo.yaml to make ECM a proper 
framework (authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6604?vs=16626=16641

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

AFFECTED FILES
  README.md
  metainfo.yaml

To: sitter, ochurlaud, dfaure, skelly
Cc: #frameworks, #build_system

D6604: add a metainfo.yaml to make ECM a proper framework

[Harald Sitter]

sitter updated this revision to Diff 16626.
sitter added a comment.


  add a readme
  
  I've tried for like 2 hours to wire this into the existing docs
  build and get away with one README. Alas, no luck.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6604?vs=16443=16626

BRANCH
  master

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

AFFECTED FILES
  README.md
  metainfo.yaml

To: sitter, ochurlaud, dfaure, skelly
Cc: #frameworks, #build_system

D6604: add a metainfo.yaml to make ECM a proper framework

[Harald Sitter]

sitter added a comment.


  In https://phabricator.kde.org/D6604#124228, @ochurlaud wrote:
  
  > At the time I thought about merging it but we would have to re-document 
everything in doxygen , which would be completely different from what upstream 
cmake does.
  >
  > The ECM page is already linked from the left menu. Does it make sense to 
have it in frameworks? It's more a prerequisite for the framework no?
  >
  > Anyway, a link would work though being suboptimal.
  
  
  The metainfo.yaml is a prerequisite for frameworks I think, yeah. If we could 
add a property to the yaml `generateApiDox: false` to prevent doxification of 
ECM that would also work all be it more work I suppose.
  
  BUT
  In a way, I am thinking that for consistency's sake it is actually nicer if 
ECM is listed alongside the other frameworks on 
https://api.kde.org/frameworks/index.html even if it then only links to the 
cmake documentation. It is, at the very least, more discoverable this way.

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, ochurlaud, dfaure, skelly
Cc: #frameworks, #build_system

D6604: add a metainfo.yaml to make ECM a proper framework

[Harald Sitter]

sitter added a comment.


  Good point on trying it locally.
  
  So, as one could expect this adds ECM as a product
  
  F3808791: Screenshot_20170711_120142.png 

  
  Unfortunately given we have no actual documentation (outside the cmake 
documentation) this leads to a fairly useless page
  
  F3808793: Screenshot_20170711_120148.png 

  
  Am I right in assuming that the way to fix this is to write a mainpage which 
links to https://api.kde.org/ecm/ for the actual cmake documentation?

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, ochurlaud, dfaure, skelly
Cc: #frameworks, #build_system

D6604: add a metainfo.yaml to make ECM a proper framework

[Harald Sitter]

sitter added a comment.


  related to https://phabricator.kde.org/D6603
  
  @ochurlaud I hope this will not break api.kde, given ECM hasn't had a 
metainfo.yaml previously it seems to have some sort of special arrangement?

REPOSITORY
  R240 Extra CMake Modules

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

To: sitter, ochurlaud, dfaure, skelly
Cc: #frameworks, #build_system

D6604: add a metainfo.yaml to make ECM a proper framework

[Harald Sitter]

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

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  metainfo.yaml

To: sitter, ochurlaud, dfaure, skelly
Cc: #frameworks, #build_system

D5741: Fix test when compiling from a tarball

[Harald Sitter]

sitter accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: apol, #frameworks, joselema, sitter
Cc: #build_system

D5525: adapt to fetchpo changes and use vars for target directories to dry code

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:77623d19b627: adapt to fetchpo changes and use vars for 
target directories to dry code (authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5525?vs=13641=13648

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol
Cc: #frameworks, #build_system

D5525: adapt to fetchpo changes and use vars for target directories to dry code

[Harald Sitter]

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

REVISION SUMMARY
  - fetchpo now expects the output dirs as named arguments
  - new argument for poqm directories where releaseme will put _qt.po files
  - variables for both po/ and poqm/ to not repeat the paths all over the place

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol
Cc: #frameworks, #build_system

D5523: use correct variable for fetch-translations injection

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:fe46a935227f: use correct variable for fetch-translations 
injection (authored by sitter).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5523?vs=13634=13635

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol
Cc: #frameworks, #build_system

D5523: use correct variable for fetch-translations injection

[Harald Sitter]

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

REVISION SUMMARY
  KDE_L10N_AUTO_TRANSLATIONS is the name of the relevant option

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: sitter, apol
Cc: #frameworks, #build_system

D5352: Add a test for _repository_name() a function added for fetch-translations

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> CMakeLists.txt:25
> +
> +if(NOT TARGET fetch-translations AND NOT EXISTS 
> "${CMAKE_CURRENT_SOURCE_DIR}/../../po")
> +message(FATAL_ERROR "should have a fetch-translations target")

`AND NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/../../po"` is the wrong path, it 
should be CMAKE_CURRENT_SOURCE_DIR/po.

The effective source dir of the test is /tests/KDEFetchTranslations, so whether 
the ecm tree has a po has no impact on the target definition as it is 
effectively "scoped" to the /tests/KDEFetchTranslations tree.

Negative side effect of the wrongness is that the test would not pass for 
tarballed builds with a /po dir as the asserted conditions suddenly is true ;)

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, sitter, aacid
Cc: #frameworks, #build_system

D5352: Add a test for _repository_name() a function added for fetch-translations

[Harald Sitter]

sitter accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: apol, sitter, aacid
Cc: #frameworks, #build_system

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid, ilic, sitter
Cc: sitter

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter added a comment.


  needs a version bump in the docs now 
  
  beyond that LGTM

INLINE COMMENTS

> KDECMakeSettings.cmake:86
> +#
> +# Since 5.33.0
>  

5.34 now

> KDECMakeSettings.cmake:323
> +COMMAND ruby "${CMAKE_BINARY_DIR}/releaseme/fetchpo.rb" --origin 
> ${KDE_L10N_BRANCH} --project "${reponame}" "${CMAKE_CURRENT_SOURCE_DIR}" 
> "${CMAKE_BINARY_DIR}/po"
> +BYPRODUCTS "${CMAKE_BINARY_DIR}/po"
> +DEPENDS "${CMAKE_BINARY_DIR}/releaseme"

This was only introduced in cmake 3.2. I am not sure we care or if this even 
would cause problems beyond a warning, but technically frameworks are 3.0 
compatible (or so they claim anyway ;)). Seeing as the feature is opt-in it 
probably doesn't matter eitherway.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid, ilic
Cc: sitter

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter added a comment.


  Oh! OTOH, given fetch-translations pulls it into bin_dir while normally we'd 
have it in src_dir this may well be awkward in implementation. e.g. 
https://phabricator.kde.org/D5197 effectively ought to be
  
ki18n_install("${CMAKE_SOURCE_DIR}/po")
ki18n_install("${CMAKE_BINARY_DIR}/po")
  
  this gets funnier with frameworks as they have a conditional wrapping the 
src_dir version
  
if (IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/po")
ki18n_install(po)
kdoctools_install(po)
endif()
ki18n_install(${CMAKE_BINARY_DIR}/po)
  
  While the manual call is more explicit and thus obvious it also makes a 
reader who doesn't know about fetch-translations go "WHAT?!?" so I'd actually 
argue that the bin_dir call needs a comment explaining why double-ki18n_install 
calls are a thing... in every single project that gets the ki18n_install call.
  Maybe we can live it, but personally I find it a bit egh. I'd find it nicer 
if ki18n_install simply finds the right po dir. If src_dir has one, use that, 
if bin_dir has one, use that, if neither has one give up (specifying an 
absolute path would bypass this of course).
  
  Food for thought.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter added a comment.


  In https://phabricator.kde.org/D5143#97985, @apol wrote:
  
  > In https://phabricator.kde.org/D5143#97847, @aacid wrote:
  >
  > > That creates a dependency for ki18n for frameworks that only use .ts 
files, no?
  >
  >
  > One thing we can do is remove the ki18n_install step from here and move it 
to each project like this, much like we're doing in KF now:
  >  https://phabricator.kde.org/D5197
  
  
  That'd be a reasonable way to resolve it indeed.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter added a comment.


  The way I understand it the use case here is during development, so I am not 
sure the ki18n circular dep is particularly concerning. Avoiding it seems 
fairly difficult as find_package needs to happen at configure-time, 
fetch-translations at build-time, and so we only actually know whether or not 
it is ts-only at build-time (when we have downloaded the translations).
  
  That being said appropximations that come to mind:
  
  - grep all Messages.sh to check if there are any pots that do not end in 
`_qt.pot`. as a matter of policy we seem to use that for all qt-only pots
  - grep all Messages.sh to check if there are lines without 
$EXTRACT_TR_STRINGS (the extractor for qt-only)

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter added a comment.


  A couple of things I am not sure about
  
  - defaults to stable translations, I'd say it should default to trunk. e.g. 
playground and kdereview don't even have stable
  - considering one has to enable this feature in the cmakecache I think it 
should be made a dependency of target all. having to manually enable the 
feature and then also manually run fetch-translations seems a bit meh, at least 
on first run, on subsequent runs with  $build/po/ present I'd agree with it not 
fetching everything again.
  
  Lastly, you need to retrigger cmake after the fetchpo.rb script was run. 
ki18n_install uses a GLOB to find all languages, but that GLOB is only 
refreshed on cmake runs, so the first time it GLOBS there is nothing and once 
fetch-translations was run the user needs to manually run cmake again to make 
the GLOB actually find something. So what happens is:
  
git clone kde:ksysguard
mkdir ksysguard/build
cd ksysguard/build
cmake .. -DKDE_L10N...etc
make fetch-translations
cmake .. # excess cmake call necessary to actually be able to build the now 
fetched translations
make
make install
  
  It's also pretty bad because ki18n doesn't actually tell you about the 
problem but simply doesn't build the translations and then you have people like 
me going ❓❓❓❓

INLINE COMMENTS

> apol wrote in KDECMakeSettings.cmake:302
> A first approach used it, I decided to remove it because it didn't add much...

Fair enough.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, aacid, ltoscano
Cc: sitter

D5143: Introduce fetch-translations build command

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> KDECMakeSettings.cmake:74
>  # - ``APPLE_SUPPRESS_X11_WARNING`` option since 5.14.0
>  
>  
> #=

Needs documentation for the l10n awesomeness.

> KDECMakeSettings.cmake:280
> +if(KDE_L10N_DOWNLOAD_TRANSLATIONS)
> +set(_EXTRA_ARGS "ALL")
> +else()

This is not used anywhere, is it?

> KDECMakeSettings.cmake:302
> +
> +add_custom_command(
> +OUTPUT "${CMAKE_BINARY_DIR}/releaseme"

I wonder if we shouldn't use `ExternalProject` here. The advantage being that 
cmake would manage the clone and make sure it is updated as necessary. 
Disadvantageously, it doesn't seem to do `depth=1` presently , so I am not 
sure this would be a net-win.

  include(ExternalProject)
  ExternalProject_Add(releaseme
  PREFIX "${CMAKE_BINARY_DIR}/releaseme"
  GIT_REPOSITORY https://anongit.kde.org/releaseme.git
  CONFIGURE_COMMAND ""
  BUILD_COMMAND ""
  INSTALL_COMMAND ""
  )

(NOTE: dependable target would then be `releaseme`, also paths change with this)

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, aacid, ltoscano
Cc: sitter

Re: Review Request 128533: Create a test that validates projects' appstream information

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128533/#review98097
---


Ship it!




Ship It!

- Harald Sitter


On Aug. 4, 2016, 11:03 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128533/
> ---
> 
> (Updated Aug. 4, 2016, 11:03 a.m.)
> 
> 
> Review request for Build System, Extra Cmake Modules, KDE Frameworks, 
> Matthias Klumpp, Scarlett Clark, and Harald Sitter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> At the moment, we're validating it in build.kde.org, but I feel it will be 
> easier for developers to test if we do so locally.
> This patch does it by seeing which `*.appdata.xml` files are being installed 
> and validating them. This way we can keep it generic for all KDE projects.
> 
> 
> Diffs
> -
> 
>   kde-modules/KDECMakeSettings.cmake dd37e7f 
>   kde-modules/appstreamtest.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128533/diff/
> 
> 
> Testing
> ---
> 
> Tested on some projects, locally.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 128533: Create a test that validates projects' appstream information

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128533/#review98085
---


Fix it, then Ship it!




I always get dizzy when I see code that is meant to test something but is 
itself not tested :P
I am fine with it either way. Some style and functional complaints though.


kde-modules/KDECMakeSettings.cmake (line 133)
<https://git.reviewboard.kde.org/r/128533/#comment66054>

Shouldn't there be visual hint of it being found/notfound? Otherwise how 
would one know that one might want to install it for testing.



kde-modules/KDECMakeSettings.cmake (line 135)
<https://git.reviewboard.kde.org/r/128533/#comment66055>

excess space before opening bracket



kde-modules/KDECMakeSettings.cmake (line 137)
<https://git.reviewboard.kde.org/r/128533/#comment66056>

excess space before opening bracket



kde-modules/appstreamtest.cmake (line 13)
<https://git.reviewboard.kde.org/r/128533/#comment66057>

excess space before opening bracket



kde-modules/appstreamtest.cmake (line 20)
<https://git.reviewboard.kde.org/r/128533/#comment66060>

This will ignore the return value of the validation. Making the ctest 
summary always claim success, which is weird and meh.

* make test won't ever output anything useful
* ctest won't output anyting useful unless run with `-V` or somesuch magic

If you actually pick up the result of the validation and then 
`message(STATUS` on ==0 or `message(FATAL_ERROR`, I think usefulness will go up 
300% by actually claiming a validation fail when there was a fail :)


- Harald Sitter


On Aug. 3, 2016, 11:21 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128533/
> ---
> 
> (Updated Aug. 3, 2016, 11:21 p.m.)
> 
> 
> Review request for Build System, Extra Cmake Modules, KDE Frameworks, 
> Matthias Klumpp, Scarlett Clark, and Harald Sitter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> At the moment, we're validating it in build.kde.org, but I feel it will be 
> easier for developers to test if we do so locally.
> This patch does it by seeing which `*.appdata.xml` files are being installed 
> and validating them. This way we can keep it generic for all KDE projects.
> 
> 
> Diffs
> -
> 
>   kde-modules/KDECMakeSettings.cmake dd37e7f 
>   kde-modules/appstreamtest.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128533/diff/
> 
> 
> Testing
> ---
> 
> Tested on some projects, locally.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 110319: drop bodega cmake finder and use cmake config instead

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/110319/
---

(Updated Sept. 20, 2015, 12:17 p.m.)


Status
--

This change has been discarded.


Review request for Build System, Aleix Pol Gonzalez and Aaron J. Seigo.


Repository: bodega-client


Description
---

replace finder script with cmake config making stuff scale

it would be cool if someone from kde-buildsystem could check that
everything is in order.

installing a finder yourself makes next to no sense considering cmake
configs do the same thing and do it better.
in particular the finder was installed into cmakedatadir/... which is not
searched by default (at least not by kde applications if kdedatadir !=
cmakedatadir, e.g. on debian /usr/share vs. /usr/share/kde4).
I retained the BODEGA naming but I think one should think about changing it
to Bodega as that looks rather a lot less silly.


Diffs
-

  CMakeLists.txt 97d1628c0af0ba3276699da20ce307ca22dde95f 
  lib/CMakeLists.txt 381c03f7e26adf95991bf020f214fbebd2060a6f 
  lib/bodega/CMakeLists.txt 1ecc498b494bdff8a75a3a4c49c61f4cdb66ccc4 
  lib/cmake/BODEGAConfig.cmake.in PRE-CREATION 
  lib/cmake/CMakeLists.txt PRE-CREATION 
  lib/cmake/modules/CMakeLists.txt 0207620331bbf949018076ee526010443e6a3173 
  lib/cmake/modules/FindBODEGA.cmake e4c69eec793ef4a7a9728241345c2d1cf9053fac 

Diff: https://git.reviewboard.kde.org/r/110319/diff/


Testing
---

found by muon and muon's backend builds


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 125139: Do not find XINPUT in FindXCB.cmake

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125139/#review85123
---


yes please!

- Harald Sitter


On Sept. 10, 2015, 2:52 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125139/
> ---
> 
> (Updated Sept. 10, 2015, 2:52 p.m.)
> 
> 
> Review request for Extra Cmake Modules, Alex Merry and Harald Sitter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> The XCB bindings for XINPUT are not finished and explictly disabled
> in XCB, see for xcb 1.11 [1].
> 
> Because of that most distributions do not include xinput. Trying to find
> xinput only creates problems - the distro of the user might provide it
> and they might not be aware that it's not available normally.
> 
> Thus let's not pretend it exists. Make life of everybody easier.
> 
> [1] http://cgit.freedesktop.org/xcb/libxcb/tree/configure.ac?id=1.11#n234
> 
> 
> Diffs
> -
> 
>   find-modules/FindXCB.cmake b2a800f73058382ee84f7d93132a96f8852b4aa7 
> 
> Diff: https://git.reviewboard.kde.org/r/125139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 125139: Do not find XINPUT in FindXCB.cmake

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125139/#review85124
---


- Harald Sitter


On Sept. 10, 2015, 2:52 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125139/
> ---
> 
> (Updated Sept. 10, 2015, 2:52 p.m.)
> 
> 
> Review request for Extra Cmake Modules, Alex Merry and Harald Sitter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> The XCB bindings for XINPUT are not finished and explictly disabled
> in XCB, see for xcb 1.11 [1].
> 
> Because of that most distributions do not include xinput. Trying to find
> xinput only creates problems - the distro of the user might provide it
> and they might not be aware that it's not available normally.
> 
> Thus let's not pretend it exists. Make life of everybody easier.
> 
> [1] http://cgit.freedesktop.org/xcb/libxcb/tree/configure.ac?id=1.11#n234
> 
> 
> Diffs
> -
> 
>   find-modules/FindXCB.cmake b2a800f73058382ee84f7d93132a96f8852b4aa7 
> 
> Diff: https://git.reviewboard.kde.org/r/125139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124892: bug 342962: kdeclarative plugins should be built as a bundle plugin and not a shared library

[Harald Sitter]

 On Aug. 25, 2015, 2:22 p.m., René J.V. Bertin wrote:
   When built as SHARED as in the current code, libdraganddropplugin.dylib 
   gets installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given 
   an OS X install_name of $PREFIX/lib/libdraganddropplugin.dylib. This 
   mismatch can cause problems.
   It is also given a compatibility_version of 0.0.0.
  
  Both properties are used only when the `dylib` is used as a shared library, 
  because AFAIK they're only used by the dynamic loader when the reference 
  information is available (i.e. stored in the binary by the link editor). 
  Using `dlopen` on a random file, dylib or other, does not provide such 
  information and thus it shouldn't matter what the loaded file claims for 
  its install_name and/or compatibility_version.
  
  By contrast, there *is* a difference between shared libraries and loadable 
  modules (plugins or `bundles` in OS X speak) on OS X. The latter cannot 
  be used as shared libraries by the link editor (something that certain KDE4 
  projects sin against) while the former can be used as plugins without any 
  issue. However, only plugins can have automatic access to the loader's 
  symbols but that requires the loader to be specified at link time so not of 
  much interest for generic plugins, kparts etc.
  NB: the `bundle` in this context is unfortunate because they are not 
  generally bundles in the app bundle way.
  NB2: those KDE projects that use modules as shared libraries (kparts, in 
  unit tests IIRC) led me to propose a patch to MacPorts's port:cmake which 
  made cmake create modules with the same linker options as shared libraries 
  (i.e. `-dynamiclib` instead of `-bundle`). I've since tried to get that 
  patch removed, but it is still out there.
 
 Marco Martin wrote:
 if this breaks on linux, it should be reverted for now

Reverted for now. I also reopened the bug report.

Also for the qml side of things: 
https://mail.kde.org/pipermail/kde-frameworks-devel/2015-August/026281.html


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124892/#review84338
---


On Aug. 23, 2015, 9:16 p.m., Hanspeter Niederstrasser wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124892/
 ---
 
 (Updated Aug. 23, 2015, 9:16 p.m.)
 
 
 Review request for Build System, KDE Software on Mac OS X, KDE Frameworks, 
 Plasma, and Harald Sitter.
 
 
 Bugs: 342962
 https://bugs.kde.org/show_bug.cgi?id=342962
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 The kdeclarative plugins (draganddropplugin, kcoreaddonsplugin, kio, 
 kquickcontrolsprivateplugin, and kquickcontrolsaddonsplugin) are being built 
 as shared libraries. They should be built as bundles (MODULE) in the 
 CMakeLists.txt file.
 
 When built as SHARED as in the current code, libdraganddropplugin.dylib gets 
 installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given an OS X 
 install_name of $PREFIX/lib/libdraganddropplugin.dylib. This mismatch can 
 cause problems. It is also given a compatibility_version of 0.0.0.
 
 
 Diffs
 -
 
   src/qmlcontrols/draganddrop/CMakeLists.txt e8127e4 
   src/qmlcontrols/kcoreaddons/CMakeLists.txt 3f77f2d 
   src/qmlcontrols/kioplugin/CMakeLists.txt 7b258e0 
   src/qmlcontrols/kquickcontrols/private/CMakeLists.txt da355c1 
   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 
 
 Diff: https://git.reviewboard.kde.org/r/124892/diff/
 
 
 Testing
 ---
 
 Since the plugin is not supposed to be a linkable library, it should be built 
 as MODULE in CMakeLists.txt. The physical install location remains the same 
 and plugins don't have install_names. This corrects the install_name/install 
 location mismatch. The change should not have any effect on non-OS X systems.
 
 
 Thanks,
 
 Hanspeter Niederstrasser
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124892: bug 342962: kdeclarative plugins should be built as a bundle plugin and not a shared library

[Harald Sitter]

 On Aug. 25, 2015, 1:41 p.m., David Edmundson wrote:
  I've got both Gentoo and Arch saying this causes a major problem [1]:
  
  libdraganddropplugin.so changes to draganddropplugin.so
  in /usr/lib/qt/qml/org/kde/draganddrop
  
  and then they don't get loaded.
  
  any ideas? Otherwise I'll have to revert this before release.
  
  [1] https://aur.archlinux.org/packages/plasma-desktop-git/
 
 Harald Sitter wrote:
 I tried it this morning and it seemed to work. Now I try it again and it 
 fails
 
 I suppose this is the point where we call for a revert hammer? :P
 
 Harald Sitter wrote:
 from qmldir documentation
 
 Declares a plugin to be made available by the module.
 Name is the plugin library name. This is usually not the same as the 
 file name of the plugin binary, which is platform dependent; e.g. the library 
 MyAppTypes would produce libMyAppTypes.so on Linux and MyAppTypes.dll on 
 Windows.

http://www.cmake.org/cmake/help/v3.0/variable/CMAKE_SHARED_MODULE_PREFIX.html


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124892/#review84333
---


On Aug. 23, 2015, 9:16 p.m., Hanspeter Niederstrasser wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124892/
 ---
 
 (Updated Aug. 23, 2015, 9:16 p.m.)
 
 
 Review request for Build System, KDE Software on Mac OS X, KDE Frameworks, 
 Plasma, and Harald Sitter.
 
 
 Bugs: 342962
 https://bugs.kde.org/show_bug.cgi?id=342962
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 The kdeclarative plugins (draganddropplugin, kcoreaddonsplugin, kio, 
 kquickcontrolsprivateplugin, and kquickcontrolsaddonsplugin) are being built 
 as shared libraries. They should be built as bundles (MODULE) in the 
 CMakeLists.txt file.
 
 When built as SHARED as in the current code, libdraganddropplugin.dylib gets 
 installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given an OS X 
 install_name of $PREFIX/lib/libdraganddropplugin.dylib. This mismatch can 
 cause problems. It is also given a compatibility_version of 0.0.0.
 
 
 Diffs
 -
 
   src/qmlcontrols/draganddrop/CMakeLists.txt e8127e4 
   src/qmlcontrols/kcoreaddons/CMakeLists.txt 3f77f2d 
   src/qmlcontrols/kioplugin/CMakeLists.txt 7b258e0 
   src/qmlcontrols/kquickcontrols/private/CMakeLists.txt da355c1 
   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 
 
 Diff: https://git.reviewboard.kde.org/r/124892/diff/
 
 
 Testing
 ---
 
 Since the plugin is not supposed to be a linkable library, it should be built 
 as MODULE in CMakeLists.txt. The physical install location remains the same 
 and plugins don't have install_names. This corrects the install_name/install 
 location mismatch. The change should not have any effect on non-OS X systems.
 
 
 Thanks,
 
 Hanspeter Niederstrasser
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124892: bug 342962: kdeclarative plugins should be built as a bundle plugin and not a shared library

[Harald Sitter]

 On Aug. 25, 2015, 1:41 p.m., David Edmundson wrote:
  I've got both Gentoo and Arch saying this causes a major problem [1]:
  
  libdraganddropplugin.so changes to draganddropplugin.so
  in /usr/lib/qt/qml/org/kde/draganddrop
  
  and then they don't get loaded.
  
  any ideas? Otherwise I'll have to revert this before release.
  
  [1] https://aur.archlinux.org/packages/plasma-desktop-git/
 
 Harald Sitter wrote:
 I tried it this morning and it seemed to work. Now I try it again and it 
 fails
 
 I suppose this is the point where we call for a revert hammer? :P
 
 Harald Sitter wrote:
 from qmldir documentation
 
 Declares a plugin to be made available by the module.
 Name is the plugin library name. This is usually not the same as the 
 file name of the plugin binary, which is platform dependent; e.g. the library 
 MyAppTypes would produce libMyAppTypes.so on Linux and MyAppTypes.dll on 
 Windows.
 
 Harald Sitter wrote:
 
 http://www.cmake.org/cmake/help/v3.0/variable/CMAKE_SHARED_MODULE_PREFIX.html
 
 David Edmundson wrote:
 are we meant to set that to lib in every project? That doesn't sound 
 right.

More like per-target even, since a kded plugin for example wouldn't want the 
prefix I suppose. It might well be that switching the qml plugins to MODULE is 
quite simply not the best solution to the initial problem on OSX, even though 
TBH they are really MODULE and not SHARED anyway so by any measure declaring 
them SHARED was weird all along.
alexmerry might know of a better way but from my quick research it appears that 
either we need to set the prefix variable (supposedly via a macro wrapping 
add_library) or we revert back to SHARED and need to find another way to coerce 
cmake into producing working results on OSX.

In a way I would argue that the problem is more with the qml loader not looking 
for a version without lib prefix if the lib prefix one is not available. So, 
regardless of how we proceed with kdeclarative I think it would be wortwhile to 
possible expand the qml loader to not require the lib prefix on unix systems. 
If not to resolve the issue at hand, at least to have it behave reasonably in 
the distant future.


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124892/#review84333
---


On Aug. 23, 2015, 9:16 p.m., Hanspeter Niederstrasser wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124892/
 ---
 
 (Updated Aug. 23, 2015, 9:16 p.m.)
 
 
 Review request for Build System, KDE Software on Mac OS X, KDE Frameworks, 
 Plasma, and Harald Sitter.
 
 
 Bugs: 342962
 https://bugs.kde.org/show_bug.cgi?id=342962
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 The kdeclarative plugins (draganddropplugin, kcoreaddonsplugin, kio, 
 kquickcontrolsprivateplugin, and kquickcontrolsaddonsplugin) are being built 
 as shared libraries. They should be built as bundles (MODULE) in the 
 CMakeLists.txt file.
 
 When built as SHARED as in the current code, libdraganddropplugin.dylib gets 
 installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given an OS X 
 install_name of $PREFIX/lib/libdraganddropplugin.dylib. This mismatch can 
 cause problems. It is also given a compatibility_version of 0.0.0.
 
 
 Diffs
 -
 
   src/qmlcontrols/draganddrop/CMakeLists.txt e8127e4 
   src/qmlcontrols/kcoreaddons/CMakeLists.txt 3f77f2d 
   src/qmlcontrols/kioplugin/CMakeLists.txt 7b258e0 
   src/qmlcontrols/kquickcontrols/private/CMakeLists.txt da355c1 
   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 
 
 Diff: https://git.reviewboard.kde.org/r/124892/diff/
 
 
 Testing
 ---
 
 Since the plugin is not supposed to be a linkable library, it should be built 
 as MODULE in CMakeLists.txt. The physical install location remains the same 
 and plugins don't have install_names. This corrects the install_name/install 
 location mismatch. The change should not have any effect on non-OS X systems.
 
 
 Thanks,
 
 Hanspeter Niederstrasser
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124892: bug 342962: kdeclarative plugins should be built as a bundle plugin and not a shared library

[Harald Sitter]

 On Aug. 25, 2015, 1:41 p.m., David Edmundson wrote:
  I've got both Gentoo and Arch saying this causes a major problem [1]:
  
  libdraganddropplugin.so changes to draganddropplugin.so
  in /usr/lib/qt/qml/org/kde/draganddrop
  
  and then they don't get loaded.
  
  any ideas? Otherwise I'll have to revert this before release.
  
  [1] https://aur.archlinux.org/packages/plasma-desktop-git/
 
 Harald Sitter wrote:
 I tried it this morning and it seemed to work. Now I try it again and it 
 fails
 
 I suppose this is the point where we call for a revert hammer? :P

from qmldir documentation

Declares a plugin to be made available by the module.
Name is the plugin library name. This is usually not the same as the file 
name of the plugin binary, which is platform dependent; e.g. the library 
MyAppTypes would produce libMyAppTypes.so on Linux and MyAppTypes.dll on 
Windows.


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124892/#review84333
---


On Aug. 23, 2015, 9:16 p.m., Hanspeter Niederstrasser wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124892/
 ---
 
 (Updated Aug. 23, 2015, 9:16 p.m.)
 
 
 Review request for Build System, KDE Software on Mac OS X, KDE Frameworks, 
 Plasma, and Harald Sitter.
 
 
 Bugs: 342962
 https://bugs.kde.org/show_bug.cgi?id=342962
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 The kdeclarative plugins (draganddropplugin, kcoreaddonsplugin, kio, 
 kquickcontrolsprivateplugin, and kquickcontrolsaddonsplugin) are being built 
 as shared libraries. They should be built as bundles (MODULE) in the 
 CMakeLists.txt file.
 
 When built as SHARED as in the current code, libdraganddropplugin.dylib gets 
 installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given an OS X 
 install_name of $PREFIX/lib/libdraganddropplugin.dylib. This mismatch can 
 cause problems. It is also given a compatibility_version of 0.0.0.
 
 
 Diffs
 -
 
   src/qmlcontrols/draganddrop/CMakeLists.txt e8127e4 
   src/qmlcontrols/kcoreaddons/CMakeLists.txt 3f77f2d 
   src/qmlcontrols/kioplugin/CMakeLists.txt 7b258e0 
   src/qmlcontrols/kquickcontrols/private/CMakeLists.txt da355c1 
   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 
 
 Diff: https://git.reviewboard.kde.org/r/124892/diff/
 
 
 Testing
 ---
 
 Since the plugin is not supposed to be a linkable library, it should be built 
 as MODULE in CMakeLists.txt. The physical install location remains the same 
 and plugins don't have install_names. This corrects the install_name/install 
 location mismatch. The change should not have any effect on non-OS X systems.
 
 
 Thanks,
 
 Hanspeter Niederstrasser
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124892: bug 342962: kdeclarative plugins should be built as a bundle plugin and not a shared library

[Harald Sitter]

 On Aug. 25, 2015, 1:41 p.m., David Edmundson wrote:
  I've got both Gentoo and Arch saying this causes a major problem [1]:
  
  libdraganddropplugin.so changes to draganddropplugin.so
  in /usr/lib/qt/qml/org/kde/draganddrop
  
  and then they don't get loaded.
  
  any ideas? Otherwise I'll have to revert this before release.
  
  [1] https://aur.archlinux.org/packages/plasma-desktop-git/

I tried it this morning and it seemed to work. Now I try it again and it 
fails

I suppose this is the point where we call for a revert hammer? :P


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124892/#review84333
---


On Aug. 23, 2015, 9:16 p.m., Hanspeter Niederstrasser wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124892/
 ---
 
 (Updated Aug. 23, 2015, 9:16 p.m.)
 
 
 Review request for Build System, KDE Software on Mac OS X, KDE Frameworks, 
 Plasma, and Harald Sitter.
 
 
 Bugs: 342962
 https://bugs.kde.org/show_bug.cgi?id=342962
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 The kdeclarative plugins (draganddropplugin, kcoreaddonsplugin, kio, 
 kquickcontrolsprivateplugin, and kquickcontrolsaddonsplugin) are being built 
 as shared libraries. They should be built as bundles (MODULE) in the 
 CMakeLists.txt file.
 
 When built as SHARED as in the current code, libdraganddropplugin.dylib gets 
 installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given an OS X 
 install_name of $PREFIX/lib/libdraganddropplugin.dylib. This mismatch can 
 cause problems. It is also given a compatibility_version of 0.0.0.
 
 
 Diffs
 -
 
   src/qmlcontrols/draganddrop/CMakeLists.txt e8127e4 
   src/qmlcontrols/kcoreaddons/CMakeLists.txt 3f77f2d 
   src/qmlcontrols/kioplugin/CMakeLists.txt 7b258e0 
   src/qmlcontrols/kquickcontrols/private/CMakeLists.txt da355c1 
   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 
 
 Diff: https://git.reviewboard.kde.org/r/124892/diff/
 
 
 Testing
 ---
 
 Since the plugin is not supposed to be a linkable library, it should be built 
 as MODULE in CMakeLists.txt. The physical install location remains the same 
 and plugins don't have install_names. This corrects the install_name/install 
 location mismatch. The change should not have any effect on non-OS X systems.
 
 
 Thanks,
 
 Hanspeter Niederstrasser
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124611: do not install namelink for private library

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124611/
---

(Updated Aug. 12, 2015, 6:09 a.m.)


Status
--

This change has been marked as submitted.


Review request for Build System and KDE Frameworks.


Changes
---

Submitted with commit 7dd4fa9f8f19bac44b54418dffefdccae58480f9 by Harald Sitter 
to branch master.


Repository: kglobalaccel


Description
---

please note that the private target is appearing in the resulting cmake
config all the same and we cannot prevent this because of a cmake oddity
Alex Merry was kind enough to track down [1].
testing suggests that this is however not breaking anything (what with the
library being private).

[1] http://public.kitware.com/pipermail/cmake-developers/2015-July/025798.html


Diffs
-

  src/runtime/CMakeLists.txt 1a45ac82bde0dba194e3a2dd4313f7b732cc8129 

Diff: https://git.reviewboard.kde.org/r/124611/diff/


Testing
---

make  make install
  no .so symlink installed


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124610: do not install namelink for private library

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124610/
---

(Updated Aug. 12, 2015, 6:06 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, Build System and KDE Frameworks.


Changes
---

Submitted with commit 9807ff7416f651a0daf289b7bd024a7bb9952f99 by Harald Sitter 
to branch master.


Repository: baloo


Description
---

please note that the private target is appearing in the resulting cmake
config all the same and we cannot prevent this because of a cmake oddity
Alex Merry was kind enough to track down [1].
testing suggests that this is however not breaking anything (what with the
library being private).

[1] http://public.kitware.com/pipermail/cmake-developers/2015-July/025798.html


Diffs
-

  src/engine/CMakeLists.txt 8e2b5b9c0294a08142f4dc486eecab442167b1ec 

Diff: https://git.reviewboard.kde.org/r/124610/diff/


Testing
---

make  make install
  no .so symlink installed

made sure there is no symlink on the system and rebuilt gwenview:
  gwenview builds just fine


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124610: do not install namelink for private library

[Harald Sitter]

 On Aug. 4, 2015, 8:39 a.m., Vishesh Handa wrote:
  I have no idea what this does. Someone else will need to give it a ShipIt. 
  Also, you probably want to get this in today before the tagging.

Actually I'd rather put it in after tagging so we get a month worth of CI on 
top of the change. KGlobalAccel has the same problem and was released with it, 
so I suppose we can live with the .so symlinks for another month to make sure 
nothing breaks.
Since the libs are not considered public and cannot be linked against unless 
one actually copies the headers from the framework source the symlink on its 
own doesn't cause immediate concern anyway.


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124610/#review83412
---


On Aug. 4, 2015, 8:04 a.m., Harald Sitter wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124610/
 ---
 
 (Updated Aug. 4, 2015, 8:04 a.m.)
 
 
 Review request for Baloo, Build System and KDE Frameworks.
 
 
 Repository: baloo
 
 
 Description
 ---
 
 please note that the private target is appearing in the resulting cmake
 config all the same and we cannot prevent this because of a cmake oddity
 Alex Merry was kind enough to track down [1].
 testing suggests that this is however not breaking anything (what with the
 library being private).
 
 [1] http://public.kitware.com/pipermail/cmake-developers/2015-July/025798.html
 
 
 Diffs
 -
 
   src/engine/CMakeLists.txt 8e2b5b9c0294a08142f4dc486eecab442167b1ec 
 
 Diff: https://git.reviewboard.kde.org/r/124610/diff/
 
 
 Testing
 ---
 
 make  make install
   no .so symlink installed
 
 made sure there is no symlink on the system and rebuilt gwenview:
   gwenview builds just fine
 
 
 Thanks,
 
 Harald Sitter
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Review Request 124611: do not install namelink for private library

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124611/
---

Review request for Build System and KDE Frameworks.


Repository: kglobalaccel


Description
---

please note that the private target is appearing in the resulting cmake
config all the same and we cannot prevent this because of a cmake oddity
Alex Merry was kind enough to track down [1].
testing suggests that this is however not breaking anything (what with the
library being private).

[1] http://public.kitware.com/pipermail/cmake-developers/2015-July/025798.html


Diffs
-

  src/runtime/CMakeLists.txt 1a45ac82bde0dba194e3a2dd4313f7b732cc8129 

Diff: https://git.reviewboard.kde.org/r/124611/diff/


Testing
---

make  make install
  no .so symlink installed


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Review Request 124610: do not install namelink for private library

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124610/
---

Review request for Baloo, Build System and KDE Frameworks.


Repository: baloo


Description
---

please note that the private target is appearing in the resulting cmake
config all the same and we cannot prevent this because of a cmake oddity
Alex Merry was kind enough to track down [1].
testing suggests that this is however not breaking anything (what with the
library being private).

[1] http://public.kitware.com/pipermail/cmake-developers/2015-July/025798.html


Diffs
-

  src/engine/CMakeLists.txt 8e2b5b9c0294a08142f4dc486eecab442167b1ec 

Diff: https://git.reviewboard.kde.org/r/124610/diff/


Testing
---

make  make install
  no .so symlink installed

made sure there is no symlink on the system and rebuilt gwenview:
  gwenview builds just fine


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124104: Make it possible to use kconfig_compiler from different sources

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124104/#review81545
---

Ship it!


Ship It!

- Harald Sitter


On June 18, 2015, 1:19 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124104/
 ---
 
 (Updated June 18, 2015, 1:19 a.m.)
 
 
 Review request for Build System, KDE Frameworks, Matthew Dawson, and Harald 
 Sitter.
 
 
 Repository: kconfig
 
 
 Description
 ---
 
 Separates the tooling that is supposed to be used at run-time so that we can 
 specify which to use explicitly.
 
 Ideally this would be done automagically, for now we'll have to specify 
 -DKF5ConfigCompiler_DIR=...
 
 Additionally, we should make kconfig_compiler co-installable by putting it in 
 libexec, but I'd prefer to do that in a future patch. This same process 
 should also happen on other frameworks.
 
 
 Diffs
 -
 
   CMakeLists.txt f510e3e 
   KF5ConfigConfig.cmake.in b4e5f56 
   src/kconfig_compiler/CMakeLists.txt ec4a733 
 
 Diff: https://git.reviewboard.kde.org/r/124104/diff/
 
 
 Testing
 ---
 
 Works as expected on my debian chroot environment.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124009: tear cmake logic for qt4 and qt5 apart

[Harald Sitter]

 On June 4, 2015, 10:28 p.m., Hrvoje Senjan wrote:
  cmake/PhononQt5.cmake, line 39
  https://git.reviewboard.kde.org/r/124009/diff/1/?file=378830#file378830line39
 
  LGTM.
  This function though could now be named differently? ;-)
  
  (or maybe you want to use KDEInstallDirs to all the paths from there?)

long-term (if we end up with phonon4 api long term anyway), it probably should 
move to kdeinstalldirs. currently such a move is only blocked by the fact that 
the names we use are non-kde specific, so changing those would require changing 
all their uses across phonon and all backends which seems yet more invasive.


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124009/#review81217
---


On June 4, 2015, 8:53 p.m., Harald Sitter wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124009/
 ---
 
 (Updated June 4, 2015, 8:53 p.m.)
 
 
 Review request for Build System, Phonon, Martin Gräßlin, and Hrvoje Senjan.
 
 
 Repository: phonon
 
 
 Description
 ---
 
 - qt4 as before
 - qt5 now uses ECM to set compiler flags and so forth
 - paths are still coming out of the joint finder file as the variable names
   in ECM are different and will potentially cause regressions
 
 
 Diffs
 -
 
   cmake/CMakeLists.txt c31d4dd88d49a7aa2ed6df3c5349f34a5cdbf916 
   cmake/FindPhononInternal.cmake 045f777af4d4cb9c9cf707bf72102c92999f490b 
   cmake/PhononQt4.cmake PRE-CREATION 
   cmake/PhononQt5.cmake PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/124009/diff/
 
 
 Testing
 ---
 
 ran cmake with both lineups and it passes, so yay, I guess.
 
 kinda crap to test properly
 
 
 Thanks,
 
 Harald Sitter
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124009: tear cmake logic for qt4 and qt5 apart

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124009/
---

(Updated June 7, 2015, 8:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for Build System, Phonon, Martin Gräßlin, and Hrvoje Senjan.


Changes
---

Submitted with commit 6d7e80cd5b0361272bb801b213e4fabde9a01784 by Harald Sitter 
to branch master.


Repository: phonon


Description
---

- qt4 as before
- qt5 now uses ECM to set compiler flags and so forth
- paths are still coming out of the joint finder file as the variable names
  in ECM are different and will potentially cause regressions


Diffs
-

  cmake/CMakeLists.txt c31d4dd88d49a7aa2ed6df3c5349f34a5cdbf916 
  cmake/FindPhononInternal.cmake 045f777af4d4cb9c9cf707bf72102c92999f490b 
  cmake/PhononQt4.cmake PRE-CREATION 
  cmake/PhononQt5.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/124009/diff/


Testing
---

ran cmake with both lineups and it passes, so yay, I guess.

kinda crap to test properly


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 124009: tear cmake logic for qt4 and qt5 apart

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124009/
---

(Updated June 4, 2015, 8:53 p.m.)


Review request for Build System, Phonon, Martin Gräßlin, and Hrvoje Senjan.


Repository: phonon


Description
---

- qt4 as before
- qt5 now uses ECM to set compiler flags and so forth
- paths are still coming out of the joint finder file as the variable names
  in ECM are different and will potentially cause regressions


Diffs
-

  cmake/CMakeLists.txt c31d4dd88d49a7aa2ed6df3c5349f34a5cdbf916 
  cmake/FindPhononInternal.cmake 045f777af4d4cb9c9cf707bf72102c92999f490b 
  cmake/PhononQt4.cmake PRE-CREATION 
  cmake/PhononQt5.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/124009/diff/


Testing
---

ran cmake with both lineups and it passes, so yay, I guess.

kinda crap to test properly


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 123874: Fix build with Qt = 5.4.2

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123874/#review80860
---

Ship it!


I'll take it.

FTR the best solution would probably mean refactoring qt5_use_modules to 
isolate the mapping logic for use with LINK_LIBRARIES, or making the entire 
compatibility stuff not be so terrible. Hardly worthwhile as Thiago suggests 
moving forward Qt simply cannot be built without visibility anyway.

- Harald Sitter


On May 25, 2015, 4:08 p.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123874/
 ---
 
 (Updated May 25, 2015, 4:08 p.m.)
 
 
 Review request for Build System, Phonon and Harald Sitter.
 
 
 Repository: phonon
 
 
 Description
 ---
 
 Or to be more accurate, with commit 3eca75de67b3fd2c890715b30c7899cebc096fe9
 
 
 Diffs
 -
 
   cmake/FindPhononInternal.cmake 44862b5 
 
 Diff: https://git.reviewboard.kde.org/r/123874/diff/
 
 
 Testing
 ---
 
 Builds with 5.4.2 branch, or mentioned commit backported.
 Otherwise check_qt_visibility fails:
 
 ```
 [  126s] -- Performing Test __KDE_HAVE_GCC_VISIBILITY - Success
 [  126s] Change Dir: 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp
 [  126s] 
 [  126s] Run Build Command:/usr/bin/gmake cmTryCompileExec3719375587/fast
 [  126s] /usr/bin/gmake -f 
 CMakeFiles/cmTryCompileExec3719375587.dir/build.make 
 CMakeFiles/cmTryCompileExec3719375587.dir/build
 [  126s] gmake[1]: Entering directory 
 '/home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp'
 [  126s] /usr/bin/cmake -E cmake_progress_report 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp/CMakeFiles 
 1
 [  126s] Building CXX object 
 CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o
 [  126s] /usr/bin/c++-fmessage-length=0 -grecord-gcc-switches -O2 -Wall 
 -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables 
 -fasynchronous-unwind-tables -g -DNDEBUG -Wnon-virtual-dtor -Wno-long-long 
 -Wundef -Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith 
 -Wformat-security -fno-exceptions -DQT_NO_EXCEPTIONS -fno-check-new 
 -fno-common -Woverloaded-virtual -fvisibility=hidden  -fPIE 
 -I/usr/include/qt5 -I/usr/include/qt5/QtCore 
 -I/usr/lib64/qt5/mkspecs/linux-g++ -I/usr/include/qt5/QtWidgets 
 -I/usr/include/qt5/QtGui-o 
 CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o -c 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeTmp/check_qt_visibility.cpp
 [  126s] In file included from /usr/include/qt5/QtCore/QtGlobal:1:0,
 [  126s]  from 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeTmp/check_qt_visibility.cpp:1:
 [  126s] /usr/include/qt5/QtCore/qglobal.h:1051:4: error: #error You must 
 build your code with position independent code if Qt was built with 
 -reduce-relocations.  Compile your code with -fPIC (-fPIE is not enough).
 [  126s]  #  error You must build your code with position independent code 
 if Qt was built with -reduce-relocations. \
 [  126s] ^
 [  126s] CMakeFiles/cmTryCompileExec3719375587.dir/build.make:57: recipe for 
 target 'CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o' 
 failed
 [  126s] gmake[1]: Leaving directory 
 '/home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp'
 [  126s] gmake[1]: *** 
 [CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o] Error 1
 [  126s] Makefile:117: recipe for target 'cmTryCompileExec3719375587/fast' 
 failed
 [  126s] gmake: *** [cmTryCompileExec3719375587/fast] Error 2
 [  126s] 
 [  126s] CMake Error at cmake/FindPhononInternal.cmake:416 (message):
 [  126s]   Qt compiled without support for -fvisibility=hidden.  This will 
 break
 [  126s]   plugins and linking of some applications.  Please fix your Qt 
 installation
 [  126s]   (try passing --reduce-exports to configure).
 [  126s] Call Stack (most recent call first):
 [  126s]   CMakeLists.txt:47 (include)
 ```
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 123874: Fix build with Qt = 5.4.2

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123874/#review80723
---


Like Thiago already suggested on kde-core-devel, this might need additional 
discussion in a different venue.

As for the diff itself, I am not sure hitting the compile flags with everything 
Qt got is a very good idea. I have no examples of when this would be 
undesirable but considering the position in the past was to opt-in for specific 
flags, as necessary, it seems a bit over the top to now simply reuse all the 
flags.

I did some research though and I think I found why it fails. Apparently cmake 
implements fPIC and fPIE mutually exclusive (at least as far as 
CMAKE_POSITION_INDEPENDENT_CODE goes) depending on the actual target type
https://github.com/Kitware/CMake/blob/e54d2fdf50d7e1170f9e3dcbcb62ebacc7205de6/Source/cmLocalGenerator.cxx#L2412

And that really makes the problem greater in scope because more things use that 
flag to enable PIC 
(http://lxr.kde.org/search?_filestring=_string=CMAKE_POSITION_INDEPENDENT_CODE)
 and it is in fact the recommended way to handle this right now 
(http://doc.qt.io/qt-5/cmake-manual.html). So yeah, I think this should 
probably be figured out with Qt/CMake people how to best enable position 
independency so both PIC and PIE are enabled at the same time.
That being said, the more accruate temporary fix probably would be to manually 
add CMAKE_${lang}_COMPILE_OPTIONS_PIC and CMAKE_${lang}_COMPILE_OPTIONS_PIE to 
the flags manually (i.e. replicate what ::AddPositionIndependentFlags does but 
using both PIC and PIE).

- Harald Sitter


On May 22, 2015, 4:45 a.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123874/
 ---
 
 (Updated May 22, 2015, 4:45 a.m.)
 
 
 Review request for Build System, Phonon and Harald Sitter.
 
 
 Repository: phonon
 
 
 Description
 ---
 
 Or to be more accurate, with commit 3eca75de67b3fd2c890715b30c7899cebc096fe9
 I am not convinced this is best solution, but requires least changes to 
 current cmake code.
 
 
 Diffs
 -
 
   cmake/FindPhononInternal.cmake 44862b5 
 
 Diff: https://git.reviewboard.kde.org/r/123874/diff/
 
 
 Testing
 ---
 
 Builds with 5.4.2 branch, or mentioned commit backported.
 Otherwise check_qt_visibility fails:
 
 ```
 [  126s] -- Performing Test __KDE_HAVE_GCC_VISIBILITY - Success
 [  126s] Change Dir: 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp
 [  126s] 
 [  126s] Run Build Command:/usr/bin/gmake cmTryCompileExec3719375587/fast
 [  126s] /usr/bin/gmake -f 
 CMakeFiles/cmTryCompileExec3719375587.dir/build.make 
 CMakeFiles/cmTryCompileExec3719375587.dir/build
 [  126s] gmake[1]: Entering directory 
 '/home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp'
 [  126s] /usr/bin/cmake -E cmake_progress_report 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp/CMakeFiles 
 1
 [  126s] Building CXX object 
 CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o
 [  126s] /usr/bin/c++-fmessage-length=0 -grecord-gcc-switches -O2 -Wall 
 -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables 
 -fasynchronous-unwind-tables -g -DNDEBUG -Wnon-virtual-dtor -Wno-long-long 
 -Wundef -Wcast-align -Wchar-subscripts -Wall -W -Wpointer-arith 
 -Wformat-security -fno-exceptions -DQT_NO_EXCEPTIONS -fno-check-new 
 -fno-common -Woverloaded-virtual -fvisibility=hidden  -fPIE 
 -I/usr/include/qt5 -I/usr/include/qt5/QtCore 
 -I/usr/lib64/qt5/mkspecs/linux-g++ -I/usr/include/qt5/QtWidgets 
 -I/usr/include/qt5/QtGui-o 
 CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o -c 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeTmp/check_qt_visibility.cpp
 [  126s] In file included from /usr/include/qt5/QtCore/QtGlobal:1:0,
 [  126s]  from 
 /home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeTmp/check_qt_visibility.cpp:1:
 [  126s] /usr/include/qt5/QtCore/qglobal.h:1051:4: error: #error You must 
 build your code with position independent code if Qt was built with 
 -reduce-relocations.  Compile your code with -fPIC (-fPIE is not enough).
 [  126s]  #  error You must build your code with position independent code 
 if Qt was built with -reduce-relocations. \
 [  126s] ^
 [  126s] CMakeFiles/cmTryCompileExec3719375587.dir/build.make:57: recipe for 
 target 'CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o' 
 failed
 [  126s] gmake[1]: Leaving directory 
 '/home/abuild/rpmbuild/BUILD/phonon-4.8.3/build/CMakeFiles/CMakeTmp'
 [  126s] gmake[1]: *** 
 [CMakeFiles/cmTryCompileExec3719375587.dir/check_qt_visibility.cpp.o] Error 1
 [  126s] Makefile:117: recipe for target

Re: Review Request 121696: Add COMPATIBILITY argument to ecm_setup_version().

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121696/#review72593
---

Ship it!


Works like a charm. Thanks for making this happen :)

- Harald Sitter


On Dec. 27, 2014, 3:33 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121696/
 ---
 
 (Updated Dec. 27, 2014, 3:33 p.m.)
 
 
 Review request for Extra Cmake Modules and Harald Sitter.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 Lots of libraries will want to use SameMajorVersion to make sure
 searching for version 1 of a library doesn't give you version 2, for
 example.
 
 We may want to add another, custom compatibility mode for
 KDE Frameworks-style versioning, where version x.90.z to x.99.z are
 alpha/beta releases for version (x+1).
 
 
 Diffs
 -
 
   modules/ECMSetupVersion.cmake 2aff0ae0dee4fbd511b6fed76e9ca778aab9c545 
   tests/ECMSetupVersionTest/CMakeLists.txt 
 2e7decfb897c9447433038cbcb5800a09779eb24 
   tests/ECMSetupVersionTest/old_version_file_anynewer/CMakeLists.txt 
 PRE-CREATION 
   tests/ECMSetupVersionTest/old_version_file_anynewer/main.c PRE-CREATION 
   tests/ECMSetupVersionTest/old_version_file_exact/CMakeLists.txt 
 PRE-CREATION 
   tests/ECMSetupVersionTest/old_version_file_exact/main.c PRE-CREATION 
   tests/ECMSetupVersionTest/old_version_file_samemajor/CMakeLists.txt 
 PRE-CREATION 
   tests/ECMSetupVersionTest/old_version_file_samemajor/main.c PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/121696/diff/
 
 
 Testing
 ---
 
 See unit tests - all pass.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

qca-qt5 package name

[Harald Sitter]

alohas.

recently the QCA maintainer and I got into a discussion [1] whether a
qca-qt5 library should be a different config inside the same cmake
package or an independent one (detailed discussion in the longest
comment thread of the review).

 find_package(Qca NAMES Qca-qt5 Qca-QT5 Qca-5 Qca REQUIRES)

or

 find_package(Qca-qt5)

former is very much in line with the maintainer's expectation of how
qca is supposed to have any odd suffix supplied by the distro [2] that
would eventually disappear, whereas my thinking in latter is that if
distros start shipping a suffixed version it is here to stay and
really should not be considered a configuration within the regular QCA
package.

any thoughts on whether one is more desirable than the other? I am not
quite sure what one would generally consider proper here.

[1] https://git.reviewboard.kde.org/r/121323/
[2] https://git.reviewboard.kde.org/r/121168/

HS
___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120655: If find_dependency() fails, so should the Config file it is in

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120655/#review68742
---

Ship it!


Confirmed working as expected with the patch.

- Harald Sitter


On Oct. 19, 2014, 6:56 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120655/
 ---
 
 (Updated Oct. 19, 2014, 6:56 p.m.)
 
 
 Review request for Extra Cmake Modules and Harald Sitter.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 If find_dependency() fails, so should the Config file it is in
 
 This matches the behaviour of CMake's version of this macro, which we defer 
 to in CMake 3.0 and later.
 
 
 Diffs
 -
 
   modules/ECMPackageConfigHelpers.cmake 
 da65c3eac39edb0b00a2172af7b58f310d967722 
 
 Diff: https://git.reviewboard.kde.org/r/120655/diff/
 
 
 Testing
 ---
 
 Ran `make test`. Built everything KF5-related using CMake 2.8.12 and ECM with 
 this change. Haven't got round to trying to remove a dep and running CMake.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/
---

(Updated Sept. 17, 2014, 10:27 a.m.)


Status
--

This change has been marked as submitted.


Review request for Build System, Extra Cmake Modules, Rohan Garg, and Sune 
Vuorela.


Repository: extra-cmake-modules


Description
---

when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
KDEInstallDirs to follow whatever is defined by qmake

this change makes sure that qml plugins will end up in a default Qt path
when using the super special magic flag.


Diffs
-

  tests/KDEInstallDirsTest/qt_vars_defined/CMakeLists.txt 
40c1cd88b0b8d0280eaf21ca0ef120cd454d1912 
  kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 

Diff: https://git.reviewboard.kde.org/r/120229/diff/


Testing
---

without patch:
solid installs qml plugins to $LIBDIR/qml

with patch:
solid installs qml plugins to path returned by qmake -query


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/
---

Review request for Build System, Extra Cmake Modules, Rohan Garg, and Sune 
Vuorela.


Repository: extra-cmake-modules


Description
---

when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
KDEInstallDirs to follow whatever is defined by qmake

this change makes sure that qml plugins will end up in a default Qt path
when using the super special magic flag.


Diffs
-

  kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 

Diff: https://git.reviewboard.kde.org/r/120229/diff/


Testing
---

without patch:
solid installs qml plugins to $LIBDIR/qml

with patch:
solid installs qml plugins to path returned by qmake -query


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/
---

(Updated Sept. 16, 2014, 7:29 a.m.)


Review request for Build System, Extra Cmake Modules, Kubuntu, Rohan Garg, and 
Sune Vuorela.


Repository: extra-cmake-modules


Description
---

when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
KDEInstallDirs to follow whatever is defined by qmake

this change makes sure that qml plugins will end up in a default Qt path
when using the super special magic flag.


Diffs
-

  kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 

Diff: https://git.reviewboard.kde.org/r/120229/diff/


Testing
---

without patch:
solid installs qml plugins to $LIBDIR/qml

with patch:
solid installs qml plugins to path returned by qmake -query


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/
---

(Updated Sept. 16, 2014, 9:15 a.m.)


Review request for Extra Cmake Modules, Rohan Garg and Sune Vuorela.


Changes
---

use right pathing macro and fix tests


Repository: extra-cmake-modules


Description
---

when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
KDEInstallDirs to follow whatever is defined by qmake

this change makes sure that qml plugins will end up in a default Qt path
when using the super special magic flag.


Diffs (updated)
-

  tests/KDEInstallDirsTest/qt_vars_defined/CMakeLists.txt 
40c1cd88b0b8d0280eaf21ca0ef120cd454d1912 
  kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 

Diff: https://git.reviewboard.kde.org/r/120229/diff/


Testing
---

without patch:
solid installs qml plugins to $LIBDIR/qml

with patch:
solid installs qml plugins to path returned by qmake -query


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/
---

(Updated Sept. 16, 2014, 9:16 a.m.)


Review request for Build System, Extra Cmake Modules, Rohan Garg, and Sune 
Vuorela.


Repository: extra-cmake-modules


Description
---

when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
KDEInstallDirs to follow whatever is defined by qmake

this change makes sure that qml plugins will end up in a default Qt path
when using the super special magic flag.


Diffs
-

  tests/KDEInstallDirsTest/qt_vars_defined/CMakeLists.txt 
40c1cd88b0b8d0280eaf21ca0ef120cd454d1912 
  kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 

Diff: https://git.reviewboard.kde.org/r/120229/diff/


Testing
---

without patch:
solid installs qml plugins to $LIBDIR/qml

with patch:
solid installs qml plugins to path returned by qmake -query


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/#review66658
---


altogether busted still...

- Harald Sitter


On Sept. 16, 2014, 9:16 a.m., Harald Sitter wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120229/
 ---
 
 (Updated Sept. 16, 2014, 9:16 a.m.)
 
 
 Review request for Build System, Extra Cmake Modules, Rohan Garg, and Sune 
 Vuorela.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
 KDEInstallDirs to follow whatever is defined by qmake
 
 this change makes sure that qml plugins will end up in a default Qt path
 when using the super special magic flag.
 
 
 Diffs
 -
 
   tests/KDEInstallDirsTest/qt_vars_defined/CMakeLists.txt 
 40c1cd88b0b8d0280eaf21ca0ef120cd454d1912 
   kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 
 
 Diff: https://git.reviewboard.kde.org/r/120229/diff/
 
 
 Testing
 ---
 
 without patch:
 solid installs qml plugins to $LIBDIR/qml
 
 with patch:
 solid installs qml plugins to path returned by qmake -query
 
 
 Thanks,
 
 Harald Sitter
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 120229: allow qmldir to follow qt_sys_path

[Harald Sitter]

 On Sept. 16, 2014, 11:16 a.m., Harald Sitter wrote:
  altogether busted still...

Nevermind, kubuntu packaging was just being weird.

Patch should work as intended


- Harald


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120229/#review66658
---


On Sept. 16, 2014, 9:16 a.m., Harald Sitter wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120229/
 ---
 
 (Updated Sept. 16, 2014, 9:16 a.m.)
 
 
 Review request for Build System, Extra Cmake Modules, Rohan Garg, and Sune 
 Vuorela.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 when running with the KDE_INSTALL_USE_QT_SYS_PATHS option allow QMLDIR in
 KDEInstallDirs to follow whatever is defined by qmake
 
 this change makes sure that qml plugins will end up in a default Qt path
 when using the super special magic flag.
 
 
 Diffs
 -
 
   tests/KDEInstallDirsTest/qt_vars_defined/CMakeLists.txt 
 40c1cd88b0b8d0280eaf21ca0ef120cd454d1912 
   kde-modules/KDEInstallDirs.cmake 7fbf6728ff1dcaa330f5b76aa199c7865f329f6d 
 
 Diff: https://git.reviewboard.kde.org/r/120229/diff/
 
 
 Testing
 ---
 
 without patch:
 solid installs qml plugins to $LIBDIR/qml
 
 with patch:
 solid installs qml plugins to path returned by qmake -query
 
 
 Thanks,
 
 Harald Sitter
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 119798: Generating PkgConig files from ECM

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119798/#review64581
---


+1

- Harald Sitter


On Aug. 14, 2014, 11:10 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119798/
 ---
 
 (Updated Aug. 14, 2014, 11:10 p.m.)
 
 
 Review request for Build System, KDE Frameworks and Harald Sitter.
 
 
 Repository: extra-cmake-modules
 
 
 Description
 ---
 
 So we decided we wanted those .pc files, so I created a small script that 
 generates one, I haven't used pc in the past, so feedback is welcome.
 
 
 Diffs
 -
 
   modules/ECMGeneratePkgConfigFile.cmake PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/119798/diff/
 
 
 Testing
 ---
 
 I added it in KCoreAddons, this is the patch:
 diff --git src/lib/CMakeLists.txt src/lib/CMakeLists.txt
 index 26eb5a1..3a07d1c 100644
 --- src/lib/CMakeLists.txt
 +++ src/lib/CMakeLists.txt
 @@ -188,4 +188,6 @@ install(FILES
  
  include(ECMGeneratePriFile)
  ecm_generate_pri_file(BASE_NAME KCoreAddons LIB_NAME KF5CoreAddons DEPS 
 core FILENAME_VAR PRI_FILENAME INCLUDE_INSTALL_DIR 
 ${KF5_INCLUDE_INSTALL_DIR}/KCoreAddons)
 +ecm_generate_pkgconfig_file(BASE_NAME KCoreAddons LIB_NAME KF5CoreAddons 
 DEPS Qt5Core FILENAME_VAR PC_FILENAME INCLUDE_INSTALL_DIR 
 ${KF5_INCLUDE_INSTALL_DIR}/KCoreAddons)
 +install(FILES ${PC_FILENAME} DESTINATION ${ECM_PKGCONFIG_INSTALL_DIR})
  install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
 
 This is the result, on my system:
 
 Name: KF5CoreAddons
 Version: 5.1.0
 Libs: -L/home/kde-devel/kde5/lib64 -l/home/kde-devel/kde5/lib64
 Cflags: -I/home/kde-devel/kde5/include/KF5/KCoreAddons 
 Requires: Qt5Core
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

ECM uninstall target

[Harald Sitter]

alohas,

I just ported phonon/five to ECM and noticed that apparently ECM does
not have an include or something to introduce an uninstall target.

Are there plans to introduce such a thing or am I simply blind and not
seeing the already existing include?

HS
___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: frameworks build instructions wrong / won't work with kubuntu 14.04

[Harald Sitter]

On Thu, Dec 19, 2013 at 3:19 PM, Sune Vuorela nos...@vuorela.dk wrote:
 On 2013-12-19, Harald Sitter sit...@kde.org wrote:
 Should be fixed as per:
 http://launchpadlibrarian.net/160197164/cmake_2.8.12.1-1ubuntu2_2.8.12.1-1ubuntu3.diff.gz

 It looks a bit better, but it is still full of open questions like:

Patches are still incomplete as xnox is still prototyping.

I suggested that he should subscribe to kde-buildsystem so that I
don't have to play middleman here :P

HS
___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: frameworks build instructions wrong / won't work with kubuntu 14.04

[Harald Sitter]

On Thu, Dec 19, 2013 at 7:45 PM, Dimitri John Ledkov
dimitri.led...@surgut.co.uk wrote:
 So I was very clueless why
 Harald brought up that failing to build from source in his PPA build
 to a such a wide audience, when it was entirely unwarranted. And also
 in such a hasty manner, minutes after filing the bug in the
 distribution without allowing any time for a fix to be written or
 uploaded.

From the log I added to the original post:

-*- xnox points out, that the correct interface, is to explicitely set
QT5::moc in that case, in your toolchain file used for custom/prefix
installed system libraries, as advised by CMake.
xnox apachelogger: you are pointing to the last resort path. in
cmake there is Qt5::moc already set to a different location.
[...]
xnox apachelogger: cmake_defaults and all the LD_LIBRARY_PATH,
PKG_CONFIG_PATH, QT_PLUGIN_PATH, QML2_IMPORT_PATH, are ought to live
in a Toolchain file which is passed to CMake by default.
xnox apachelogger: anything else is fragile, and is free to be
changed by CMake both upstream and distribution.
xnox apachelogger: a Toolchain file is the only contract-based
interface to guarantee correct and expected compilation in CMake.
apachelogger xnox: you should probably raise that at
kde-buildsystem@kde.org because that is the actuall way they expect
things to be done
xnox apachelogger: Anybody who uses CMake and does reproducible
builds is using Toolchain files (to e.g. enforce compiler versions /
editions / pick this or that) it's pretty standard.

Since neon is basically doing what [1]  kdesrc-build describe, I
understood that KDE is abusing CMake in a fragile manner. Since
neither abusing nor fragile things are particularly likable I raised
the topic here so things can be made !abusing and !fragile. Apparently
I communicated that concern badly.

[1] http://community.kde.org/Frameworks/Building

HS
___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: frameworks build instructions wrong / won't work with kubuntu 14.04

[Harald Sitter]

On Wed, Dec 18, 2013 at 6:09 PM, Harald Sitter sit...@kde.org wrote:
 tldr: in ubuntu 14.04 automoc will (currently does) fall over dead
 with a qt5 built according to frameworks build instructions. what to
 do?

xnox was nice enough to look into this in detail and identified the
problem as having a much smaller scope than I had originally thought.
In particular this should *not* affected non-distro Qt builds that are
used in manual builds (i.e. not using dpkg build tools). A
solution/improvement for the existing issue is being worked on and
should work as expected in the final.

HS
___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

frameworks build instructions wrong / won't work with kubuntu 14.04

[Harald Sitter]

Alohas,

tldr: in ubuntu 14.04 automoc will (currently does) fall over dead
with a qt5 built according to frameworks build instructions. what to
do?

According to Ubuntu getting cmake to pick up the correct build
binaries (outside system paths) via environment variables is not a
sane way to do it and a toolchain file should be used instead. This is
rendering the present build instructions [1] incompatible with
Ubuntu/Kubuntu 14.04 as its cmake will pick up wrong Qt5 tools (moc
etc.) and therefore fail to build anything with a Qt build created
using the existing instructions.

The related IRC discussion is further down.

Thoughts on this? What do we do about it?

[1] http://community.kde.org/Frameworks/Building

from #kubuntu-devel:
apachelogger bug 1262273
ubottu bug 1262273 in cmake (Ubuntu) 2.8.12.1-1ubuntu2 broke
automoc [Undecided,New] https://launchpad.net/bugs/1262273
apachelogger agateau: ^ in case you want to subscribe as well
xnox apachelogger: does project neon builds it's own qt5?
xnox apachelogger: and is it multiarched like the stock qt5 in ubuntu?
xnox apachelogger: cause in my tool chain I set an explicit path to qt5::moc
agateau apachelogger: does the cmake package has any patch?
xnox apachelogger: is proejct-neon-qt5 multiarched?
xnox apachelogger: if it isn't you are really ought to build
project-neon-cmake, as cmake package in the distribution has been
tailor specifically to the qt5 package as shipped in ubuntu.
apachelogger hm
xnox apachelogger: i think there are variables that you can export
to make it act more like stock cmake, i've made sure there is a
fallback, but it's rather opt-out kind of thing as by default i
enabled cross-compilation without modifying all sources.
apachelogger that's a bug
-*- apachelogger puts on his kde developer hat
xnox apachelogger: what is the full path to moc in qt5? as in not
the qtchooser one.
-*- xnox adjusts my kubuntu-developer badge
apachelogger when working on kde libraries I often want to have my
own qt build, this is not working properly on ubuntu :P
apachelogger xnox: /opt/project-neon5/bin/moc
apachelogger http://paste.ubuntu.com/6595065/
-*- xnox points out, that the correct interface, is to explicitely set
QT5::moc in that case, in your toolchain file used for custom/prefix
installed system libraries, as advised by CMake.
xnox apachelogger: you are pointing to the last resort path. in
cmake there is Qt5::moc already set to a different location.
apachelogger Oo
xnox (as in the code path before that paste)
apachelogger why?
-*- apachelogger fails to compute this just now xD
xnox apachelogger: because the one generated at qt5 package
build-time is always wrong for the cross-compile case on Debian OS.
apachelogger I think the solution is to fix the qt5 cmake config,
not hardcode stuff into cmake?
xnox apachelogger: thus, it's adjusted to the right one. Which is
also, a wild guess, if you don't happen to use stock-cmake with
stock-qt5. Both of them are failing to guess it at all times.
xnox i can guard my changes better, but your are exploiting
implementation details of cmake here.
xnox and it's sad to see that project-neon is diverging so much.
apachelogger it's exploiting the fact that cmake is not supposed to
hardcode stuff :P
xnox apachelogger: wrong. in CMake one should use a Toolchain file
for any non-standard locations. Actual modules are, last fallback, not
the first look up.
xnox apachelogger: if toolchain file sets all variables, none of the
Find* foo modules are loaded, nor executed.
xnox apachelogger: please start using multiarch qt5 packaging then.
apachelogger I'll put it on my todo
xnox cause at the moment one can co-install qt5:i386 and qt5:amd64
which doesn't look possible with project-neon at all.
xnox apachelogger: when you'll do that, you will find your automoc
broken, fyi ;-)
xnox apachelogger: what's your pkg-config? what's your location of
.pc files? this should be passed to CMake via Toolchain file!
apachelogger export PKG_CONFIG_PATH :=
$(NEONDIR)/lib/pkgconfig:$(NEONDIR)/lib/$(DEB_HOST_MULTIARCH)/pkgconfig:$(PKG_CONFIG_PATH)
apachelogger xnox: build foo is here
http://bazaar.launchpad.net/~neon/project-neon5/project-neon5-runtime/view/head:/opt/project-neon5/share/pkg-project-neon5/0/default-settings.mk
xnox apachelogger: cmake_defaults and all the LD_LIBRARY_PATH,
PKG_CONFIG_PATH, QT_PLUGIN_PATH, QML2_IMPORT_PATH, are ought to live
in a Toolchain file which is passed to CMake by default.
xnox apachelogger: anything else is fragile, and is free to be
changed by CMake both upstream and distribution.
xnox apachelogger: a Toolchain file is the only contract-based
interface to guarantee correct and expected compilation in CMake.
apachelogger xnox: you should probably raise that at
kde-buildsystem@kde.org because that is the actuall way they expect
things to be done
xnox apachelogger: Anybody who uses CMake and does reproducible
builds is using Toolchain files (to e.g. enforce compiler versions /
editions / pick this or 

Re: Review Request 110319: drop bodega cmake finder and use cmake config instead

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110319/
---

(Updated May 6, 2013, 1:24 p.m.)


Review request for Build System, Aleix Pol Gonzalez and Aaron J. Seigo.


Description
---

replace finder script with cmake config making stuff scale

it would be cool if someone from kde-buildsystem could check that
everything is in order.

installing a finder yourself makes next to no sense considering cmake
configs do the same thing and do it better.
in particular the finder was installed into cmakedatadir/... which is not
searched by default (at least not by kde applications if kdedatadir !=
cmakedatadir, e.g. on debian /usr/share vs. /usr/share/kde4).
I retained the BODEGA naming but I think one should think about changing it
to Bodega as that looks rather a lot less silly.


Diffs (updated)
-

  CMakeLists.txt 97d1628c0af0ba3276699da20ce307ca22dde95f 
  lib/CMakeLists.txt 381c03f7e26adf95991bf020f214fbebd2060a6f 
  lib/bodega/CMakeLists.txt 1ecc498b494bdff8a75a3a4c49c61f4cdb66ccc4 
  lib/cmake/BODEGAConfig.cmake.in PRE-CREATION 
  lib/cmake/CMakeLists.txt PRE-CREATION 
  lib/cmake/modules/CMakeLists.txt 0207620331bbf949018076ee526010443e6a3173 
  lib/cmake/modules/FindBODEGA.cmake e4c69eec793ef4a7a9728241345c2d1cf9053fac 

Diff: http://git.reviewboard.kde.org/r/110319/diff/


Testing
---

found by muon and muon's backend builds


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Review Request 110319: drop bodega cmake finder and use cmake config instead

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110319/
---

Review request for Build System, Aleix Pol Gonzalez and Aaron J. Seigo.


Description
---

replace finder script with cmake config making stuff scale

it would be cool if someone from kde-buildsystem could check that
everything is in order.

installing a finder yourself makes next to no sense considering cmake
configs do the same thing and do it better.
in particular the finder was installed into cmakedatadir/... which is not
searched by default (at least not by kde applications if kdedatadir !=
cmakedatadir, e.g. on debian /usr/share vs. /usr/share/kde4).
I retained the BODEGA naming but I think one should think about changing it
to Bodega as that looks rather a lot less silly.


Diffs
-

  lib/CMakeLists.txt 381c03f7e26adf95991bf020f214fbebd2060a6f 
  lib/bodega/CMakeLists.txt 1ecc498b494bdff8a75a3a4c49c61f4cdb66ccc4 
  lib/cmake/BODEGAConfig.cmake.in PRE-CREATION 
  lib/cmake/BODEGAConfigVersion.cmake.in PRE-CREATION 
  lib/cmake/CMakeLists.txt PRE-CREATION 
  lib/cmake/modules/CMakeLists.txt 0207620331bbf949018076ee526010443e6a3173 
  lib/cmake/modules/FindBODEGA.cmake e4c69eec793ef4a7a9728241345c2d1cf9053fac 

Diff: http://git.reviewboard.kde.org/r/110319/diff/


Testing
---

found by muon and muon's backend builds


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 110319: drop bodega cmake finder and use cmake config instead

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110319/
---

(Updated May 5, 2013, 3:48 p.m.)


Review request for Build System, Aleix Pol Gonzalez and Aaron J. Seigo.


Description
---

replace finder script with cmake config making stuff scale

it would be cool if someone from kde-buildsystem could check that
everything is in order.

installing a finder yourself makes next to no sense considering cmake
configs do the same thing and do it better.
in particular the finder was installed into cmakedatadir/... which is not
searched by default (at least not by kde applications if kdedatadir !=
cmakedatadir, e.g. on debian /usr/share vs. /usr/share/kde4).
I retained the BODEGA naming but I think one should think about changing it
to Bodega as that looks rather a lot less silly.


Diffs
-

  lib/CMakeLists.txt 381c03f7e26adf95991bf020f214fbebd2060a6f 
  lib/bodega/CMakeLists.txt 1ecc498b494bdff8a75a3a4c49c61f4cdb66ccc4 
  lib/cmake/BODEGAConfig.cmake.in PRE-CREATION 
  lib/cmake/BODEGAConfigVersion.cmake.in PRE-CREATION 
  lib/cmake/CMakeLists.txt PRE-CREATION 
  lib/cmake/modules/CMakeLists.txt 0207620331bbf949018076ee526010443e6a3173 
  lib/cmake/modules/FindBODEGA.cmake e4c69eec793ef4a7a9728241345c2d1cf9053fac 

Diff: http://git.reviewboard.kde.org/r/110319/diff/


Testing
---

found by muon and muon's backend builds


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 110319: drop bodega cmake finder and use cmake config instead

[Harald Sitter]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110319/
---

(Updated May 5, 2013, 3:48 p.m.)


Review request for Build System, Aleix Pol Gonzalez and Aaron J. Seigo.


Description
---

replace finder script with cmake config making stuff scale

it would be cool if someone from kde-buildsystem could check that
everything is in order.

installing a finder yourself makes next to no sense considering cmake
configs do the same thing and do it better.
in particular the finder was installed into cmakedatadir/... which is not
searched by default (at least not by kde applications if kdedatadir !=
cmakedatadir, e.g. on debian /usr/share vs. /usr/share/kde4).
I retained the BODEGA naming but I think one should think about changing it
to Bodega as that looks rather a lot less silly.


Diffs
-

  lib/CMakeLists.txt 381c03f7e26adf95991bf020f214fbebd2060a6f 
  lib/bodega/CMakeLists.txt 1ecc498b494bdff8a75a3a4c49c61f4cdb66ccc4 
  lib/cmake/BODEGAConfig.cmake.in PRE-CREATION 
  lib/cmake/BODEGAConfigVersion.cmake.in PRE-CREATION 
  lib/cmake/CMakeLists.txt PRE-CREATION 
  lib/cmake/modules/CMakeLists.txt 0207620331bbf949018076ee526010443e6a3173 
  lib/cmake/modules/FindBODEGA.cmake e4c69eec793ef4a7a9728241345c2d1cf9053fac 

Diff: http://git.reviewboard.kde.org/r/110319/diff/


Testing
---

found by muon and muon's backend builds


Thanks,

Harald Sitter

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: PulseAudio 1.0 and kdemultimedia 4.7.1

[Harald Sitter]

On Fri, Sep 30, 2011 at 4:02 PM, Colin Guthrie co...@guthr.ie wrote:
 Or is this bit of code copied around horribly?

Yes it is, at least there is a version in kdelibs and one in phonon
(since phonon is a dep of kdelibs we need our own finder), from
looking at the code it seems that phonon's finder should work just
fine whereas kdelibs' fails as it does not use the new macros...
bringing me back to a statement I made on IRC yesterday:
It would be best if PulseAudio were to simply install a cmake config
file, which is roughly the same as pkgconfig, except different format
and path [1]. Maybe something worth considering, since I do not think
there would be much effort involved.

[1] 
http://www.vtk.org/Wiki/CMake/Tutorials/How_to_create_a_ProjectConfig.cmake_file

regards,
H
___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem