Re: Review Request 126348: Make it possible to provide the metadata in json

[Sebastian Kügler]

> On Dec. 14, 2015, 9:50 p.m., Sebastian Kügler wrote:
> > src/kpackage/private/packagejobthread.cpp, line 189
> > 
> >
> > Why this change?
> 
> Aleix Pol Gonzalez wrote:
> because currently it's not a text file. I can commit separately if you 
> want.

.desktop and .json are not text files? (Sorry, trying to not be dense here, but 
I don't understand...)


- Sebastian


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


On Dec. 15, 2015, 3:24 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
> 
> (Updated Dec. 15, 2015, 3:24 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Due to KCoreAddons transition from using desktop files to json files, 
> KPackage ended in a weird state where desktop files were allowed to use 
> desktop files but not json.
> 
> This patch makes sure that manifest.json files are also acceptable.
> 
> 
> Diffs
> -
> 
>   autotests/data/testjsonmetadatapackage/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testjsonmetadatapackage/metadata.json PRE-CREATION 
>   autotests/querytest.cpp 0a2f11a 
>   src/kpackage/package.cpp 8416054 
>   src/kpackage/packageloader.cpp 1e1e382 
>   src/kpackage/private/packagejobthread.cpp 444dacc 
>   src/kpackage/private/packages.cpp 2f6bbcf 
> 
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass (except for 
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
>  that is not passing in master).
> Adds a test plugin with a json file in it.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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

Re: Review Request 126366: Make KPluginMetaData constructible from a json path.

[Aleix Pol Gonzalez]

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

(Updated Dec. 15, 2015, 4:14 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 64bf23c85bb86e8e1a15bc7baf875ab0b6a8 by Aleix Pol to 
branch master.


Repository: kcoreaddons


Description
---

Loads the json file upon construction. Simplifies adoption of json metadata 
files for KPackage.


Diffs
-

  src/lib/plugin/kpluginmetadata.h a91b94a 
  src/lib/plugin/kpluginmetadata.cpp 3674360 

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


Testing
---

Tests still pass.


Thanks,

Aleix Pol Gonzalez

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

Jenkins-kde-ci: kcoreaddons master stable-kf5-qt5 » Linux,gcc - Build # 92 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/92/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 15 Dec 2015 16:14:27 +
Build duration: 20 sec

CHANGE SET
No changes
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

lib/x86_64-linux-gnu/libKF5FileMetaData.so | lib/libKF5FileMetaData.3.dylib

[René J . V . Bertin]

Out of curiosity: is it intended that KFileMetaData sets the library 
compatibility version to 3, leading to

/opt/local/lib/x86_64-linux-gnu/libKF5FileMetaData.so
/opt/local/lib/x86_64-linux-gnu/libKF5FileMetaData.so.3
/opt/local/lib/x86_64-linux-gnu/libKF5FileMetaData.so.5.17.0

and on OS X

%> otool -L /opt/local/lib/libKF5FileMetaData.3.dylib
/opt/local/lib/libKF5FileMetaData.3.dylib:
/opt/local/lib/libKF5FileMetaData.3.dylib (compatibility version 3.0.0, 
current version 5.17.0)
/opt/local/lib/libKF5I18n.5.dylib (compatibility version 5.0.0, current 
version 5.17.0)

/opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Versions/5/QtCore 
(compatibility version 5.5.0, current version 5.5.1)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
120.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 1197.1.1)

for packaging purposes it would be preferable if all libraries (symlinks) that 
are recorded in dependents' dependency lists are of the form libKF5*.5.dylib or 
libKF5*.so.5 (KFileMetadata is the only deviant one).

Thanks,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: lib/x86_64-linux-gnu/libKF5FileMetaData.so | lib/libKF5FileMetaData.3.dylib

[René J . V . Bertin]

On Tuesday December 15 2015 18:55:52 René J.V. Bertin wrote:
> for packaging purposes it would be preferable if all libraries (symlinks) 
> that are recorded in dependents' dependency lists are of the form 
> libKF5*.5.dylib or libKF5*.so.5 (KFileMetadata is the only deviant one).

PS : KDE4's kfilemetadata has a compatibility/so version 4 ...

R.

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

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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



kde-modules/KDETemplateGenerator.cmake (line 41)


I'd prefer ``kde_package_app_templates`` (and the module to be called 
``KDEPackageAppTemplates.cmake``).


- Alex Merry


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: system tray test?

[René J . V . Bertin]

René J. V. Bertin wrote:

> Sebastian Kügler wrote:
>> Seems to make sense to me. Could you file a review request for this?
> 
> Ok, will put that on my todo list :)

https://git.reviewboard.kde.org/r/126369/

:)

R.

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

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:40 p.m., Alex Merry wrote:
> > Ooh, also, please write a unit test. I can help with that if you find the 
> > idea of writing a CMake-based unit test daunting, but you can look in the 
> > tests directory for inspiration.
> 
> Marco Martin wrote:
> I'm not sure what the test should do...
> perhaps having in the repo a tarball and a source template, make it 
> generate and then compare the files?

Yes, that would work. Or you could have a source template, and check the 
installed file can be extracted and check the extracted contents. Don't forget 
to make sure that files you think should be excluded are in fact omitted.


- Alex


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


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 84
> > 
> >
> > Use ``KDE_INSTALL_KTEMPLATESDIR`` instead of 
> > ``KTEMPLATES_INSTALL_DIR``. You also need to document that this variable 
> > needs to be set before using the command.
> > 
> > My preference would actually be to make it explicit in the signature 
> > that installation will happen, by having the usage be something like
> > 
> > kde_package_app_templates(
> > TEMPLATES
> > foo
> > bar
> > baz
> > INSTALL_DIR
> > "${KDE_INSTALL_KTEMPLATESDIR}"
> > )
> > 
> > My view is that this makes it explicit what will happen when you're 
> > reading the code - it will package the templates and install them. But I'm 
> > not going to force your hand on this if you'd rather not do it that way.
> 
> Marco Martin wrote:
> would it still take ${KDE_INSTALL_KTEMPLATESDIR} as default or would be 
> mandatory to be passed by the caller?

My preference would be to make it mandatory.


> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 52
> > 
> >
> > Honestly, I'd just use ARG as the prefix - you're in a function, it's 
> > not going to leak anyway. That means you can refer to ARG_TEMPLATES, which 
> > is both shorter and more descriptive.
> > 
> > You should also check for unparsed arguments, and for the TEMPLATES 
> > argument being missing or empty. See, eg, ECMAddAppIcon.cmake for how to do 
> > this.
> 
> Alex Merry wrote:
> Oops, missed one thing here - you need to 
> ``include(CMakeParseArguments)`` earlier in the file.

You still need to do the check for unparsed arguments and for ARG_TEMPLATES not 
being set (ie: the TEMPLATES keyword being missing).


> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 14
> > 
> >
> > OK, what needs to be in the subdirectory? are template1, template2 etc 
> > the subdriectory names? The packages file name?
> > 
> > The following sentence suggests this command only creates a single 
> > template tarball, but the signature here has multiple arguments.
> > 
> > Also, the command doesn't need to include the word "template" twice. I 
> > suggest kde_add_app_template.
> > 
> > Finally, the signature now includes the TEMPLATES keyword.
> 
> Marco Martin wrote:
> the subdirectories have the uncompressed templates in it
> the subdirectory names will correspond to the final package name.

OK, so this needs to go in the documentation. I would write it as

kde_package_app_templates(TEMPLATES  [ [...]])

TEMPLATES lists subdirectories containing template files; each 
 directory will be packaged into a file named 
``.tar.bz2`` and installed to the appropriate location.

You should also either describe what should go into a template, or point to 
documentation elsewhere that describes this.


- Alex


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


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Kde-frameworks-devel mailing list

Re: kdeinit freezes on Wayland in OOM protection

[Martin Gräßlin]

Am 2015-12-15 08:33, schrieb Martin Gräßlin:

Am 2015-12-15 03:20, schrieb Michael Pyne:

On Mon, December 14, 2015 16:07:38 Martin Graesslin wrote:

On Friday, November 27, 2015 1:05:26 PM CET Michael Pyne wrote:
> On Thu, November 26, 2015 13:16:04 Martin Graesslin wrote:
> > we are facing a problem during the startup of Plasma on Wayland. If OOM
> > protection is enabled for kdeinit and we already have a running X
> > server,
> > kdeinit freezes dead.
> >
> > I'm sorry for having ignored the issue for too long and had just
> > disabled
> > OOM protection on my system, so I never hit it. Now I enabled it again
> > to
> > get the problem. On my system I have now two frozen kdeinit processes:
> >
> > martin1960  1956  0 77832 26448   1 13:05 ?00:00:00
> > /opt/kf5/bin/ kdeinit5 --oom-pipe 4 --kded +kcminit_startup
> > martin1961  1960  0 77832  2816   3 13:05 ?00:00:00
> > /opt/kf5/bin/ kdeinit5 --oom-pipe 4 --kded +kcminit_startup
> >
> > One has the following stacktrace:
> > It's frozen in this line of code:
> > sigsuspend();   // wait for the signal to come
> >
> > The other one has the following stacktrace:
> > which is:
> > d.n = read(d.fd[0], , 1);
> >
> > Given that it looks to me like these two processes dead-lock. I do not
> > understand why, why it only happens on Wayland, why the fact that an X
> > server must already be running is relevant and what the OOM protection
> > has
> > to do with it.
>
> I don't have the answer but I can help explain the deadlock better I
> think.

thanks for your input. It helped me understanding quite a bit.

Some more testing results:
Weston+Xwayland: doesn't show the problem
Weston without Xwayland (and DISPLAY=$WAYLAND_DISPLAY): doesn't show 
the

problem.

What I absolutely do not understand how KWin could influence it. From 
all

the backtraces I see it always freezes before interacting with the
windowing system.

Any more ideas to test and investigate, highly appreciated. I got a 
rather
high number of complaints due to that problem and it's a showstopper 
and I'm

lost with it.


Did you add an error check around the set_protection call in 
start_kdeinit.c

and see if that call is failing? (i.e. does "kill(pid, SIGUSR1)" ever
execute?).


yep I added it, but I'm not sure whether it changed anything. When I
gdb'ed into the process it was hanging in the read in the for loop. So
it might or might not have proceeded to the set_protection call.



If the kill() call *is* reached then perhaps SIGUSR1 is 
unintentionally masked
in the 'grandchild' process (the child of kdeinit about to be 
exec()'d).
Perhaps something in the wayland/kwin/weston/x11 library interaction 
blocks

SIGUSR1 from being received in that case?


good news: I found the reason. It was KWin blocking SIGUSR through 
pthread_sigmask and passing it to the child processes created through 
QProcess. By reimplementing setupChildProcess I was able to fix the 
problem.


Thanks a lot for pointing me in the right direction!

And yes, I'll still look into changing to the wait variant.

Cheers
Martin
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kdeclarative master kf5-qt5 » Linux,gcc - Build # 66 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kdeclarative%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/66/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 15 Dec 2015 08:03:19 +
Build duration: 2 min 30 sec

CHANGE SET
Revision aa412c02657828f68f2ac4a3a303853f255525a5 by kde: 
([KQuickControlsAddons MimeDatabase] Expose QMimeType comment)
  change: edit src/qmlcontrols/kquickcontrolsaddons/mimedatabase.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 5/5 (100%)FILES 19/24 (79%)CLASSES 19/24 (79%)LINE 720/1214 
(59%)CONDITIONAL 318/566 (56%)

By packages
  
autotests
FILES 7/7 (100%)CLASSES 7/7 (100%)LINE 388/470 (83%)CONDITIONAL 
192/370 (52%)
src.kdeclarative
FILES 3/4 (75%)CLASSES 3/4 (75%)LINE 153/314 (49%)CONDITIONAL 
59/98 (60%)
src.kdeclarative.private
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 24/58 (41%)CONDITIONAL 
6/10 (60%)
src.qmlcontrols.kquickcontrolsaddons
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 67/118 (57%)CONDITIONAL 
36/50 (72%)
src.quickaddons
FILES 2/6 (33%)CLASSES 2/6 (33%)LINE 88/254 (35%)CONDITIONAL 
25/38 (66%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kdeclarative master kf5-qt5 » Linux,gcc - Build # 66 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kdeclarative%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/66/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 15 Dec 2015 08:03:19 +
Build duration: 2 min 30 sec

CHANGE SET
Revision aa412c02657828f68f2ac4a3a303853f255525a5 by kde: 
([KQuickControlsAddons MimeDatabase] Expose QMimeType comment)
  change: edit src/qmlcontrols/kquickcontrolsaddons/mimedatabase.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 2 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 5/5 (100%)FILES 19/24 (79%)CLASSES 19/24 (79%)LINE 720/1214 
(59%)CONDITIONAL 318/566 (56%)

By packages
  
autotests
FILES 7/7 (100%)CLASSES 7/7 (100%)LINE 388/470 (83%)CONDITIONAL 
192/370 (52%)
src.kdeclarative
FILES 3/4 (75%)CLASSES 3/4 (75%)LINE 153/314 (49%)CONDITIONAL 
59/98 (60%)
src.kdeclarative.private
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 24/58 (41%)CONDITIONAL 
6/10 (60%)
src.qmlcontrols.kquickcontrolsaddons
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 67/118 (57%)CONDITIONAL 
36/50 (72%)
src.quickaddons
FILES 2/6 (33%)CLASSES 2/6 (33%)LINE 88/254 (35%)CONDITIONAL 
25/38 (66%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kdeclarative master stable-kf5-qt5 » Linux,gcc - Build # 66 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kdeclarative%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/66/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 15 Dec 2015 08:03:38 +
Build duration: 10 sec

CHANGE SET
Revision aa412c02657828f68f2ac4a3a303853f255525a5 by kde: 
([KQuickControlsAddons MimeDatabase] Expose QMimeType comment)
  change: edit src/qmlcontrols/kquickcontrolsaddons/mimedatabase.cpp
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

about ki18n/locales: installing only a subset?

[René J . V . Bertin]

Hi,

The KDE4 approach to internationalisation (sic :)) had the huge advantage that 
it was rather trivial to provide installer packages for individual languages so 
users could install only those few languages they would actually use.
It seems that with KF5 we have gotten back in the situation where you get every 
possible language installed. Now that may be nice for the occasional office 
prank, but it'll end up amounting to a significant disk overhead (the one that 
comes with lots of small files) for something that has absolutely no use for 
the vast majority of users.

Is there some central mechanism to control which languages are installed? 
Looking at KF5I18NMacros it would seem that there's only an external "central 
mechanism"; binning the unwanted files before building...

Thanks,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126362: Allow timeout in reset_oom_protection while waiting for SIGUSR1

[Martin Gräßlin]

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

Review request for KDE Frameworks and Michael Pyne.


Repository: kinit


Description
---

If for whatever reason the child process does not get SIGUSR1, it
should not block an infinite time, but timeout and continue.


Diffs
-

  src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 

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


Testing
---

Seems to not work correctly yet as I don't hit the timeout condition with the 
broken kwin_wayland.


Thanks,

Martin Gräßlin

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

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Marco Martin]

> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 14
> > 
> >
> > OK, what needs to be in the subdirectory? are template1, template2 etc 
> > the subdriectory names? The packages file name?
> > 
> > The following sentence suggests this command only creates a single 
> > template tarball, but the signature here has multiple arguments.
> > 
> > Also, the command doesn't need to include the word "template" twice. I 
> > suggest kde_add_app_template.
> > 
> > Finally, the signature now includes the TEMPLATES keyword.

the subdirectories have the uncompressed templates in it
the subdirectory names will correspond to the final package name.


> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 84
> > 
> >
> > Use ``KDE_INSTALL_KTEMPLATESDIR`` instead of 
> > ``KTEMPLATES_INSTALL_DIR``. You also need to document that this variable 
> > needs to be set before using the command.
> > 
> > My preference would actually be to make it explicit in the signature 
> > that installation will happen, by having the usage be something like
> > 
> > kde_package_app_templates(
> > TEMPLATES
> > foo
> > bar
> > baz
> > INSTALL_DIR
> > "${KDE_INSTALL_KTEMPLATESDIR}"
> > )
> > 
> > My view is that this makes it explicit what will happen when you're 
> > reading the code - it will package the templates and install them. But I'm 
> > not going to force your hand on this if you'd rather not do it that way.

would it still take ${KDE_INSTALL_KTEMPLATESDIR} as default or would be 
mandatory to be passed by the caller?


- Marco


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


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 126350: Fixed all Clazy level 1 and level 2 warnings

[Artur Puzio]

> On Gru 14, 2015, 8:10 po południu, Sergio Martins wrote:
> > src/klocalizedstring.cpp, line 389
> > 
> >
> > this will break the build with MSVC, it doesn't support QStringLiteral 
> > with multi-line literals.
> > 
> > (hint:
> > export 
> > CLAZY_EXTRA_OPTIONS="qstring-uneeded-heap-allocations-msvc-compat"
> > )
> 
> Nick Shaforostoff wrote:
> why QString is used for debug output in the first place?
> there is operator<<(const char * s) for latin1 text, which is the case
> (i.e. no non-latin symbols are used in the literals)
> 
> using strings for debugging output is suboptimal
> 
> Artur Puzio wrote:
> look at line 487:
> ```
> qWarning() << QStringLiteral(
>  "Plural argument to message {%1} not 
> supplied before conversion.")
>  
> .arg(shortenMessage(QString::fromUtf8(text)));
> ```
> Some of QString-s can by changed to const char *, but in other places 
> they are required.
> 
> Aleix Pol Gonzalez wrote:
> `qWarning() << "Plural argument to message {" << 
> shortenMessage(QString::fromUtf8(text))<< "} not supplied before 
> conversion.";`
> 
> Still, I don't believe it's up to this iteration to fix this one, the 
> code was already like this.
> Artur, can you look at the issue pointed out by Sergio?

Just fixed the issue pointed out by Sergio. Also chenged to char[] when 
writeing literals to warning.


- Artur


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


On Gru 15, 2015, 12:34 po południu, Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126350/
> ---
> 
> (Updated Gru 15, 2015, 12:34 po południu)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Fixed `warning: KuitSetup has dtor, copy-ctor but not copy-assignment 
> [-Wclazy-rule-of-three]`
> Fixed `warning: Reserve candidate [-Wclazy-reserve-candidates]`
> Fixed `warning: Use QVariant::toFoo() instead of QVariant::value() 
> [-Wclazy-variant-sanitizer]`
> Fixed `warning: Use midRef() instead [-Wclazy-qstring-ref]`
> Fixed `warning: Pass small and trivially-copyable type by value (const class 
> QChar &) [-Wclazy-foreach]`
> Fixed `warning: QString::fromLatin1() being passed a literal 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(const char*) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(QLatin1String) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Changed `QLatin1String`-s to `char[]` when writteing to warning.
> Changed to `QStringLitera`-s in some lines, where Clazy isn't warning.
> 
> 
> Diffs
> -
> 
>   src/common_helpers.cpp dad7f84 
>   src/kcatalog.cpp 7711e9b 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
>   src/ktranscript.cpp 04dda66 
>   src/kuitmarkup.h d110ca3 
>   src/kuitmarkup.cpp 02b891a 
> 
> Diff: https://git.reviewboard.kde.org/r/126350/diff/
> 
> 
> Testing
> ---
> 
> Automated tests 1,3 and 4 passing.
> Test 2 fails on my system both: after changes and before.
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

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

Re: Review Request 126350: Fixed all Clazy level 1 and level 2 warnings

[Sergio Martins]

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



src/kuitmarkup.h (line 193)


extra whitespace


- Sergio Martins


On Dec. 15, 2015, 11:34 a.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126350/
> ---
> 
> (Updated Dec. 15, 2015, 11:34 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Fixed `warning: KuitSetup has dtor, copy-ctor but not copy-assignment 
> [-Wclazy-rule-of-three]`
> Fixed `warning: Reserve candidate [-Wclazy-reserve-candidates]`
> Fixed `warning: Use QVariant::toFoo() instead of QVariant::value() 
> [-Wclazy-variant-sanitizer]`
> Fixed `warning: Use midRef() instead [-Wclazy-qstring-ref]`
> Fixed `warning: Pass small and trivially-copyable type by value (const class 
> QChar &) [-Wclazy-foreach]`
> Fixed `warning: QString::fromLatin1() being passed a literal 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(const char*) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(QLatin1String) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Changed `QLatin1String`-s to `char[]` when writteing to warning.
> Changed to `QStringLitera`-s in some lines, where Clazy isn't warning.
> 
> 
> Diffs
> -
> 
>   src/common_helpers.cpp dad7f84 
>   src/kcatalog.cpp 7711e9b 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
>   src/ktranscript.cpp 04dda66 
>   src/kuitmarkup.h d110ca3 
>   src/kuitmarkup.cpp 02b891a 
> 
> Diff: https://git.reviewboard.kde.org/r/126350/diff/
> 
> 
> Testing
> ---
> 
> Automated tests 1,3 and 4 passing.
> Test 2 fails on my system both: after changes and before.
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

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

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Marco Martin]

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

(Updated Dec. 15, 2015, 10:38 a.m.)


Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
and Simon Wächter.


Repository: extra-cmake-modules


Description
---

templates are very useful as teaching tool in order to make
a minimal application that uses a certain framework.
templates in the KAppTemplate repository will always get forgotten
(plus kapptemplate is not really necessary as they work in kdevelop as well)
An ideal situation would be frameworks having templates in their own repos
with templates of barebone apps using the main framework features.
In order to do that, the cmake stuff needed in order to correctly install
a template needs to be ported to a place avaiable to all frameworks


Diffs (updated)
-

  kde-modules/KDEInstallDirs.cmake b7cd34d 
  kde-modules/KDETemplateGenerator.cmake PRE-CREATION 

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


Testing
---

done some templates installed by plasma-framework


Thanks,

Marco Martin

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

Re: Review Request 126350: Fixed all Clazy level 1 and level 2 warnings

[Artur Puzio]

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

(Updated Gru 15, 2015, 12:34 po południu)


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Fixed issue with MSVC not supporting multiline QStringLiteral-s.


Repository: ki18n


Description (updated)
---

Fixed `warning: KuitSetup has dtor, copy-ctor but not copy-assignment 
[-Wclazy-rule-of-three]`
Fixed `warning: Reserve candidate [-Wclazy-reserve-candidates]`
Fixed `warning: Use QVariant::toFoo() instead of QVariant::value() 
[-Wclazy-variant-sanitizer]`
Fixed `warning: Use midRef() instead [-Wclazy-qstring-ref]`
Fixed `warning: Pass small and trivially-copyable type by value (const class 
QChar &) [-Wclazy-foreach]`
Fixed `warning: QString::fromLatin1() being passed a literal 
[-Wclazy-qstring-uneeded-heap-allocations]`
Fixed `warning: QString(const char*) being called 
[-Wclazy-qstring-uneeded-heap-allocations]`
Fixed `warning: QString(QLatin1String) being called 
[-Wclazy-qstring-uneeded-heap-allocations]`
Changed `QLatin1String`-s to `char[]` when writteing to warning.
Changed to `QStringLitera`-s in some lines, where Clazy isn't warning.


Diffs (updated)
-

  src/common_helpers.cpp dad7f84 
  src/kcatalog.cpp 7711e9b 
  src/klocalizedcontext.cpp 3bc42dd 
  src/klocalizedstring.cpp 69950d2 
  src/ktranscript.cpp 04dda66 
  src/kuitmarkup.h d110ca3 
  src/kuitmarkup.cpp 02b891a 

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


Testing
---

Automated tests 1,3 and 4 passing.
Test 2 fails on my system both: after changes and before.


Thanks,

Artur Puzio

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

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Marco Martin]

> On Dec. 12, 2015, 3:40 p.m., Alex Merry wrote:
> > Ooh, also, please write a unit test. I can help with that if you find the 
> > idea of writing a CMake-based unit test daunting, but you can look in the 
> > tests directory for inspiration.

I'm not sure what the test should do...
perhaps having in the repo a tarball and a source template, make it generate 
and then compare the files?


- Marco


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


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 126350: Fixed all Clazy level 1 and level 2 warnings

[Artur Puzio]

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

(Updated Gru 15, 2015, 12:45 po południu)


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

removed extra whitespace


Repository: ki18n


Description
---

Fixed `warning: KuitSetup has dtor, copy-ctor but not copy-assignment 
[-Wclazy-rule-of-three]`
Fixed `warning: Reserve candidate [-Wclazy-reserve-candidates]`
Fixed `warning: Use QVariant::toFoo() instead of QVariant::value() 
[-Wclazy-variant-sanitizer]`
Fixed `warning: Use midRef() instead [-Wclazy-qstring-ref]`
Fixed `warning: Pass small and trivially-copyable type by value (const class 
QChar &) [-Wclazy-foreach]`
Fixed `warning: QString::fromLatin1() being passed a literal 
[-Wclazy-qstring-uneeded-heap-allocations]`
Fixed `warning: QString(const char*) being called 
[-Wclazy-qstring-uneeded-heap-allocations]`
Fixed `warning: QString(QLatin1String) being called 
[-Wclazy-qstring-uneeded-heap-allocations]`
Changed `QLatin1String`-s to `char[]` when writteing to warning.
Changed to `QStringLitera`-s in some lines, where Clazy isn't warning.


Diffs (updated)
-

  src/common_helpers.cpp dad7f84 
  src/kcatalog.cpp 7711e9b 
  src/klocalizedcontext.cpp 3bc42dd 
  src/klocalizedstring.cpp 69950d2 
  src/ktranscript.cpp 04dda66 
  src/kuitmarkup.h d110ca3 
  src/kuitmarkup.cpp 02b891a 

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


Testing
---

Automated tests 1,3 and 4 passing.
Test 2 fails on my system both: after changes and before.


Thanks,

Artur Puzio

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

Jenkins-kde-ci: kcoreaddons master stable-kf5-qt5 » Linux,gcc - Build # 93 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/93/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Wed, 16 Dec 2015 00:29:03 +
Build duration: 2 min 30 sec

CHANGE SET
Revision 64bf23c85bb86e8e1a15bc7baf875ab0b6a8 by aleixpol: (Make 
KPluginMetaData constructible from a json path)
  change: edit autotests/kpluginmetadatatest.cpp
  change: edit src/lib/plugin/kpluginmetadata.cpp
  change: add autotests/data/testmetadata.json
  change: edit src/lib/plugin/kpluginmetadata.h
Revision 2e809b362fae6faaab320ebf86ef6dc297f7e0e5 by Kevin Funk: (Minor: 
Cleanup include grouping)
  change: edit src/lib/plugin/kpluginmetadata.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 21 test(s), Skipped: 0 test(s), Total: 
21 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 10/10 (100%)FILES 74/88 (84%)CLASSES 74/88 (84%)LINE 5510/7658 
(72%)CONDITIONAL 11768/21869 (54%)

By packages
  
autotests
FILES 31/38 (82%)CLASSES 31/38 (82%)LINE 2351/2455 
(96%)CONDITIONAL 7060/13969 (51%)
src.desktoptojson
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 84/106 (79%)CONDITIONAL 
214/417 (51%)
src.lib
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 230/482 (48%)CONDITIONAL 
188/351 (54%)
src.lib.caching
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 352/764 (46%)CONDITIONAL 
202/384 (53%)
src.lib.io
FILES 9/10 (90%)CLASSES 9/10 (90%)LINE 740/1226 
(60%)CONDITIONAL 894/1466 (61%)
src.lib.jobs
FILES 5/7 (71%)CLASSES 5/7 (71%)LINE 157/305 (51%)CONDITIONAL 
55/92 (60%)
src.lib.plugin
FILES 7/7 (100%)CLASSES 7/7 (100%)LINE 625/721 (87%)CONDITIONAL 
1194/2049 (58%)
src.lib.randomness
FILES 1/2 (50%)CLASSES 1/2 (50%)LINE 21/98 (21%)CONDITIONAL 
22/36 (61%)
src.lib.text
FILES 5/8 (63%)CLASSES 5/8 (63%)LINE 312/729 (43%)CONDITIONAL 
1094/1755 (62%)
src.lib.util
FILES 10/10 (100%)CLASSES 10/10 (100%)LINE 638/772 
(83%)CONDITIONAL 845/1350 (63%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126362: Allow timeout in reset_oom_protection while waiting for SIGUSR1

[Michael Pyne]

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

Ship it!


I hacked my kinit.git to not send the SIGUSR1 at all and the patch still 
allowed kdeinit to exec processes (albeit noticeably slower), so seems to me it 
should work. No other ill effects noted either.

- Michael Pyne


On Dec. 15, 2015, 10:19 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126362/
> ---
> 
> (Updated Dec. 15, 2015, 10:19 a.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> If for whatever reason the child process does not get SIGUSR1, it
> should not block an infinite time, but timeout and continue.
> 
> 
> Diffs
> -
> 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126362/diff/
> 
> 
> Testing
> ---
> 
> Seems to not work correctly yet as I don't hit the timeout condition with the 
> broken kwin_wayland.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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

Re: kdeinit freezes on Wayland in OOM protection

[Michael Pyne]

On Tue, December 15, 2015 09:05:52 Martin Gräßlin wrote:
> >> If the kill() call *is* reached then perhaps SIGUSR1 is
> >> unintentionally masked
> >> in the 'grandchild' process (the child of kdeinit about to be
> >> exec()'d).
> >> Perhaps something in the wayland/kwin/weston/x11 library interaction
> >> blocks
> >> SIGUSR1 from being received in that case?
> 
> good news: I found the reason. It was KWin blocking SIGUSR through
> pthread_sigmask and passing it to the child processes created through
> QProcess. By reimplementing setupChildProcess I was able to fix the
> problem.
> 
> Thanks a lot for pointing me in the right direction!

\o/

:)

Regards,
 - Michael Pyne
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126366: Make KPluginMetaData constructible from a json path.

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

Loads the json file upon construction. Simplifies adoption of json metadata 
files for KPackage.


Diffs
-

  src/lib/plugin/kpluginmetadata.h a91b94a 
  src/lib/plugin/kpluginmetadata.cpp 3674360 

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


Testing
---

Tests still pass.


Thanks,

Aleix Pol Gonzalez

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

Re: Review Request 126362: Allow timeout in reset_oom_protection while waiting for SIGUSR1

[Michael Pyne]

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

Ship it!


I can confirm the change appears correct, though I'd like to try to force it to 
work myself if we can't get the broken kwin_wayland to trip the bug. One very 
minor comment but that's your call.


src/kdeinit/kinit.cpp (line 465)


I'd recommend Q_NULLPTR instead of NULL here, but that's mere quibbling 
(though being bit once by a C-style NULL in C++ code in 64-bit code was more 
than enough).


- Michael Pyne


On Dec. 15, 2015, 10:19 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126362/
> ---
> 
> (Updated Dec. 15, 2015, 10:19 a.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> If for whatever reason the child process does not get SIGUSR1, it
> should not block an infinite time, but timeout and continue.
> 
> 
> Diffs
> -
> 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126362/diff/
> 
> 
> Testing
> ---
> 
> Seems to not work correctly yet as I don't hit the timeout condition with the 
> broken kwin_wayland.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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

Re: Review Request 126339: remove kdewin dependency

[David Faure]

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

Ship it!


Ship It!

- David Faure


On Dec. 14, 2015, 10:37 a.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126339/
> ---
> 
> (Updated Dec. 14, 2015, 10:37 a.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: khtml
> 
> 
> Description
> ---
> 
> This removes the direct kdewin dependency by replacing problematic
> function calls (uname, snprintf) with their Qt counterparts.
> Some leftover unix header includes removed too.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 51fe02a01c8166046d1c6085ec4a5b6e617e1fea 
>   src/ecma/kjs_binding.cpp 5e122f0f0d70e6734565e7ab205d14a92c79d287 
>   src/ecma/kjs_navigator.cpp 2f7be8d11e5af84e08ac640ccbc97f70aeac8abd 
>   src/ecma/kjs_proxy.h 24abd1e1bac0932f8829f02185953142c74aadc8 
>   src/ecma/kjs_proxy.cpp 20430f48fce986ca654c49c5771ad839845f11ab 
>   src/khtml_pagecache.cpp 8e1841f6b0e816dfd8faa76f2191b269c4716011 
>   src/khtml_part.cpp adbcd800a526e9f8fd92a553e62ee64791872938 
>   src/kmultipart/kmultipart.cpp 1ad3bbb9b6d6a022799d5ef85f426fc9f911d45b 
> 
> Diff: https://git.reviewboard.kde.org/r/126339/diff/
> 
> 
> Testing
> ---
> 
> Windows, Linux compiles.
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

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

Re: Review Request 126348: Make it possible to provide the metadata in json

[Aleix Pol Gonzalez]

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

(Updated Dec. 15, 2015, 4:24 p.m.)


Review request for KDE Frameworks and Plasma.


Changes
---

Address some issues.


Repository: kpackage


Description
---

Due to KCoreAddons transition from using desktop files to json files, KPackage 
ended in a weird state where desktop files were allowed to use desktop files 
but not json.

This patch makes sure that manifest.json files are also acceptable.


Diffs (updated)
-

  autotests/data/testjsonmetadatapackage/contents/ui/main.qml PRE-CREATION 
  autotests/data/testjsonmetadatapackage/metadata.json PRE-CREATION 
  autotests/querytest.cpp 0a2f11a 
  src/kpackage/package.cpp 8416054 
  src/kpackage/packageloader.cpp 1e1e382 
  src/kpackage/private/packagejobthread.cpp 444dacc 
  src/kpackage/private/packages.cpp 2f6bbcf 

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


Testing
---

Tests still pass (except for 
https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
 that is not passing in master).
Adds a test plugin with a json file in it.


Thanks,

Aleix Pol Gonzalez

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

Re: Review Request 126348: Make it possible to provide the metadata in json

[Aleix Pol Gonzalez]

> On Dec. 14, 2015, 10:50 p.m., Sebastian Kügler wrote:
> > src/kpackage/private/packagejobthread.cpp, line 189
> > 
> >
> > Why this change?

because currently it's not a text file. I can commit separately if you want.


- Aleix


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


On Dec. 14, 2015, 6:11 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
> 
> (Updated Dec. 14, 2015, 6:11 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Due to KCoreAddons transition from using desktop files to json files, 
> KPackage ended in a weird state where desktop files were allowed to use 
> desktop files but not json.
> 
> This patch makes sure that manifest.json files are also acceptable.
> 
> 
> Diffs
> -
> 
>   autotests/querytest.cpp 0a2f11a 
>   src/kpackage/package.cpp 8416054 
>   src/kpackage/packageloader.cpp 1e1e382 
>   src/kpackage/private/packagejobthread.cpp 444dacc 
>   src/kpackage/private/packagejobthread_p.h 1babf0b 
>   src/kpackage/private/packages.cpp 2f6bbcf 
> 
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass (except for 
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
>  that is not passing in master).
> Adds a test plugin with a json file in it.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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

Re: Review Request 126366: Make KPluginMetaData constructible from a json path.

[Aleix Pol Gonzalez]

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

(Updated Dec. 15, 2015, 4:27 p.m.)


Review request for KDE Frameworks.


Changes
---

turn assert into warning.


Repository: kcoreaddons


Description
---

Loads the json file upon construction. Simplifies adoption of json metadata 
files for KPackage.


Diffs (updated)
-

  src/lib/plugin/kpluginmetadata.h a91b94a 
  src/lib/plugin/kpluginmetadata.cpp 3674360 

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


Testing
---

Tests still pass.


Thanks,

Aleix Pol Gonzalez

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

Re: Review Request 126366: Make KPluginMetaData constructible from a json path.

[Alex Richardson]

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

Ship it!


If you have time, a minimal unit test would be nice to make sure we don't break 
this in future.

- Alex Richardson


On Dec. 15, 2015, 3:27 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126366/
> ---
> 
> (Updated Dec. 15, 2015, 3:27 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Loads the json file upon construction. Simplifies adoption of json metadata 
> files for KPackage.
> 
> 
> Diffs
> -
> 
>   src/lib/plugin/kpluginmetadata.h a91b94a 
>   src/lib/plugin/kpluginmetadata.cpp 3674360 
> 
> Diff: https://git.reviewboard.kde.org/r/126366/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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

Re: Review Request 126348: Make it possible to provide the metadata in json

[Alex Richardson]

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



src/kpackage/package.cpp (line 24)


no longer required, right?



src/kpackage/package.cpp (line 916)


`path.endsWith("/metadata.desktop") || path.endsWith("/metadata.json")`? 
QRegExp seems like overkill here.


- Alex Richardson


On Dec. 15, 2015, 3:24 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
> 
> (Updated Dec. 15, 2015, 3:24 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> Due to KCoreAddons transition from using desktop files to json files, 
> KPackage ended in a weird state where desktop files were allowed to use 
> desktop files but not json.
> 
> This patch makes sure that manifest.json files are also acceptable.
> 
> 
> Diffs
> -
> 
>   autotests/data/testjsonmetadatapackage/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testjsonmetadatapackage/metadata.json PRE-CREATION 
>   autotests/querytest.cpp 0a2f11a 
>   src/kpackage/package.cpp 8416054 
>   src/kpackage/packageloader.cpp 1e1e382 
>   src/kpackage/private/packagejobthread.cpp 444dacc 
>   src/kpackage/private/packages.cpp 2f6bbcf 
> 
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass (except for 
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
>  that is not passing in master).
> Adds a test plugin with a json file in it.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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

Re: Review Request 126366: Make KPluginMetaData constructible from a json path.

[Sebastian Kügler]

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


An autotest would be nice, and I think the ASSERT is too aggressive.


src/lib/plugin/kpluginmetadata.cpp (line 68)


I don't like the assert here. It may be third party data, and we're 
crashing the application then?


- Sebastian Kügler


On Dec. 15, 2015, 3:04 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126366/
> ---
> 
> (Updated Dec. 15, 2015, 3:04 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Loads the json file upon construction. Simplifies adoption of json metadata 
> files for KPackage.
> 
> 
> Diffs
> -
> 
>   src/lib/plugin/kpluginmetadata.h a91b94a 
>   src/lib/plugin/kpluginmetadata.cpp 3674360 
> 
> Diff: https://git.reviewboard.kde.org/r/126366/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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