D7069: kpackagetool can now write appstream data to a file

2017-08-03 Thread Harald Sitter 
This revision was automatically updated to reflect the committed changes.
sitter marked an inline comment as done.
Closed by commit R290:ef15ef0889d2: kpackagetool can now write appstream data 
to a file (authored by sitter).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D7069?vs=17556=17616#toc

REPOSITORY
  R290 KPackage

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7069?vs=17556=17616

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

AFFECTED FILES
  KF5PackageMacros.cmake
  autotests/CMakeLists.txt
  autotests/data/testpackage-nodisplay/contents.hash
  autotests/data/testpackage-nodisplay/contents/images/empty.png
  autotests/data/testpackage-nodisplay/contents/ui/main.qml
  autotests/data/testpackage-nodisplay/contents/ui/otherfile.qml
  autotests/data/testpackage-nodisplay/metadata.desktop
  autotests/kpackagetoolappstreamtest.cmake
  src/kpackagetool/kpackagetool.cpp
  src/kpackagetool/main.cpp

To: sitter, apol, sebas, mart
Cc: #frameworks

D7069: kpackagetool can now write appstream data to a file

2017-08-03 Thread Harald Sitter 
sitter marked an inline comment as done.
sitter added inline comments.

INLINE COMMENTS

> apol wrote in kpackagetool.cpp:473
> Why the `? true : false`?

lol, brain fart that.

REPOSITORY
  R290 KPackage

BRANCH
  master

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

To: sitter, apol, sebas, mart
Cc: #frameworks

D7069: kpackagetool can now write appstream data to a file

2017-08-02 Thread Aleix Pol Gonzalez 
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  LGTM

INLINE COMMENTS

> kpackagetool.cpp:473
> +// Standard value is unprocessed string we'll need to deal 
> with.
> +object.value("NoDisplay").toString() == 
> QStringLiteral("true") ? true : false) {
> +std::exit(0);

Why the `? true : false`?

REPOSITORY
  R290 KPackage

BRANCH
  master

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

To: sitter, apol, sebas, mart
Cc: #frameworks

D7069: kpackagetool can now write appstream data to a file

2017-08-02 Thread Harald Sitter 
sitter created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  Previously kpackagetool used STDOUT as output for the appstream xml data
  it generated. This has various problems:
  
  - cannot print arbitrary stuff on stdout
  - cmake capturing stdout will always create a file even when there was no 
output
  - NoDisplay sensitive can result in exit(0) but no output resulting in such 
an empty file
  - appstreamcli will complain about bogus files (e.g. empty files)
  - complaints from appstreamcli are treated as test failures
  
  There now is a `--appstream-metainfo-output PATH` argument for kpackagetool
  which switches the tool from STDOUT output mode to file output mode.
  This new mode is used by the kpackage macro to generate appstream data,
  meaning we won't be creating empty files anymore.
  
  kpackagetool testing has been complicated a bit to make sure backwards
  compatible stdout output mode works as well as NoDisplay containing desktop
  files causing no empty files when used with the new file output mode.
  To implement the test NoDisplay checks have been made more aggressive as
  the data structure of Generic packages is actually incompatible with what
  we checked for in kpackagetool, the tool now tries very aggressively to
  find a NoDisplay property.
  
  this fixes a majority of test issues caused by automatic creation of
  appstream data for plasma applets as they no longer create empty files.
  
  CHANGELOG: kpackagetool now can output appstream data to a file

TEST PLAN
  - old tests pass
  - new test passes
  - plasma-workspace no longer generates bogus appstream xml files

REPOSITORY
  R290 KPackage

BRANCH
  master

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

AFFECTED FILES
  KF5PackageMacros.cmake
  autotests/CMakeLists.txt
  autotests/data/testpackage-nodisplay/contents.hash
  autotests/data/testpackage-nodisplay/contents/images/empty.png
  autotests/data/testpackage-nodisplay/contents/ui/main.qml
  autotests/data/testpackage-nodisplay/contents/ui/otherfile.qml
  autotests/data/testpackage-nodisplay/metadata.desktop
  autotests/kpackagetoolappstreamtest.cmake
  src/kpackagetool/kpackagetool.cpp
  src/kpackagetool/main.cpp

To: sitter, apol, sebas, mart
Cc: #frameworks