Re: Review Request 119590: Disable the DDS and JPEG-2000 plugins when Qt version is 5.3 or later
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119590/ --- (Updated Aug. 4, 2014, 9:51 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Alex Merry. Repository: kimageformats Description --- QtImageFormats 5.3 comes with DDS and JPEG-2000 plugins that support more options and are generally better than our plugins. The only advantage our plugins offer is that the Qt DDS plugin does not work on sequential devices, while ours does. This is outweighed by other improvements, though, such as supporting more variants. (NB: I opted for disabling rather than removing because we want distros to push new KF5 versions back to stable distro versions, but they are probably not going to do the same with Qt 5.2 - 5.3. Diffs - CMakeLists.txt 9aa14b44b6d5ccb484dc2ef58cc8d23cb392f1c0 src/imageformats/CMakeLists.txt af169c1be308afae92992751d4cb655ef9b30a2f src/imageformats/dds-qt.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119590/diff/ Testing --- Built with Qt 5.3.1 - the JPEG-2000 and DDS plugins are not built, and the JPEG-2000 tests are not built or run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KDE Frameworks 5.1.0 release
On Saturday 02 August 2014 15:48:01 David Faure wrote: I just tagged and packed 5.1.0. Main changes: * KTextEditor: Major refactorings and improvements of the vi-mode * KAuth: Now based on PolkitQt5-1 * New migration agent for KWallet * Windows compilation fixes * Internationalization fixes * New install dir for kxmlgui files * A number of bugfixes k-f-d: anything to add? e-c-m 1.1.0 defines two new installation directories in KDEInstallDirs: CMAKE_INSTALL_KXMLGUI5DIR (for the above-noted new kxmlgui installation dir), and CMAKE_INSTALL_METAINFODIR, for AppStream metainfo (aka upstream metadata) files. If changelogs in announcements are useful, then we should start using ChangeLog lines in commits like Qt does, it would save me time digging into my email archive and trying to summarize series of commits... Good idea. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119590: Disable the JPEG-2000 plugin when Qt version is 4.3 or later
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119590/ --- Review request for KDE Frameworks and Alex Merry. Repository: kimageformats Description --- QtImageFormats 4.3 comes with a JPEG-2000 plugin, which supports more options and is more intelligent about what format to use for writing. Diffs - CMakeLists.txt 9aa14b44b6d5ccb484dc2ef58cc8d23cb392f1c0 Diff: https://git.reviewboard.kde.org/r/119590/diff/ Testing --- Built with Qt 4.3.1 - the JPEG-2000 plugin is not built, and its tests are not built or run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119590: Disable the JPEG-2000 plugin when Qt version is 5.3 or later
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119590/ --- (Updated Aug. 3, 2014, 5:53 p.m.) Review request for KDE Frameworks and Alex Merry. Changes --- Oops, Qt 5.3, not Qt 4.3. Summary (updated) - Disable the JPEG-2000 plugin when Qt version is 5.3 or later Repository: kimageformats Description (updated) --- QtImageFormats 5.3 comes with a JPEG-2000 plugin, which supports more options and is more intelligent about what format to use for writing. Diffs (updated) - CMakeLists.txt 9aa14b44b6d5ccb484dc2ef58cc8d23cb392f1c0 Diff: https://git.reviewboard.kde.org/r/119590/diff/ Testing (updated) --- Built with Qt 5.3.1 - the JPEG-2000 plugin is not built, and its tests are not built or run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119590: Disable the DDS and JPEG-2000 plugins when Qt version is 5.3 or later
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119590/ --- (Updated Aug. 3, 2014, 7:12 p.m.) Review request for KDE Frameworks and Alex Merry. Changes --- Also, DDS. Summary (updated) - Disable the DDS and JPEG-2000 plugins when Qt version is 5.3 or later Repository: kimageformats Description (updated) --- QtImageFormats 5.3 comes with DDS and JPEG-2000 plugins that support more options and are generally better than our plugins. The only advantage our plugins offer is that the Qt DDS plugin does not work on sequential devices, while ours does. This is outweighed by other improvements, though, such as supporting more variants. Diffs (updated) - CMakeLists.txt 9aa14b44b6d5ccb484dc2ef58cc8d23cb392f1c0 src/imageformats/CMakeLists.txt af169c1be308afae92992751d4cb655ef9b30a2f src/imageformats/dds-qt.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119590/diff/ Testing (updated) --- Built with Qt 5.3.1 - the JPEG-2000 and DDS plugins are not built, and the JPEG-2000 tests are not built or run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119590: Disable the DDS and JPEG-2000 plugins when Qt version is 5.3 or later
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119590/ --- (Updated Aug. 3, 2014, 7:16 p.m.) Review request for KDE Frameworks and Alex Merry. Repository: kimageformats Description (updated) --- QtImageFormats 5.3 comes with DDS and JPEG-2000 plugins that support more options and are generally better than our plugins. The only advantage our plugins offer is that the Qt DDS plugin does not work on sequential devices, while ours does. This is outweighed by other improvements, though, such as supporting more variants. (NB: I opted for disabling rather than removing because we want distros to push new KF5 versions back to stable distro versions, but they are probably not going to do the same with Qt 4.2 - 4.3. Diffs - CMakeLists.txt 9aa14b44b6d5ccb484dc2ef58cc8d23cb392f1c0 src/imageformats/CMakeLists.txt af169c1be308afae92992751d4cb655ef9b30a2f src/imageformats/dds-qt.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119590/diff/ Testing --- Built with Qt 5.3.1 - the JPEG-2000 and DDS plugins are not built, and the JPEG-2000 tests are not built or run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119590: Disable the DDS and JPEG-2000 plugins when Qt version is 5.3 or later
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119590/ --- (Updated Aug. 3, 2014, 7:57 p.m.) Review request for KDE Frameworks and Alex Merry. Changes --- 5.3, not 4.3 Repository: kimageformats Description (updated) --- QtImageFormats 5.3 comes with DDS and JPEG-2000 plugins that support more options and are generally better than our plugins. The only advantage our plugins offer is that the Qt DDS plugin does not work on sequential devices, while ours does. This is outweighed by other improvements, though, such as supporting more variants. (NB: I opted for disabling rather than removing because we want distros to push new KF5 versions back to stable distro versions, but they are probably not going to do the same with Qt 5.2 - 5.3. Diffs - CMakeLists.txt 9aa14b44b6d5ccb484dc2ef58cc8d23cb392f1c0 src/imageformats/CMakeLists.txt af169c1be308afae92992751d4cb655ef9b30a2f src/imageformats/dds-qt.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119590/diff/ Testing --- Built with Qt 5.3.1 - the JPEG-2000 and DDS plugins are not built, and the JPEG-2000 tests are not built or run. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119564: Add define to re-enable Qt functionality we depend on.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/#review63665 --- I would rather change the code. The Qt behaviour was changed for a reason, to prevent accidental use of dangerous behaviour, and I'm not too keen on undoing that move for all software that uses KDECompilerSettings. - Alex Merry On Aug. 1, 2014, 4:06 p.m., Axel Rasmussen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- (Updated Aug. 1, 2014, 4:06 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119457: Script to Fix ecm_install_icons Deprecation Warnings
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119457/#review63113
---
Ship it!
Thank you, you are awesome!
A couple of notes about your test repo (for the benefit of your future porting
efforts): ${ECM_KDE_MODULE_DIR} is contained in ${ECM_MODULE_PATH}, and so
shouldn't be added to the CMAKE_MODULE_PATH, and DATA_INSTALL_DIR won't be
defined if you don't include KDEInstallDirs (and you should be using
CMAKE_INSTALL_DATADIR anyway, really). I had to replace ${DATA_INSTALL_DIR}
with share/icons to test installation.
- Alex Merry
On July 25, 2014, 2:49 a.m., David Narváez wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119457/
---
(Updated July 25, 2014, 2:49 a.m.)
Review request for KDE Frameworks.
Repository: kde-dev-scripts
Description
---
The script
1) Fixes icon categories when found wrong, eg. action - actions
2) Renames icons to remove theme prefix
3) Rewrites the CMakeLists.txt file to install icons grouped by themes
It does not support l10n codes.
Diffs
-
kf5/fix-ecm-install-icons.pl PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/119457/diff/
Testing
---
1) Unpack the attached test repository
2) Configure the repository using cmake - should display warnings
3) Fix the CMakeLists.txt file using this script
4) Reconfigure the repository using cmake - warnings should be gone
File Attachments
Test Repository
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/25/f598db77-95a6-4505-b02d-72304b79d65b__iconstest.tgz
Thanks,
David Narváez
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Porting: Installing Icons
On Friday 25 July 2014 02:52:09 Eike Hein wrote: On 07/24/2014 08:35 PM, Alex Merry wrote: As Jonathan said, yes, you are correct. And, yes, I should really write a porting script for that. Yay! That's the excuse I needed to avoid fixing Konvi's icon installs until your script is ready ;) Thanks to David Narváez, there is now such a script. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119417: kconfig: Fix QBasicAtomicInt being treated as int (reposted)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119417/#review63142 --- Ship it! Ship It! - Alex Merry On July 22, 2014, 11:35 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119417/ --- (Updated July 22, 2014, 11:35 p.m.) Review request for KDE Frameworks. Repository: kconfig Description --- kconfig fails building because QBasicAtomicInt is being treated as an int. This patch fixes that problem. It was posted for review by Andreas Xavier (https://git.reviewboard.kde.org/r/119257/), but he later discarded it. I have asked him many times why it was discarded, with no answer whatsoever. Since this patch is needed to compile kconfig, I resend it. Diffs - src/core/kconfig.cpp 14a5d39 Diff: https://git.reviewboard.kde.org/r/119417/diff/ Testing --- Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Porting: Installing Icons
On Wednesday 23 July 2014 18:08:42 David Narvaez wrote: Hi, The information at [0] seems to indicate I just need to worry about renaming files to match the new categories (btw, is there a script for that?) but what I get from CMake Warning (dev) at /usr/share/ECM/modules/ECMInstallIcons.cmake:205 (message): ecm_install_icons() with no ICONS argument is deprecated Call Stack (most recent call first): icons/CMakeLists.txt:2 (ecm_install_icons) This warning is for project developers. Use -Wno-dev to suppress it. is that I need to update to # ecm_install_icons(ICONS icon [icon [...]] # DESTINATION icon_install_dir # [LANG l10n_code] # [THEME theme]) which seems to me like I need to explicitly list all icons in the dir, and # The given icons, whose names must match the pattern:: # # size-group-name.ext seems to indicate I need to rename all of the files to remove the theme from the prefix. Am I correct? Should I update the wiki with the complete oddyssey to a correct ecm_install_icions? As Jonathan said, yes, you are correct. And, yes, I should really write a porting script for that. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119079: Add utility function for loading all plugins from a given dir + easy accessor for metadata
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/#review62731 --- src/lib/plugin/kpluginloader.h https://git.reviewboard.kde.org/r/119079/#comment43481 Even that isn't technically guaranteed - QLibrary::isLibrary() just looks at the extension. autotests/kpluginloadertest.cpp https://git.reviewboard.kde.org/r/119079/#comment43482 Just noticed that none of these tests check that your methods ever load more than one plugin :-) - Alex Merry On July 19, 2014, 3:41 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/ --- (Updated July 19, 2014, 3:41 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- This class simplifies reading the metadata from a qt plugin by providing type safe accessor functions for the standard plugininfo keys that are also used by the .desktop file based KPluginInfo KPluginMetaData: Read the translated value for name and description The Name and Comment fields of the metadata should be translated since they will be shown to the user (e.g. in the plugin selection dialog) Add a unit test for KPluginMetaData Add KPluginMetaData::findPlugins() Add a unit test for KPluginMetaData::findPlugins() Introduce KPluginLoader::instantiatePlugins() and add a unit test This method allows easily instantiating all plugins in a given directory KPluginMetaData::pluginName() was changed to return the base name of the plugin file if no plugin name was set in the JSON metadata Diffs - autotests/CMakeLists.txt 75d12932b36fcfe4ae1d538176ef9f85f60f15dd autotests/jsonplugin.json d86fad49e5d074762d70282b3ace4bf3e6db58df autotests/kpluginloadertest.cpp c8225c02de3a64cae29d88954700dbc6f03ff1b0 autotests/kpluginmetadatatest.cpp PRE-CREATION src/lib/CMakeLists.txt 26eb5a1d4d56742a3395ba2645290bea15aee181 src/lib/plugin/kpluginloader.h 0b7a53d3b879cec1d755b849d9d8c640d251a379 src/lib/plugin/kpluginloader.cpp 9b3c5b6aec537b03b0d8341b33f6f4d7a76c8344 src/lib/plugin/kpluginmetadata.h PRE-CREATION src/lib/plugin/kpluginmetadata.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119079/diff/ Testing --- Added a unit test Should easily allow loading all plugins from a given directory without needing kbuildsycoca Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119079: Add utility function for loading all plugins from a given dir + easy accessor for metadata
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/#review62680 --- src/lib/plugin/kpluginloader.h https://git.reviewboard.kde.org/r/119079/#comment43448 What about plugins that do not have metadata? src/lib/plugin/kpluginmetadata.h https://git.reviewboard.kde.org/r/119079/#comment43449 If you don't put at least minimal documentation in, Doxygen will warn about undocumented methods. src/lib/plugin/kpluginmetadata.cpp https://git.reviewboard.kde.org/r/119079/#comment43450 I think this would be clearer as passing QFileInfo an empty string gives the CWD, which is not what we want. And put the comment one line up, so it applies to the if() statement, rather than the assignment. I see you haven't changed the KPluginLoader methods - have you looked to see if they are the ones that are likely to be useful? - Alex Merry On July 18, 2014, 1:34 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/ --- (Updated July 18, 2014, 1:34 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- This class simplifies reading the metadata from a qt plugin by providing type safe accessor functions for the standard plugininfo keys that are also used by the .desktop file based KPluginInfo KPluginMetaData: Read the translated value for name and description The Name and Comment fields of the metadata should be translated since they will be shown to the user (e.g. in the plugin selection dialog) Add a unit test for KPluginMetaData Add KPluginMetaData::findPlugins() Add a unit test for KPluginMetaData::findPlugins() Introduce KPluginLoader::instantiatePlugins() and add a unit test This method allows easily instantiating all plugins in a given directory KPluginMetaData::pluginName() was changed to return the base name of the plugin file if no plugin name was set in the JSON metadata Diffs - autotests/CMakeLists.txt 75d12932b36fcfe4ae1d538176ef9f85f60f15dd autotests/jsonplugin.json d86fad49e5d074762d70282b3ace4bf3e6db58df autotests/kpluginloadertest.cpp c8225c02de3a64cae29d88954700dbc6f03ff1b0 autotests/kpluginmetadatatest.cpp PRE-CREATION src/lib/CMakeLists.txt 26eb5a1d4d56742a3395ba2645290bea15aee181 src/lib/plugin/kpluginloader.h 0b7a53d3b879cec1d755b849d9d8c640d251a379 src/lib/plugin/kpluginloader.cpp 9b3c5b6aec537b03b0d8341b33f6f4d7a76c8344 src/lib/plugin/kpluginmetadata.h PRE-CREATION src/lib/plugin/kpluginmetadata.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119079/diff/ Testing --- Added a unit test Should easily allow loading all plugins from a given directory without needing kbuildsycoca Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119079: Add utility function for loading all plugins from a given dir + easy accessor for metadata
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/#review62699 --- Ship it! One more thing then I, at least, am happy for it to go in. src/lib/plugin/kpluginloader.h https://git.reviewboard.kde.org/r/119079/#comment43454 I think it's worth explicitly saying that the plugins will not necessarily have JSON metadata, and in fact are not even necessarily loadable via QPluginLoader (although they will generally be loadable by QLibrary). - Alex Merry On July 19, 2014, 1:15 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119079/ --- (Updated July 19, 2014, 1:15 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- This class simplifies reading the metadata from a qt plugin by providing type safe accessor functions for the standard plugininfo keys that are also used by the .desktop file based KPluginInfo KPluginMetaData: Read the translated value for name and description The Name and Comment fields of the metadata should be translated since they will be shown to the user (e.g. in the plugin selection dialog) Add a unit test for KPluginMetaData Add KPluginMetaData::findPlugins() Add a unit test for KPluginMetaData::findPlugins() Introduce KPluginLoader::instantiatePlugins() and add a unit test This method allows easily instantiating all plugins in a given directory KPluginMetaData::pluginName() was changed to return the base name of the plugin file if no plugin name was set in the JSON metadata Diffs - src/lib/plugin/kpluginmetadata.cpp PRE-CREATION autotests/kpluginmetadatatest.cpp PRE-CREATION src/lib/CMakeLists.txt 26eb5a1d4d56742a3395ba2645290bea15aee181 src/lib/plugin/kpluginloader.h 0b7a53d3b879cec1d755b849d9d8c640d251a379 src/lib/plugin/kpluginloader.cpp 9b3c5b6aec537b03b0d8341b33f6f4d7a76c8344 src/lib/plugin/kpluginmetadata.h PRE-CREATION autotests/CMakeLists.txt 75d12932b36fcfe4ae1d538176ef9f85f60f15dd autotests/jsonplugin.json d86fad49e5d074762d70282b3ace4bf3e6db58df autotests/kpluginloadertest.cpp c8225c02de3a64cae29d88954700dbc6f03ff1b0 Diff: https://git.reviewboard.kde.org/r/119079/diff/ Testing --- Added a unit test Should easily allow loading all plugins from a given directory without needing kbuildsycoca Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119331: Use CMAKE_INSTALL_FULL_LIBEXECDIR_KF5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119331/#review62636 --- Ship it! Ship It! - Alex Merry On July 17, 2014, 11:01 a.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119331/ --- (Updated July 17, 2014, 11:01 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Resolves the problem of passing relative vs. absolute KF5_LIBEXEC_INSTALL_DIR/LIBEXEC_INSTALL_DIR. Diffs - autotests/CMakeLists.txt a9075e3 autotests/krununittest.cpp 9bbceb2 src/core/config-kiocore.h.cmake 6041c9d src/core/desktopexecparser.cpp be62791 src/core/slave.cpp c842f01 src/ioslaves/http/config-kioslave-http.h.cmake 3f313e9 src/ioslaves/http/http.cpp 55a19f4 src/kpac/config-kpac.h.cmake 1e73657 src/kpac/discovery.cpp d108fee Diff: https://git.reviewboard.kde.org/r/119331/diff/ Testing --- Builds. Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On July 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). Cristian Oneț wrote: I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. Alex Merry wrote: I can't reproduce this (and, additionally, the [CI system](http://build.kde.org) does a completely clean build and install every time). Can you post the output of `cmake --version`, please? Andreas Xavier wrote: Entirely my mistake. All of frameworks compiled with no changes. Thank you for taking the time to look at this. I am now running with : cmake version 3.0.20140712-g2eadd1 One nameless Konsole tab over I was running with : cmake version 2.8.12.2 It might be worth noting that with the changes in this patch cmake 2.8.12.2 was able to compile frameworks all the way to KHTML with no complaints. Clearly, I am a cmake novice, but if this patch doesn't break the build with 3.0 and it makes it possible to build it with 2.8.12.2 it might make the overall build more robust. Once again thanks for your time. Michael Pyne wrote: I'll look into whether kdesrc-build is failing on the first-ever-build use case, there's a recently-opened bug about it (bug 337446) so it's very possible. Cristian Oneț wrote: Now this is starting to make sense to me, altough kdesrc-build using extragear/utils/kdesrc-build/kf5-qt5-build-include builds cmake-git maybe it was using up cmake from my system which is 2.8.12.2 and was causing this problem. Using cmake 2.8.12.2 should be fine. I'll have to do a build with that version and check it still works. It is possible that using a mix of different CMake versions could cause issues, though. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On July 15, 2014, 9:36 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 15, 2014, 9:36 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119291: Use an input type=search for the search box.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119291/#review62407 --- Ship it! Ship It! - Alex Merry On July 15, 2014, 8:29 a.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119291/ --- (Updated July 15, 2014, 8:29 a.m.) Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- Use an `input` with a proper type and a placeholder: this allows the platform to style it differently if necessary, and the use of the placeholder attribute lets us to get rid of the custom script to clean up its value. Diffs - src/kapidox/data/templates/base.html 10f8aa6dae14e0ad6e649bdb28fcba81b7d39341 Diff: https://git.reviewboard.kde.org/r/119291/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On July 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). Cristian Oneț wrote: I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. I can't reproduce this (and, additionally, the [CI system](http://build.kde.org) does a completely clean build and install every time). Can you post the output of `cmake --version`, please? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On July 14, 2014, 6 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 14, 2014, 6 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119300: extra-cmake-modules: Fix using the same variable for camelCase and REQUIRED_HEADERS causes problems.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119300/#review62408
---
I don't think this should be necessary. Let's keep discussion on
https://git.reviewboard.kde.org/r/119275/ for now, though.
modules/ECMGenerateHeaders.cmake
https://git.reviewboard.kde.org/r/119300/#comment43339
This test doesn't do what you think it does. Rather than comparing
${camelcase_headers_var} with ${EGH_REQUIRED_HEADERS}, it will compare
${${camelcase_headers_var}} with ${${EGH_REQUIRED_HEADERS}}. Unintuitive, I
know.
- Alex Merry
On July 15, 2014, 1:38 p.m., Andreas Xavier wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119300/
---
(Updated July 15, 2014, 1:38 p.m.)
Review request for KDE Frameworks, Alex Merry and Michael Pyne.
Repository: extra-cmake-modules
Description
---
First, this is my first attempt at building frameworks, so the problem is
most likely on my end. However, following the instructions at
https://community.kde.org/Frameworks/Building/Details I found this problem.
Using the same variable name for camelCase var1 and REQUIRED_HEADERS var2 in
the new ecm_generate_headers() syntax, when it is called more than once only
exports the headers from the first invokation of ecm_generate_headers(),
where var1 and var2 are defined as follows.
ecm_generate_headers(var1
...
REQUIRED_HEADERS var2
)
It doesn't show up in existing builds because cmake doesn't delete old header
files, so all accumulated headers remain in the build.
There are some projects in kde-frameworks that use the same variable name for
var1 and var2, for example kcoreaddons.
Steps to Replicate Using KCoreAddons as an example:
1. Delete the existing header files for KCoreAddons and the existing build
files.
rm -r $KF5/KcoreAddons
rm -r your kcoreaddons/build directory
2. Re-build kcoreaddons from a new build dir
cmake -DCMAKE_INSTALL_PREFIX=$KF5 ..
3. Check in $KF5/KcoreAddons and there should only be these headers:
KAboutData kaboutdata.h kcoreaddons_export.h
Problem:
If the same variable name is used for both var1 and var2 they overwrite each
other at the PARENT level and the result is that only one invocation of
ecm_generate_headers sticks.
Solution:
I assume that the CMakeList writer is intentionally using the same variable
name for var1 and var2 and wants to accumulate both the camelCase and the
lowercase.h names on the same variable. If the variable names are the same
if accumulates both the camelCase list and the lowercase.h list on the single
variable.
If my assumption is incorrect and var1 and var2 must be different to separate
the camelCase and lowercase.h lists, then a new patch should be submitted to
generate either a warning or an error message when var1 and var2 are the same
name.
This review is linked with https://git.reviewboard.kde.org/r/119275/. If
this review is accepted then https://git.reviewboard.kde.org/r/119275/ is
unecessary.
Diffs
-
modules/ECMGenerateHeaders.cmake bac5086
Diff: https://git.reviewboard.kde.org/r/119300/diff/
Testing
---
make test
Compiled kcoreaddons, checked that all the headers were exported. Then
compiled kauth and checked that it found the exported headers.
Thanks,
Andreas Xavier
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119289: Remove api_searchbox.html.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119289/#review62414 --- Ship it! Huh, guess I missed that :-) - Alex Merry On July 15, 2014, 6:21 p.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119289/ --- (Updated July 15, 2014, 6:21 p.m.) Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- It's been part of base.html itself since 07b35f28a334584114be89a5f293cd39ff003cd6. Diffs - src/kapidox/data/api_searchbox.html 9fd1ade894680f7f41ca498d0bc4a8dd684c0e98 Diff: https://git.reviewboard.kde.org/r/119289/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119258: kcoreaddons: Fix ecm_generate_headers don't accumulate and only kaboutdata.h is exported.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119258/#review62247 --- Passing the same variable to ecm_generate_headers for both the CamelCase headers and the REQUIRED_HEADERS should work just fine. I suspect you have a build issue at your end. Can you try removing both your build and install directories, and building again? - Alex Merry On July 13, 2014, 9:54 a.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119258/ --- (Updated July 13, 2014, 9:54 a.m.) Review request for KDE Frameworks and Michael Pyne. Repository: kcoreaddons Description --- While bulding frameworks following https://community.kde.org/Frameworks/Building/Details, I was unable to use kcoreaddons in anything because the only header that was exported was kaboutdata. I think that the problem is that ecm_generate_headers will not accumulate the list of required_headers on the same variable as the initial variables. In the patch I created a new variable KCoreAddons_HEADERSout to fix this problem. I may also have incorrectly configured ecm, in which case this patch will not be useful. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119258/diff/ Testing --- Compiled and then linked with kauth, which previously couldn't find kjob.h. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119259: kservice: Fix typo in autotests/nsaplugin.cpp nsapluginfa to nsaplugin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119259/#review62250 --- Ship it! A note for the future: your Testing done field suggests you haven't tried running the tests. Running make test to check your changes is always a good idea. I applied your patch to my checkout, and they ran fine, though, so you can ship it. - Alex Merry On July 13, 2014, 11:19 a.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119259/ --- (Updated July 13, 2014, 11:19 a.m.) Review request for KDE Frameworks, Alex Merry and David Faure. Repository: kservice Description --- When compiling framworks following https://community.kde.org/Frameworks/Building/Details, I was unable to compile kservice. I think that the problem was the typo nsapluginfa for nsaplugin. This patch fixes that. Diffs - autotests/nsaplugin.cpp 48aeaf4 Diff: https://git.reviewboard.kde.org/r/119259/diff/ Testing --- Compiled. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119249: Discuss fixes for building kdelibs4support on Windows using msvc 2013
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119249/#review62198 --- Generally seems sensible, apart from the things I noted below. autotests/kdebug_qcoreapptest.cpp https://git.reviewboard.kde.org/r/119249/#comment43245 See later comment about fixing this in KCoreAddons. src/kdeui/kapplication.cpp https://git.reviewboard.kde.org/r/119249/#comment43243 Ooh, that's a nasty trap. We should fix this in KCoreAddons, not work around it here. Possibly by using Q_DECL_IMPORT in the forward declaration. tests/kprintpreview_test.cpp https://git.reviewboard.kde.org/r/119249/#comment43244 This should have a comment, otherwise someone well-meaning will undo your change. - Alex Merry On July 12, 2014, 5:02 p.m., Cristian Oneț wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119249/ --- (Updated July 12, 2014, 5:02 p.m.) Review request for KDE Frameworks and kdewin. Repository: kdelibs4support Description --- Let me begin by saying that this review request was started to highlight current build issues using MSVC 2013 in order to get some feedback about the best way to fix them. That's why I'll explain each issue that was fixed as a comment in the diff (I'll add the comments after I'll publish the review request, it's the only way I can add them). Here is a list of issues: 1. r:\include\QtCore/qlist.h(300) : error C2678: binary '==' : no operator found which takes a left-hand operand of type 'const KNetwork::KResolverEntry' 2. error C2375: 'KCrash::defaultCrashHandler' : redefinition; different linkage 3. error C2487: 'identifier' : member of dll interface class may not be declared with dll interface 4. QStringLiteral does not work when using string concatenation https://bugreports.qt-project.org/browse/QTBUG-28885 5. KEditListBox::CustomEditor linker error caused by missing export Diffs - autotests/kdebug_qcoreapptest.cpp 0bfe5d6f911d3ec6bd6f919a5d666b4eab63e2e8 src/kdecore/k3resolver.h e956c6f05e0fd821bb41dc63bc2f0933b818aafd src/kdemacros.h.cmake c406623401a5e47ecfd45fe135c9f2019f2abe04 src/kdeui/kapplication.cpp 6ffaf6e2c383e781e498b6d2b777366d4c4a53fe src/kdeui/keditlistbox.h e19474b3bb03587421f0afbc20346a8d359417b1 src/kio/netaccess.h 91f3aba055cceddf10a7c48c710356ce04d9939f tests/kprintpreview_test.cpp 79cac037ab38bce89b97e4ede58eb58d821b25f3 Diff: https://git.reviewboard.kde.org/r/119249/diff/ Testing --- Build with msvc2013 on Windows and gcc-4.8.3 on Linux. Thanks, Cristian Oneț ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119251: Fix a conflict with KCrash where the function is declared as exported
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119251/#review62209 --- src/lib/kaboutdata.h https://git.reviewboard.kde.org/r/119251/#comment43252 Normally, it goes before the return type. It presumably still works where you put it, but it looks very odd. - Alex Merry On July 12, 2014, 7:02 p.m., Cristian Oneț wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119251/ --- (Updated July 12, 2014, 7:02 p.m.) Review request for KDE Frameworks and kdewin. Repository: kcoreaddons Description --- On MSVC this could cause a compile error if the compiler would see the forward declaration first because it would consider it a redefinition with different linkage. Found while cbuilding KDELibs4Support on Windows using MSVC, see https://git.reviewboard.kde.org/r/119249/ Diffs - src/lib/kaboutdata.h f0f3e12bf353019d86f582416cec90390de00f88 Diff: https://git.reviewboard.kde.org/r/119251/diff/ Testing --- Build with MSVC 2013 on Windows and gcc-4.8.3 on Linux. Thanks, Cristian Oneț ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119249: Discuss fixes for building kdelibs4support on Windows using msvc 2013
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119249/#review62211 --- Ship it! Please make it one commit for each issue you listed (except issue 2, of course, since that's covered in the other review request). - Alex Merry On July 12, 2014, 7:15 p.m., Cristian Oneț wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119249/ --- (Updated July 12, 2014, 7:15 p.m.) Review request for KDE Frameworks and kdewin. Repository: kdelibs4support Description --- Let me begin by saying that this review request was started to highlight current build issues using MSVC 2013 in order to get some feedback about the best way to fix them. That's why I'll explain each issue that was fixed as a comment in the diff (I'll add the comments after I'll publish the review request, it's the only way I can add them). Here is a list of issues: 1. r:\include\QtCore/qlist.h(300) : error C2678: binary '==' : no operator found which takes a left-hand operand of type 'const KNetwork::KResolverEntry' 2. error C2375: 'KCrash::defaultCrashHandler' : redefinition; different linkage 3. error C2487: 'identifier' : member of dll interface class may not be declared with dll interface 4. QStringLiteral does not work when using string concatenation https://bugreports.qt-project.org/browse/QTBUG-28885 5. KEditListBox::CustomEditor linker error caused by missing export Diffs - src/kdecore/k3resolver.h e956c6f05e0fd821bb41dc63bc2f0933b818aafd src/kdemacros.h.cmake c406623401a5e47ecfd45fe135c9f2019f2abe04 src/kdeui/keditlistbox.h e19474b3bb03587421f0afbc20346a8d359417b1 src/kio/netaccess.h 91f3aba055cceddf10a7c48c710356ce04d9939f tests/kprintpreview_test.cpp 79cac037ab38bce89b97e4ede58eb58d821b25f3 Diff: https://git.reviewboard.kde.org/r/119249/diff/ Testing --- Build with msvc2013 on Windows and gcc-4.8.3 on Linux. Thanks, Cristian Oneț ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119198: Don't search default paths when finding lconvert.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119198/#review62214 --- Ship it! Yep, seems sensible to me. We definitely want the lconvert that matches the lrelease we're using. - Alex Merry On July 9, 2014, 2:12 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119198/ --- (Updated July 9, 2014, 2:12 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Otherwise, if lconvert exists in normal system paths (eg. /usr/bin) that one will be used instead of the one alongside Qt5::lrelease. This could cause Qt4 lconvert to be incorrectly used on some systems. Diffs - modules/ECMPoQmTools.cmake 3ce695817cb3da5ec9eebec86e632438c5941ee6 modules/ECMCreateQmFromPoFiles.cmake 4a31a93e7900780dc5b9424b148f19f5c22061af Diff: https://git.reviewboard.kde.org/r/119198/diff/ Testing --- When building solid-5.0.0, Qt5 lconvert from lrelease_path is now correctly used. Previously Qt4 version was used as it exists in /usr/bin. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119230: Import FindOpenGLES from kwin.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119230/#review62129 --- I second Martin's comment. I suggest looking at http://www.cmake.org/cmake/help/v3.0/manual/cmake-developer.7.html#modules and http://api.kde.org/ecm/manual/ecm-developer.7.html - Alex Merry On July 11, 2014, 10:56 a.m., Heiko Becker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119230/ --- (Updated July 11, 2014, 10:56 a.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Import FindOpenGLES from kwin. Besides kwin it's also needed for kinfocenter and plasma-desktop. This is a follow-up of https://git.reviewboard.kde.org/r/119227/. Diffs - find-modules/FindOpenGLES.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119230/diff/ Testing --- Thanks, Heiko Becker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119111: Make FindGettext compatible with the one provided by CMake
On July 8, 2014, 9:51 p.m., Albert Astals Cid wrote: I know i'm late, but if CMake provides one, can't we just kill ours? The CMake one isn't quite compatible, so I wanted to keep this to make porting a little easier. Note that this is in kdelibs4support, and therefore deprecated. As soon as you stop your project finding kdelibs4support, it will no longer be available, and you will be using the CMake version. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119111/#review61935 --- On July 7, 2014, 5:27 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119111/ --- (Updated July 7, 2014, 5:27 p.m.) Review request for KDE Frameworks and Jonathan Riddell. Repository: kdelibs4support Description --- This version will accept the old GETTEXT_PROCESS_PO_FILES() syntax (no PO_FILES argument), but will also accept the new syntax required by CMake's version of this file. It will also warn when PO_FILES is not given. Diffs - cmake/modules/FindGettext.cmake 91e88f7e00ac52539066e71eeffb7df6c2148196 Diff: https://git.reviewboard.kde.org/r/119111/diff/ Testing --- None whatsoever: Jonathan, you know where this isssue has been seen, can you test? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Update Meeting Minutes 2014-w28
On Tuesday 08 July 2014 17:30:19 Kevin Ottens wrote: * he's also like to see the android port progress, in particular he got no answer about getting a file in ECM to that effect; Sorry, life's a bit crazy at the mo. Will try to look at soon. Although I'd appreciate the input of anyone else with android development experience. My plans: * more tests for ECM * work on rewriting/cleaning up image format handlers * get the remaining KImageIO methods into QImageReader/Writer I also second the better docs call. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119111: Make FindGettext compatible with the one provided by CMake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119111/ --- (Updated July 7, 2014, 5:27 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Jonathan Riddell. Repository: kdelibs4support Description --- This version will accept the old GETTEXT_PROCESS_PO_FILES() syntax (no PO_FILES argument), but will also accept the new syntax required by CMake's version of this file. It will also warn when PO_FILES is not given. Diffs - cmake/modules/FindGettext.cmake 91e88f7e00ac52539066e71eeffb7df6c2148196 Diff: https://git.reviewboard.kde.org/r/119111/diff/ Testing --- None whatsoever: Jonathan, you know where this isssue has been seen, can you test? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119142: KXMLGUI: load .rc files from DATADIR/kxmlgui5/component_name/file.rc
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119142/#review61693 --- A little concerned that there are no documentation changes as part of this patch. Is this stuff noted in the apidocs? Also, I assume that not changing the behaviour of setXMLFile is a deliberate decision. What's the reasoning? That should probably be noted in a comment or in the apidocs. - Alex Merry On July 6, 2014, 9:47 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119142/ --- (Updated July 6, 2014, 9:47 a.m.) Review request for KDE Frameworks, Alex Merry and Kevin Ottens. Repository: kxmlgui Description --- KXMLGUI: load .rc files from DATADIR/kxmlgui5/component_name/file.rc The old files (DATADIR/component_name/file.rc) can still be loaded, and a runtime warning is shown in such a case, telling people to install into KXMLGUI_INSTALL_DIR (ECM patch posted to k-f-d) This keeps DATADIR slightly cleaner, getting rid of many application subdirs that would just contain a rc file. Some people seem to care about the cleanliness of their DATADIR :-) Diffs - src/kedittoolbar.cpp 32512e9caccdbee1b07ff791fbecdcc817ca5726 src/kxmlguiclient.cpp f7c90b261645dfe2efeb2edbc8f32cb134fc9b7d src/kxmlguifactory.cpp aa657bcc24a949f4350975609f19fae8b6c9a4f0 Diff: https://git.reviewboard.kde.org/r/119142/diff/ Testing --- Running kwrite unmodified - works, with runtime warning Porting kwrite to KXMLGUI_INSTALL_DIR - works, runtime warning is gone I also did the same with konsole, special case since it called setXMLFile(konsole/partui.rc), had to be ported to setComponentName(konsole) + setXMLFile(partui.rc) while porting to KXMLGUI_INSTALL_DIR. This patch is still SC/BC, it's just a porting trap -- that affects none of the other existing kf5-based apps though. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119142: KXMLGUI: load .rc files from DATADIR/kxmlgui5/component_name/file.rc
On July 6, 2014, 12:52 p.m., Alex Merry wrote: A little concerned that there are no documentation changes as part of this patch. Is this stuff noted in the apidocs? Also, I assume that not changing the behaviour of setXMLFile is a deliberate decision. What's the reasoning? That should probably be noted in a comment or in the apidocs. David Faure wrote: You're right that this could be documented in the kxmlgui apidocs. Note that (AFAICS) the old install location wasn't documented either. I don't understand your second paragraph though. This patch DOES change the behaviour of setXMLFile: it looks in kxmlgui5/ first. Oh, yes, so it does. I was assuming it didn't from not properly reading your Testing Done section about porting konsole. Now I see why the old call wouldn't work with the new install location. Forget that para, then (well, except that the porting trap should be noted in the docs, of course). - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119142/#review61693 --- On July 6, 2014, 9:47 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119142/ --- (Updated July 6, 2014, 9:47 a.m.) Review request for KDE Frameworks, Alex Merry and Kevin Ottens. Repository: kxmlgui Description --- KXMLGUI: load .rc files from DATADIR/kxmlgui5/component_name/file.rc The old files (DATADIR/component_name/file.rc) can still be loaded, and a runtime warning is shown in such a case, telling people to install into KXMLGUI_INSTALL_DIR (ECM patch posted to k-f-d) This keeps DATADIR slightly cleaner, getting rid of many application subdirs that would just contain a rc file. Some people seem to care about the cleanliness of their DATADIR :-) Diffs - src/kedittoolbar.cpp 32512e9caccdbee1b07ff791fbecdcc817ca5726 src/kxmlguiclient.cpp f7c90b261645dfe2efeb2edbc8f32cb134fc9b7d src/kxmlguifactory.cpp aa657bcc24a949f4350975609f19fae8b6c9a4f0 Diff: https://git.reviewboard.kde.org/r/119142/diff/ Testing --- Running kwrite unmodified - works, with runtime warning Porting kwrite to KXMLGUI_INSTALL_DIR - works, runtime warning is gone I also did the same with konsole, special case since it called setXMLFile(konsole/partui.rc), had to be ported to setComponentName(konsole) + setXMLFile(partui.rc) while porting to KXMLGUI_INSTALL_DIR. This patch is still SC/BC, it's just a porting trap -- that affects none of the other existing kf5-based apps though. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119142: KXMLGUI: load .rc files from DATADIR/kxmlgui5/component_name/file.rc
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119142/#review61739 --- Ship it! Ship It! - Alex Merry On July 6, 2014, 3:32 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119142/ --- (Updated July 6, 2014, 3:32 p.m.) Review request for KDE Frameworks, Alex Merry and Kevin Ottens. Repository: kxmlgui Description --- KXMLGUI: load .rc files from DATADIR/kxmlgui5/component_name/file.rc The old files (DATADIR/component_name/file.rc) can still be loaded, and a runtime warning is shown in such a case, telling people to install into KXMLGUI_INSTALL_DIR (ECM patch posted to k-f-d) This keeps DATADIR slightly cleaner, getting rid of many application subdirs that would just contain a rc file. Some people seem to care about the cleanliness of their DATADIR :-) REVIEW: 119142 Diffs - src/kedittoolbar.cpp 32512e9caccdbee1b07ff791fbecdcc817ca5726 src/kxmlguiclient.h 2331b6934f99528c5d266c92d78a8affaccad2a0 src/kxmlguiclient.cpp f7c90b261645dfe2efeb2edbc8f32cb134fc9b7d src/kxmlguifactory.cpp aa657bcc24a949f4350975609f19fae8b6c9a4f0 Diff: https://git.reviewboard.kde.org/r/119142/diff/ Testing --- Running kwrite unmodified - works, with runtime warning Porting kwrite to KXMLGUI_INSTALL_DIR - works, runtime warning is gone I also did the same with konsole, special case since it called setXMLFile(konsole/partui.rc), had to be ported to setComponentName(konsole) + setXMLFile(partui.rc) while porting to KXMLGUI_INSTALL_DIR. This patch is still SC/BC, it's just a porting trap -- that affects none of the other existing kf5-based apps though. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Deprecation warnings not showing on CMake 3
On Tuesday 03 June 2014 23:01:23 Anuj Pahuja wrote: Hi, I am building libkdegames on KF5/Qt5 using CMake 3 but I'm not getting deprecation warnings for KDELibs4Support classes (KDialog, KFileDialog etc.). However, I get all the deprecation warnings when I use CMake 2.8. Also, functions like KGlobal::dirs() do give a warning on CMake 3. Is it some change in the ecm-modules or KDELibs4Support? Or a change in cmake itself? Well, libkdegames doesn't even build with Frameworks 5.0.0, but I manged to get far enough with CMake 3.0.0 (Arch Linux packages) to see warnings about KUrl being deprecated. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: OSX/MacPorts KDE CI System: Asking for trouble due to installations outside the expected $DATA_INSTALL_DIR/kf5 directory for khtml and katepart5
On Saturday 05 July 2014 18:31:20 Marko Käning wrote: Hi David and all, Which proves my point: .rc files don't go into the kf5 subfolder. I was pondering this again just now and came to the conclusion that all the KF5/KDE frameworks and apps should install everything into some subdir of the system's directory like /Library/Application Support/KF5/ by which KF5 wouldn't clutter /Library/Application Support with a ton of KF5-related files. This is what other companies do as well on OSX, see e.g. the content on the CI system: --- $ ls -1 /Library/Application\ Support App Store Apple CrashReporter ProApps Script Editor com.apple.TCC iLifeMediaBrowser kf5 If we want to follow this pattern, I think we want to use KDE as a subdirectory name, and then have kf5-specific stuff under KDE/kf5. This would apply to any software that is *produced* by KDE, but not to software that merely uses KF5. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119079: Add utility function for loading all plugins from a given dir + easy accessor for metadata
On July 5, 2014, 9:33 a.m., David Faure wrote:
src/lib/plugin/kpluginmetadata.h, lines 204-205
https://git.reviewboard.kde.org/r/119079/diff/3/?file=286685#file286685line204
Make member vars private.
It's not like deriving from a value class makes any sense anyway, and
this makes future changes possible (as long as the object size doesn't
change).
Alexander Richardson wrote:
I thought applications that have extra metadata keys could just extend
this class to provide a getter for that.
class FooPluginMetaData : public KPluginMetaData {
public:
using KPluginMetaData::KPluginMetaData;
QString fooBar() {
return m_metadata[X-Foo-BarInfo];
}
}
But I can make them private anyway if you think this is not a valid
use-case
Well, you can have a public or protected method instead, like
QString value(const QString value) { return m_metadata.value(value); }
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119079/#review61635
---
On July 5, 2014, 3:20 p.m., Alexander Richardson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119079/
---
(Updated July 5, 2014, 3:20 p.m.)
Review request for KDE Frameworks.
Repository: kcoreaddons
Description
---
This class simplifies reading the metadata from a qt plugin by providing
type safe accessor functions for the standard plugininfo keys that are
also used by the .desktop file based KPluginInfo
KPluginMetaData: Read the translated value for name and description
The Name and Comment fields of the metadata should be translated
since they will be shown to the user (e.g. in the plugin selection
dialog)
Add a unit test for KPluginMetaData
Add KPluginMetaData::findPlugins()
Add a unit test for KPluginMetaData::findPlugins()
Introduce KPluginLoader::instantiatePlugins() and add a unit test
This method allows easily instantiating all plugins in a given directory
KPluginMetaData::pluginName() was changed to return the base name of the
plugin file if no plugin name was set in the JSON metadata
Diffs
-
autotests/CMakeLists.txt 75d12932b36fcfe4ae1d538176ef9f85f60f15dd
autotests/kpluginloadertest.cpp c8225c02de3a64cae29d88954700dbc6f03ff1b0
autotests/kpluginmetadatatest.cpp PRE-CREATION
src/lib/CMakeLists.txt 26eb5a1d4d56742a3395ba2645290bea15aee181
src/lib/plugin/kpluginloader.h 0b7a53d3b879cec1d755b849d9d8c640d251a379
src/lib/plugin/kpluginloader.cpp 9b3c5b6aec537b03b0d8341b33f6f4d7a76c8344
src/lib/plugin/kpluginmetadata.h PRE-CREATION
src/lib/plugin/kpluginmetadata.cpp PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/119079/diff/
Testing
---
Added a unit test
Should easily allow loading all plugins from a given directory without
needing kbuildsycoca
Thanks,
Alexander Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: kdelibs4support FindGettext
On 03/07/14 22:06, Jonathan Riddell wrote: kdelibs4support ships an old copy of FindGettext.cmake. This is incompatible with the version shipping in cmake and causes some compile failures. Can I remove it? The interfaces of both files look like they should be the same. Do you have any info about the compile failures? Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: kdelibs4support FindGettext
On 04/07/14 10:07, Jonathan Riddell wrote: On Fri, Jul 04, 2014 at 10:01:46AM +0100, Alex Merry wrote: On 03/07/14 22:06, Jonathan Riddell wrote: kdelibs4support ships an old copy of FindGettext.cmake. This is incompatible with the version shipping in cmake and causes some compile failures. Can I remove it? The interfaces of both files look like they should be the same. Do you have any info about the compile failures? http://techbase.kde.org/Development/ECM_SourceIncompatChanges#FindGettext.cmake We use the version from CMake now. The syntax of GETTEXT_PROCESS_PO_FILES() changed a bit, it now needs the additional keyword PO_FILES. If you add the PO_FILES keyword then the old FindGettext.cmake breaks on GETTEXT_PROCESS_PO_FILES() So we could make the old one accept but ignore the PO_FILES keyword argument. In fact, we could make it warn when you don't pass it (as it will stop working as soon as you remove kdelibs4support from the list of packages to find). Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119111: Make FindGettext compatible with the one provided by CMake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119111/ --- Review request for Build System, KDE Frameworks and Jonathan Riddell. Repository: kdelibs4support Description --- This version will accept the old GETTEXT_PROCESS_PO_FILES() syntax (no PO_FILES argument), but will also accept the new syntax required by CMake's version of this file. It will also warn when PO_FILES is not given. Diffs - cmake/modules/FindGettext.cmake 91e88f7e00ac52539066e71eeffb7df6c2148196 Diff: https://git.reviewboard.kde.org/r/119111/diff/ Testing --- None whatsoever: Jonathan, you know where this isssue has been seen, can you test? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: kdelibs4support FindGettext
On 04/07/14 10:41, Alex Merry wrote: On 04/07/14 10:07, Jonathan Riddell wrote: On Fri, Jul 04, 2014 at 10:01:46AM +0100, Alex Merry wrote: On 03/07/14 22:06, Jonathan Riddell wrote: kdelibs4support ships an old copy of FindGettext.cmake. This is incompatible with the version shipping in cmake and causes some compile failures. Can I remove it? The interfaces of both files look like they should be the same. Do you have any info about the compile failures? http://techbase.kde.org/Development/ECM_SourceIncompatChanges#FindGettext.cmake We use the version from CMake now. The syntax of GETTEXT_PROCESS_PO_FILES() changed a bit, it now needs the additional keyword PO_FILES. If you add the PO_FILES keyword then the old FindGettext.cmake breaks on GETTEXT_PROCESS_PO_FILES() So we could make the old one accept but ignore the PO_FILES keyword argument. In fact, we could make it warn when you don't pass it (as it will stop working as soon as you remove kdelibs4support from the list of packages to find). See https://git.reviewboard.kde.org/r/119111/ ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119111: Make FindGettext compatible with the one provided by CMake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119111/ --- (Updated July 4, 2014, 1:05 p.m.) Review request for KDE Frameworks and Jonathan Riddell. Changes --- Fixed list(REMOVE_AT) call. Repository: kdelibs4support Description --- This version will accept the old GETTEXT_PROCESS_PO_FILES() syntax (no PO_FILES argument), but will also accept the new syntax required by CMake's version of this file. It will also warn when PO_FILES is not given. Diffs (updated) - cmake/modules/FindGettext.cmake 91e88f7e00ac52539066e71eeffb7df6c2148196 Diff: https://git.reviewboard.kde.org/r/119111/diff/ Testing --- None whatsoever: Jonathan, you know where this isssue has been seen, can you test? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KIO: thread safety commit broke Get hot new stuff
On 04/07/14 17:40, Marco Martin wrote: HI all, I recently noticed that the two GHNS windows in Plasma 5 very recently freeze the whole process as soon the dialog opens I tried to bisect things and the problem is definitely in the commit 8cb050ea61dfb195fee0293c7c0eb87cb0a90654 in the KIO framework KIO thread-safety: protect concurrent access to KProtocolManager methods Right now frameworks are tagged, but either kio or knewstuff framework are broken. And we need the dialog working in the Plasma 5 release. Sooo, how do we proceed? Is this fixed by https://git.reviewboard.kde.org/r/119110/ ? I noticed you already commented on that RR. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Added current release schedule to techbase
On 27/06/14 11:24, Mario Fux wrote: Morning As I was asked recently why the release schedule of KDE Frameworks 5 is not (yet) on techbase.kde.org I took the liberty to change this: http://techbase.kde.org/Schedules/Frameworks It's link to the main page as well: http://techbase.kde.org/Schedules The content is taken from: http://community.kde.org/Frameworks/Epics I hope that's ok for your guys. If you tell me I'll remove it from the community.kde.org wiki and set a link. And what about the future releases of KF5? Any decisions made? At least Plasma 5 has a tentative schedule now: http://techbase.kde.org/Schedules/Plasma_5 I think we should have this in a single place, or they'll get out of sync. I'm not sure where that place should be, though. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[extra-cmake-modules] modules: ECMAddTests: make NAME_PREFIX only apply to the test, not the target
Git commit f6f1e8c7b8321389fe8268fc25254bd512a6f399 by Alex Merry.
Committed on 15/06/2014 at 16:21.
Pushed by alexmerry into branch 'master'.
ECMAddTests: make NAME_PREFIX only apply to the test, not the target
David Faure and Patrick Spendrin have convinced me that NAME_PREFIX
should be informational only, and not be used to prevent clashes, since
it makes things confusing when you run tests manually.
This is a SIC change (although in practice only kio and kconfig should
be affected, currently).
REVIEW: 118768
CCMAIL: kde-frameworks-devel@kde.org
CCMAIL: kde-buildsys...@kde.org
M +21 -14 modules/ECMAddTests.cmake
http://commits.kde.org/extra-cmake-modules/f6f1e8c7b8321389fe8268fc25254bd512a6f399
diff --git a/modules/ECMAddTests.cmake b/modules/ECMAddTests.cmake
index 6cc20b6..ed1e0f6 100644
--- a/modules/ECMAddTests.cmake
+++ b/modules/ECMAddTests.cmake
@@ -12,12 +12,19 @@
# [GUI])
#
# Add a new unit test using the passed source files. The parameter TEST_NAME is
-# used to set the name of the resulting test. It may be omitted if there is
-# exactly one source file. In that case the name of the source file (without
the
-# file extension) will be used as the test name. If the flag GUI is passed the
-# test binary will be a GUI executable, otherwise the resulting binary will be
a
-# console application. The test will be linked against the libraries and/or
-# targets passed to LINK_LIBRARIES.
+# used to set the name of the resulting test, and the target built for and run
+# by the test. It may be omitted if there is exactly one source file. In that
+# case the name of the source file (without the file extension) will be used as
+# the test name.
+#
+# If NAME_PREFIX is given, this prefix will be prepended to the test name, but
+# not the target name. As a result, it will not prevent clashes between tests
+# with the same name in different parts of the project, but it can be used to
+# give an indication of where to look for a failing test.
+#
+# If the flag GUI is passed the test binary will be a GUI executable, otherwise
+# the resulting binary will be a console application. The test will be linked
+# against the libraries and/or targets passed to LINK_LIBRARIES.
#
#
# ::
@@ -54,23 +61,23 @@ function(ecm_add_test)
set(_sources ${ECM_ADD_TEST_UNPARSED_ARGUMENTS})
list(LENGTH _sources _sourceCount)
if(ECM_ADD_TEST_TEST_NAME)
-set(_testname ${ECM_ADD_TEST_TEST_NAME})
+set(_targetname ${ECM_ADD_TEST_TEST_NAME})
elseif(${_sourceCount} EQUAL 1)
#use the source file name without extension as the testname
-get_filename_component(_testname ${_sources} NAME_WE)
+get_filename_component(_targetname ${_sources} NAME_WE)
else()
#more than one source file passed, but no test name given - error
message(FATAL_ERROR ecm_add_test() called with multiple source files but
without setting \TEST_NAME\)
endif()
- set(_testname ${ECM_ADD_TEST_NAME_PREFIX}${_testname})
- add_executable(${_testname} ${_sources})
+ set(_testname ${ECM_ADD_TEST_NAME_PREFIX}${_targetname})
+ add_executable(${_targetname} ${_sources})
if(NOT ECM_ADD_TEST_GUI)
-ecm_mark_nongui_executable(${_testname})
+ecm_mark_nongui_executable(${_targetname})
endif()
- add_test(NAME ${_testname} COMMAND ${_testname})
- target_link_libraries(${_testname} ${ECM_ADD_TEST_LINK_LIBRARIES})
- ecm_mark_as_test(${_testname})
+ add_test(NAME ${_testname} COMMAND ${_targetname})
+ target_link_libraries(${_targetname} ${ECM_ADD_TEST_LINK_LIBRARIES})
+ ecm_mark_as_test(${_targetname})
endfunction()
function(ecm_add_tests)
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118768: ECMAddTests: make NAME_PREFIX only apply to the test, not the target
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118768/ --- (Updated June 21, 2014, 3:42 p.m.) Status -- This change has been marked as submitted. Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- ECMAddTests: make NAME_PREFIX only apply to the test, not the target David Faure and Patrick Spendrin have convinced me that NAME_PREFIX should be informational only, and not be used to prevent clashes, since it makes things confusing when you run tests manually. Diffs - modules/ECMAddTests.cmake 6cc20b65e25df02be6a91ac6004142d6b0f9e307 Diff: https://git.reviewboard.kde.org/r/118768/diff/ Testing --- Built and tested frameworks. Some need patching up (kconfig and kio in particular). I think the safest thing is to remove the use of NAME_PREFIX from those, so that people don't have to update ECM in order to build them. Also, I should write some unit tests... Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KCoreAddons does not install most of its headers on my system
On 19/06/14 00:32, Aleix Pol wrote: On Wed, Jun 18, 2014 at 10:56 PM, Ben Cooksley bcooks...@kde.org mailto:bcooks...@kde.org wrote: Note though that I consider this a workaround for an upstream regression which we have detected. One of the reasons the CI system runs the next branch is to detect such regressions before they end up in a release and impact us. Well, we need to make sure that we're using the cmake version that is useful for KDE developers, not for cmake. At the moment, the situation we are in is not acceptable where CI in general is totally unreliable.Furthermore, it's quite unlikely that we'll get to depend on 3.1 anytime soon, especially given that we're not requiring 3.0. It's not a question of depending on 3.1. It's a matter of people using 3.1 when they compile frameworks. For example, I use ArchLinux and so tend to have the latest released version of a given piece of software. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: git hooks for reviews mandatory?
On 19/06/14 22:21, Marco Martin wrote: Hi all, I was thinking, since the policy for committing in frameworks is to always asking for a review, what about on repositories under frameworks/* adding an hook that accepts pushes only if the comment has a REVIEW: line? I have been guilty too many times of not respecting that, mostly for not thinking about it at all, maybe I'm not the only one, an artificial enforce of discipline like that *may* make sense. opinions? would be useful, or mostly just an annoyance? My main concern is the scripted changes that some of us occasionally do (especially David with the releasing). I don't think there's any point reviewing more than the first one or two such changes (and none at all for the release version updates). So how do we get those past such hooks? Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118498: Make ECM language-independent again, but make the tests use C
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118498/ --- (Updated June 17, 2014, 6:17 p.m.) Status -- This change has been marked as submitted. Review request for Build System, Extra Cmake Modules, KDE Frameworks, Christophe Giboudeaux, Nicolás Alvarez, and Allen Winter. Repository: extra-cmake-modules Description --- Provide an option to exclude the tests While the tests in ECM are not built as such (at least, not until they are run), disabling the tests might be desirable to avoid the compiler checks and to make the whole build process architecture-independent. Make ECM language-independent again, but make the tests use C Setting the language for ECM's project() call to C had unanticipated side-effects - notably that the installed version file required the architecture to match the one used at build time. Instead, we make the tests a sub-project, setting up C as the language there (since most of the tests do use C, albeit slightly indirectly). Diffs - CMakeLists.txt df0759f18f1ed091b43128c1c5844aead560fe5b tests/CMakeLists.txt dfcc252a4e723d7376f87153adcd23d00dbc7846 Diff: https://git.reviewboard.kde.org/r/118498/diff/ Testing --- Configures properly, tests pass. Visually inspected ECMConfigVersion.cmake file to see that it will return before the architecture check. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118498: Make ECM language-independent again, but make the tests use C
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118498/ --- (Updated June 17, 2014, 6:17 p.m.) Status -- This change has been marked as submitted. Review request for Build System, Extra Cmake Modules, KDE Frameworks, Christophe Giboudeaux, Nicolás Alvarez, and Allen Winter. Repository: extra-cmake-modules Description --- Provide an option to exclude the tests While the tests in ECM are not built as such (at least, not until they are run), disabling the tests might be desirable to avoid the compiler checks and to make the whole build process architecture-independent. Make ECM language-independent again, but make the tests use C Setting the language for ECM's project() call to C had unanticipated side-effects - notably that the installed version file required the architecture to match the one used at build time. Instead, we make the tests a sub-project, setting up C as the language there (since most of the tests do use C, albeit slightly indirectly). Diffs - CMakeLists.txt df0759f18f1ed091b43128c1c5844aead560fe5b tests/CMakeLists.txt dfcc252a4e723d7376f87153adcd23d00dbc7846 Diff: https://git.reviewboard.kde.org/r/118498/diff/ Testing --- Configures properly, tests pass. Visually inspected ECMConfigVersion.cmake file to see that it will return before the architecture check. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118498: Make ECM language-independent again, but make the tests use C
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118498/ --- (Updated June 15, 2014, 4:13 p.m.) Review request for Build System, Extra Cmake Modules, KDE Frameworks, Christophe Giboudeaux, Nicolás Alvarez, and Allen Winter. Changes --- Adding people who've noted / reviewed this issue in the past. Repository: extra-cmake-modules Description --- Provide an option to exclude the tests While the tests in ECM are not built as such (at least, not until they are run), disabling the tests might be desirable to avoid the compiler checks and to make the whole build process architecture-independent. Make ECM language-independent again, but make the tests use C Setting the language for ECM's project() call to C had unanticipated side-effects - notably that the installed version file required the architecture to match the one used at build time. Instead, we make the tests a sub-project, setting up C as the language there (since most of the tests do use C, albeit slightly indirectly). Diffs - CMakeLists.txt df0759f18f1ed091b43128c1c5844aead560fe5b tests/CMakeLists.txt dfcc252a4e723d7376f87153adcd23d00dbc7846 Diff: https://git.reviewboard.kde.org/r/118498/diff/ Testing --- Configures properly, tests pass. Visually inspected ECMConfigVersion.cmake file to see that it will return before the architecture check. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [extra-cmake-modules] modules: Revert use the NAME_PREFIX only for the test, not for the executable
On 07/06/14 23:31, Alex Merry wrote: OK, since this seems to be what pretty much everyone who has said anything to me about this wants, let's make the name prefix only for the test name, then. I won't have time until Friday. If anyone wants to create an RR for it before then, I'll just say that the documentation needs to make the behaviour and the intended use clear. So... I didn't manage to do this yesterday (ill...), and I'm away for the weekend, so committing a change right now that will probably break a bunch of stuff seems like a bad idea. I should be able to find some time next week to do it, though. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Jenkins build became unstable: ktexteditor_master_qt5 #420
On 07/06/14 15:40, David Faure wrote: On Saturday 07 June 2014 13:51:17 KDE CI System wrote: See http://build.kde.org/job/ktexteditor_master_qt5/420/changes The following tests FAILED: 43 - indenttest (Timeout) Not sure what happened after PASS : IndentTest::testPascal(case6) ... Almost certainly a timeout. That test needs splitting up - it takes around 2 minutes to run, and two minutes is the cut-off point. So it sometimes passes and sometimes fails. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118499: Put kparts into a kf5/parts subdirectory of the plugins dir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118499/ --- (Updated June 7, 2014, 3:04 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Repository: khtml Description --- Put kparts into a kf5/parts subdirectory of the plugins dir Diffs - src/CMakeLists.txt 8a64eace8a579e8a588fcb08e193ce26c463bd68 src/java/CMakeLists.txt 9ff8cdf6ac3b168a83e46db62638119ac011de83 src/java/kjavaappletviewer.desktop 7c3e7fa8bc484659ae7946fcc584c58cad82d47f src/khtml.desktop f8596232692a6eba1052527e169305771e72cc5a src/khtmladaptorpart.desktop fc5587e683a07baf03b2913612d558b8e11c163c src/khtmlimage.desktop 5bdd5b1e0070b439cfbaa7dd4ec8a18a917f54b6 src/kmultipart/CMakeLists.txt 40f5051b6d76615e939d35bedd2ebd37f2bf337f src/kmultipart/kmultipart.desktop 6bc38afb9b6bca4d9792c0ecdcf6697a1976e1e7 Diff: https://git.reviewboard.kde.org/r/118499/diff/ Testing --- Built, installed, ran tests. The partviewer in the tests/ directory of KParts successfully loads khtml when you point it at a HTML file. I can't get it to load any of the other parts, though, with or without this patch. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Jenkins build became unstable: ktexteditor_master_qt5 #420
On 07/06/14 16:03, Alex Merry wrote: On 07/06/14 15:40, David Faure wrote: On Saturday 07 June 2014 13:51:17 KDE CI System wrote: See http://build.kde.org/job/ktexteditor_master_qt5/420/changes The following tests FAILED: 43 - indenttest (Timeout) Not sure what happened after PASS : IndentTest::testPascal(case6) ... Almost certainly a timeout. That test needs splitting up - it takes around 2 minutes to run, and two minutes is the cut-off point. So it sometimes passes and sometimes fails. In fact, it even says timeout in the list. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [extra-cmake-modules] modules: Revert use the NAME_PREFIX only for the test, not for the executable
On 06/06/14 20:29, David Faure wrote: On Saturday 10 May 2014 10:28:43 Alex Merry wrote: Git commit 7213137a75e832976c25168ae640d26369c771a7 by Alex Merry. Committed on 10/05/2014 at 10:26. Pushed by alexmerry into branch 'master'. Revert use the NAME_PREFIX only for the test, not for the executable I didn't follow the full story on this topic, but I'm not very happy with the current solution. It used to be that the prefix was just for the test, i.e. for the cmake output. Now it's part of the executable name. So I can't do make jobtest ./jobtest anymore, I have to do make kiocore-jobtest ./kiocore-jobtest That's unintuitive (it's jobtest.cpp) and too much typing. Why did we go from test names to executable names? Because I thought it wasn't that useful to namespace the test names without namespacing the targets/executables as well. I guess this depends on the purpose of the name prefix. If it's purely to say go look here in the codebase for this test, which is how it was used in kdelibs before the splitting, just changing the test name is fine. If it's to do actual namespacing, so that you can have a jobtest in one place and a jobtest in another place in the same project (but maybe one in some server code and one in some client code), then you need to namespace the targets as well. I don't think any of the frameworks should use the NAME_PREFIX argument anyway. It's no longer useful now that everything is split. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118490: Properly mark ServiceBrowser::isRunning as deprecated.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118490/#review59299 --- Ship it! Ship It! - Alex Merry On June 5, 2014, 3:56 a.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118490/ --- (Updated June 5, 2014, 3:56 a.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kdnssd Description --- Properly mark ServiceBrowser::isRunning as deprecated. Fix the @deprecated notation to be correct. Also, make the function a simple inline function that always returns false. This should cause minimal breakage while avoiding overhead from the function. Diffs - src/mdnsd-servicetypebrowser.cpp d16dd1fa1c692cd20e21f1a7d5471ac7956469b3 src/dummy-servicetypebrowser.cpp 39a4b6b660bb47b68b5721535994487dbe2abbf6 src/avahi-servicetypebrowser.cpp b3c14f84367d5093a6cee5ef3f684edc201a3f96 src/servicetypebrowser.h 3cd10e36bf089e088e8a48bb0cf73daa4fb859c5 Diff: https://git.reviewboard.kde.org/r/118490/diff/ Testing --- Code still compiles. Thanks, Matthew Dawson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118474: Move networkstatus kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/ --- (Updated June 4, 2014, 9:15 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs4support Description --- Move networkstatus kded module to kf5/kded subdirectory This makes sure the installed file is properly versioned. Diffs - src/solid-networkstatus/kded/CMakeLists.txt ca6bef21d4a8450651eaa8326898b6bfecdbb51a src/solid-networkstatus/kded/networkstatus.desktop b792b3cbe1b4dc95e74426ca358ea7005f8c7998 Diff: https://git.reviewboard.kde.org/r/118474/diff/ Testing --- Module is properly loaded when accessing the org.kde.kded5/modules/networkstatus D-Bus interface. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118476: Move kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/ --- (Updated June 4, 2014, 9:16 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Move kded module to kf5/kded subdirectory This makes sure the module is properly versioned. Diffs - src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 src/platformstatus/kded_platformstatus.desktop 9976fa3041a6c12fc4b3ce2146fb481181fa65b8 Diff: https://git.reviewboard.kde.org/r/118476/diff/ Testing --- qdbus org.kde.kded5 /kded loadModule kded_platformstatus succeeds Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118486: Put transcript plugin in kf5 subdir and simplify search logic
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/ --- (Updated June 4, 2014, 9:18 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Put transcript plugin in kf5 subdir and simplify search logic Putting the plugin in a versioned subdirectory ensures there will not be any coinstallability issues with KF6 if it is Qt5-based. Using QPluginLoader to find the plugin makes for simpler, more reliable code. Diffs - src/CMakeLists.txt 3099d2d29e43e40ac761694019f06aec41e54147 src/klocalizedstring.cpp 4a4f6f8c1a79d4b101ae3df5e589cb629f4bd596 Diff: https://git.reviewboard.kde.org/r/118486/diff/ Testing --- Tests pass, but not sure how to test the installed location. Chusslove: do you have a standard way to test the loading and operation of the transcript plugin? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118490: Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
On June 3, 2014, 10:16 a.m., Alex Merry wrote:
src/mdnsd-servicetypebrowser.cpp, line 51
https://git.reviewboard.kde.org/r/118490/diff/1/?file=277690#file277690line51
Interesting... any idea which monday?
Matthew Dawson wrote:
I'm assuming that is any Monday, as that used to be the day to break
SIC/BIC on kdelibs. It was there when I inherited the code. I'll just
remove the comment in an updated patch.
Ah, yes, I recall now.
On June 3, 2014, 10:16 a.m., Alex Merry wrote:
src/servicetypebrowser.h, line 93
https://git.reviewboard.kde.org/r/118490/diff/1/?file=277691#file277691line93
Since when?
Matthew Dawson wrote:
At least since KDELibs 4 time. Interestingly, Git shows the @deprecated
came in 2008, from a commit you made :). Looking back further, it seems to
just have been considered unnecessary and should be removed as of 2007, and
for the 4.4 releases effectively. I'll update the comment if the function is
kept.
Heh, so the sparse message is my fault :-)
On June 3, 2014, 10:16 a.m., Alex Merry wrote:
src/servicetypebrowser.h, lines 97-99
https://git.reviewboard.kde.org/r/118490/diff/1/?file=277691#file277691line97
It looks like the mdnsd backend even just returns false, so this is not
even reliable. With that in mind, I'd be tempted to make it an inline
method that just always returns false.
Matthew Dawson wrote:
Is it still late to do a minor SIC change? It appears the function is
useless, and is uneeded. Its only use seems to be for a structure similar to:
if (!isRunning()) {
startBrowse();
}
which can just be reduced to:
startBrowse();
as startBrowse can be called multiple times.
If it is preferred to keep it for SC purposes, then I'll take your
suggestion and just make it inline and return false.
I'd say at this point (after the last beta), and given the negligible cost of
keeping it, it should stay.
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118490/#review59066
---
On June 3, 2014, 6:59 a.m., Matthew Dawson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118490/
---
(Updated June 3, 2014, 6:59 a.m.)
Review request for KDE Frameworks.
Repository: kdnssd
Description
---
Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
Inline with the new defines used by Frameworks, remove the usage of the
KDE_NO_DEPRECATED define.
Diffs
-
src/avahi-servicetypebrowser.cpp b3c14f84367d5093a6cee5ef3f684edc201a3f96
src/dummy-servicetypebrowser.cpp 39a4b6b660bb47b68b5721535994487dbe2abbf6
src/mdnsd-servicetypebrowser.cpp d16dd1fa1c692cd20e21f1a7d5471ac7956469b3
src/servicetypebrowser.h 3cd10e36bf089e088e8a48bb0cf73daa4fb859c5
Diff: https://git.reviewboard.kde.org/r/118490/diff/
Testing
---
Code still compiles.
Thanks,
Matthew Dawson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118489: Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
On June 3, 2014, 10:12 a.m., Alex Merry wrote: Changing the macro name is a no-brainer. Comments below; a lot of them are about binary compatibility issues, and these depend on how we expect this macro to be used. I believe that Qt5 makes all its deprecated functions header-only so that compiling Qt without deprecated symbols produces a version BC with the version with deprecated symbols, and you can also safely set the macro yourself in application code to hide all Qt's deprecated symbols without messing anything up. As you've currently got it, the only way this macro is useful is to produce a separate, binary-incompatible version of the framework that has no deprecated code or symbols. And maybe that's the only thing it's worth supporting. I also highlighted some of the @deprecated apidox things - they really need to be more helpful, and this seemed to be an all the deprecated stuff review. Matthew Dawson wrote: Regarding the binary compatibility, kdelibs4support does a similar thing, which is why I thought it might be appropriate, but I also published the RR to be sure :) . Keeping with my don't rock the boat policy, I'll take whatever the Frameworks community deems to be appropriate. We should decide on a policy for deprecation in frameworks, and then publish that somewhere (I may have a new volunteer that would even be willing to write it!). Regarding the @deprecated apidox, I'll try to write something useful for them and update the patch later tonight. I won't comment on each one invidivually. Do you happen to have a good example of a @deprecated comment? Yeah, I think that policy thing is a good idea. If you have a volunteer for that, that's great! The most productive approach is probably to draft something and send it to the list for feedback. Example: @deprecated since 5.0, use SomeClass::someMethod() instead On June 3, 2014, 10:12 a.m., Alex Merry wrote: src/core/kcoreconfigskeleton.h, lines 1211-1220 https://git.reviewboard.kde.org/r/118489/diff/1/?file=277673#file277673line1211 This wrapping of the apidox in the deprecated macro has been done inconsistently. Matthew Dawson wrote: Is there a standard for how Frameworks code should wrap the declarations? I'll fix any to be consistent with the wider codebase. I don't think there's a standard, no. However, I think it makes sense to wrap the docs - that way, it should be possible to tell doxygen to assume KCONFIGCORE_NO_DEPRECATED is set, and it won't get confused by dox that aren't attached to a declaration. On June 3, 2014, 10:12 a.m., Alex Merry wrote: src/core/kcoreconfigskeleton.h, lines 1475-1487 https://git.reviewboard.kde.org/r/118489/diff/1/?file=277673#file277673line1475 I think this is a bad idea. Binary compatibility is very important in Frameworks, and this breaks it. And messing with virtuals is not just BIC, it's BIC in a way that can cause weird error messages (rather than just linking failures). Matthew Dawson wrote: As mentioned above, I wasn't sure what the policy should be. I'm happy doing it either way, but I think the wider community should decide. Can I suggest sending a separate mail to the list? People tend to brush over RRs, especially ones someone else has already commented on. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118489/#review59060 --- On June 3, 2014, 6:34 a.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118489/ --- (Updated June 3, 2014, 6:34 a.m.) Review request for KDE Frameworks. Repository: kconfig Description --- Change all occurrences of KDE_NO_DEPRECATED to an appropriate define. Inline with the new defines used by Frameworks, remove the KDE_NO_DEPRECATED. Also remove some deprecated virtuals when compiling without deprecated features. Make sure there are no deprecated features left when deprecated features are compiled out. When compiling without deprecated features, make sure not to use them. KEmailSettings was using deprecated entries when the enumerations for the features were compiled out, which would fail to compile. Thus, when compiling without deprecated features don't read extra configuration entries. Remove usages of deprecated virtuals, instead use their replacement. Go through and rename uses of usrWriteConfig to usrSave. The change should invoke no behavioural differences. Note that the kconfig_compiler has changed to the new function as well. Note that removing the deprecated virtuals does cause the non
Re: Review Request 118489: Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118489/#review59060 --- Changing the macro name is a no-brainer. Comments below; a lot of them are about binary compatibility issues, and these depend on how we expect this macro to be used. I believe that Qt5 makes all its deprecated functions header-only so that compiling Qt without deprecated symbols produces a version BC with the version with deprecated symbols, and you can also safely set the macro yourself in application code to hide all Qt's deprecated symbols without messing anything up. As you've currently got it, the only way this macro is useful is to produce a separate, binary-incompatible version of the framework that has no deprecated code or symbols. And maybe that's the only thing it's worth supporting. I also highlighted some of the @deprecated apidox things - they really need to be more helpful, and this seemed to be an all the deprecated stuff review. src/core/kconfig.h https://git.reviewboard.kde.org/r/118489/#comment41106 Since when? How to port away? src/core/kconfig.h https://git.reviewboard.kde.org/r/118489/#comment41105 Duplicate @deprecated, and neither of them say since when or give any indication of how to port away from them. src/core/kcoreconfigskeleton.h https://git.reviewboard.kde.org/r/118489/#comment41107 This wrapping of the apidox in the deprecated macro has been done inconsistently. src/core/kcoreconfigskeleton.h https://git.reviewboard.kde.org/r/118489/#comment41100 I think this is a bad idea. Binary compatibility is very important in Frameworks, and this breaks it. And messing with virtuals is not just BIC, it's BIC in a way that can cause weird error messages (rather than just linking failures). src/core/kcoreconfigskeleton.cpp https://git.reviewboard.kde.org/r/118489/#comment41102 I think this (and some of the other trivial deprecated methods) could be reasonably made header-only. This (a) makes versions of KConfig compiled with and without deprecated symbols more BC, and (b) documents the replacement code right there in the header. src/core/kemailsettings.h https://git.reviewboard.kde.org/r/118489/#comment41103 I would force the value of this, so that it is the same whether or not KCONFIGCORE_NO_DEPRECATED is set. src/core/kemailsettings.h https://git.reviewboard.kde.org/r/118489/#comment41104 What are you supposed to replace it with? src/gui/kconfigloader.h https://git.reviewboard.kde.org/r/118489/#comment4 @reimp? src/gui/kconfigloader.h https://git.reviewboard.kde.org/r/118489/#comment41110 Q_DECL_OVERRIDE? - Alex Merry On June 3, 2014, 6:34 a.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118489/ --- (Updated June 3, 2014, 6:34 a.m.) Review request for KDE Frameworks. Repository: kconfig Description --- Change all occurrences of KDE_NO_DEPRECATED to an appropriate define. Inline with the new defines used by Frameworks, remove the KDE_NO_DEPRECATED. Also remove some deprecated virtuals when compiling without deprecated features. Make sure there are no deprecated features left when deprecated features are compiled out. When compiling without deprecated features, make sure not to use them. KEmailSettings was using deprecated entries when the enumerations for the features were compiled out, which would fail to compile. Thus, when compiling without deprecated features don't read extra configuration entries. Remove usages of deprecated virtuals, instead use their replacement. Go through and rename uses of usrWriteConfig to usrSave. The change should invoke no behavioural differences. Note that the kconfig_compiler has changed to the new function as well. Note that removing the deprecated virtuals does cause the non-deprecated headers and deprecated libraries to be binary incompatible. Since you can't really do this I don't think its an issue. Also, kdelibs4support (others may too, haven't checked) uses a similar pattern. Diffs - autotests/kconfig_compiler/test_signal.cpp.ref 58e73efd77614edc4a5bd54bc06fbc34ccff2342 autotests/kconfig_compiler/test_signal.h.ref 19b8b4005e543e9e660f3ea016853c7a689ac17d autotests/kconfigtest.cpp 2b6de0d7d63df6aee69210aa09418628f0b8110a src/core/kconfig.h d27eebe7c41cb433b1808882c53cbf7b4c870950 src/core/kconfig.cpp ea9746c001e235529a1cdd5865b9e1b5c129b56a src/core/kconfiggroup.h 3c4bce8433e3c5d4cb2d9fdd111a43f04cf3c295 src/core/kconfiggroup.cpp 6f609baefec5beaf38fdfedd6d192b395e3f8acb src/core/kcoreconfigskeleton.h
Re: Review Request 118490: Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118490/#review59066 --- Ship it! The actual changes are sensible, hence the shipit, but see also the comments below that you may want to address. src/mdnsd-servicetypebrowser.cpp https://git.reviewboard.kde.org/r/118490/#comment41112 Interesting... any idea which monday? src/servicetypebrowser.h https://git.reviewboard.kde.org/r/118490/#comment41113 Since when? src/servicetypebrowser.h https://git.reviewboard.kde.org/r/118490/#comment41114 It looks like the mdnsd backend even just returns false, so this is not even reliable. With that in mind, I'd be tempted to make it an inline method that just always returns false. - Alex Merry On June 3, 2014, 6:59 a.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118490/ --- (Updated June 3, 2014, 6:59 a.m.) Review request for KDE Frameworks. Repository: kdnssd Description --- Change all occurrences of KDE_NO_DEPRECATED to an appropriate define. Inline with the new defines used by Frameworks, remove the usage of the KDE_NO_DEPRECATED define. Diffs - src/avahi-servicetypebrowser.cpp b3c14f84367d5093a6cee5ef3f684edc201a3f96 src/dummy-servicetypebrowser.cpp 39a4b6b660bb47b68b5721535994487dbe2abbf6 src/mdnsd-servicetypebrowser.cpp d16dd1fa1c692cd20e21f1a7d5471ac7956469b3 src/servicetypebrowser.h 3cd10e36bf089e088e8a48bb0cf73daa4fb859c5 Diff: https://git.reviewboard.kde.org/r/118490/diff/ Testing --- Code still compiles. Thanks, Matthew Dawson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118486: Put transcript plugin in kf5 subdir and simplify search logic
On June 3, 2014, 7:11 a.m., Chusslove Illich wrote: For KF5 I don't have anything other than tests at the moment. I should soon adapt some of the test programs I used earlier... Could you also check how this patch relates to the commit 06fdfea5? It does the opposite to that commit. Rather than removing the kf5 namespace from the search code, it changes the install location of the plugin to include it. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/#review59035 --- On June 2, 2014, 10:03 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/ --- (Updated June 2, 2014, 10:03 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Put transcript plugin in kf5 subdir and simplify search logic Putting the plugin in a versioned subdirectory ensures there will not be any coinstallability issues with KF6 if it is Qt5-based. Using QPluginLoader to find the plugin makes for simpler, more reliable code. Diffs - src/CMakeLists.txt 3099d2d29e43e40ac761694019f06aec41e54147 src/klocalizedstring.cpp 4a4f6f8c1a79d4b101ae3df5e589cb629f4bd596 Diff: https://git.reviewboard.kde.org/r/118486/diff/ Testing --- Tests pass, but not sure how to test the installed location. Chusslove: do you have a standard way to test the loading and operation of the transcript plugin? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118484: Move katepart into a kf5/parts subdir of plugin dir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118484/ --- (Updated June 3, 2014, 10:19 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Christoph Cullmann. Repository: ktexteditor Description --- Move katepart into a kf5/parts subdir of plugin dir This makes sure the file is properly versioned. Diffs - src/part/CMakeLists.txt b4d1fe9e4c60f5fbf30fb9315ea134faebcaec9c src/part/katepart.desktop fa760e9d1257c1d613d4fc59e7798202cde25fdd Diff: https://git.reviewboard.kde.org/r/118484/diff/ Testing --- kwrite still works. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118498: Make ECM language-independent again, but make the tests use C
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118498/ --- Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Provide an option to exclude the tests While the tests in ECM are not built as such (at least, not until they are run), disabling the tests might be desirable to avoid the compiler checks and to make the whole build process architecture-independent. Make ECM language-independent again, but make the tests use C Setting the language for ECM's project() call to C had unanticipated side-effects - notably that the installed version file required the architecture to match the one used at build time. Instead, we make the tests a sub-project, setting up C as the language there (since most of the tests do use C, albeit slightly indirectly). Diffs - CMakeLists.txt df0759f18f1ed091b43128c1c5844aead560fe5b tests/CMakeLists.txt dfcc252a4e723d7376f87153adcd23d00dbc7846 Diff: https://git.reviewboard.kde.org/r/118498/diff/ Testing --- Configures properly, tests pass. Visually inspected ECMConfigVersion.cmake file to see that it will return before the architecture check. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: extra-cmake-modules broken noarch support
On 03/06/14 13:08, Daniel Vrátil wrote:
Hi,
I noticed that in ECM, the generated file in
/usr/share/ECM/cmake/ECMConfigVersion.cmake contains an architecture check:
# check that the installed version has the same 32/64bit-ness as the one
which
is currently searching:
if(NOT ${CMAKE_SIZEOF_VOID_P} STREQUAL 4)
math(EXPR installedBits 4 * 8)
set(PACKAGE_VERSION ${PACKAGE_VERSION} (${installedBits}bit))
set(PACKAGE_VERSION_UNSUITABLE TRUE)
endif()
However ECM is noarch - it installs only non-binary files to /usr/share,
so
it should not matter what architecture it has been compiled on and what
arch
it's installed on. However thanks to the arch check, when the package gets
build on an i686 builder (which happened to me in this case), it's not
possible to build any 64bit packages against it:
Ah, this was some unexpected fallout from giving ECM a language. See
https://git.reviewboard.kde.org/r/118498/ for a fix.
BTW, for future reference, discussion about ECM happens on the
kde-buildsystem mailing list.
Alex
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118499: Put kparts into a kf5/parts subdirectory of the plugins dir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118499/ --- Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Repository: khtml Description --- Put kparts into a kf5/parts subdirectory of the plugins dir Diffs - src/CMakeLists.txt 8a64eace8a579e8a588fcb08e193ce26c463bd68 src/java/CMakeLists.txt 9ff8cdf6ac3b168a83e46db62638119ac011de83 src/java/kjavaappletviewer.desktop 7c3e7fa8bc484659ae7946fcc584c58cad82d47f src/khtml.desktop f8596232692a6eba1052527e169305771e72cc5a src/khtmladaptorpart.desktop fc5587e683a07baf03b2913612d558b8e11c163c src/khtmlimage.desktop 5bdd5b1e0070b439cfbaa7dd4ec8a18a917f54b6 src/kmultipart/CMakeLists.txt 40f5051b6d76615e939d35bedd2ebd37f2bf337f src/kmultipart/kmultipart.desktop 6bc38afb9b6bca4d9792c0ecdcf6697a1976e1e7 Diff: https://git.reviewboard.kde.org/r/118499/diff/ Testing --- Built, installed, ran tests. The partviewer in the tests/ directory of KParts successfully loads khtml when you point it at a HTML file. I can't get it to load any of the other parts, though, with or without this patch. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Keeping CI green
On 03/06/14 17:24, Kevin Ottens wrote: I see room for improvement in what gets evaluated when (like ability to run a patch in jenkins as part of the review process), I'm just stuck on the term enforcing there, not sure what you have in mind. Neither am I, to be honest. It just feels like we're ignoring the situation. I don't think Jenkins currently sends emails directly to people who appear to have broken something - can we make it do so? Do we want to? AFAIK we can't do that reliably. Now, what we could do is to always have the maintainer in CC of breakages, expecting said maintainer to react. I'm not fully sold on that idea, as that's likely to create a situation where the maintainer hunt down and point finger to the person who pushed the commit which broke something, while ideally it should be treated as a team thing. Well, ideally the commit was reviewed, so that at least spreads things out a bit... But, yes, it should be we need to fix this, not you need to fix this. The problem is that we're too good at ignoring breakages... it's a cultural thing, it needs changing. I didn't find the right tool for that yet, if someone has an idea I'm all ears. Yeah, I agree. Changing the culture is doable. It would involve us (by which I loosely mean the core frameworks developers) checking Jenkins status before and after pushing, and encouraging others to do the same. This encouragement can include asking people to hold off on shipping RRs until the build is fixed, and (gently) poking people who commit to a project that isn't green. It also means making an effort not to just ignore those emails Jenkins sends to the list. Do we want to have a fix it or have it reverted policy? How would that interact with failures that can only be reproduced on Jenkins (which does have an unusual setup)? That's something we could do, but that means we should do something about flaky tests. A flaky test is a mostly useless test, so either it should be fixed or it should be removed... Definitely. I know Qt's Gerritt setup tries to compensate for flaky tests by running them a second time if there's a failure, but that's hardly ideal. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118234: [frameworksintegration] Ensure the xcb connection gets flushed before the event dispatcher blocks
On May 26, 2014, 8:11 a.m., Àlex Fiestas wrote: src/platformtheme/CMakeLists.txt, line 4 https://git.reviewboard.kde.org/r/118234/diff/1/?file=273903#file273903line4 This workaround is quite important for 5.3.0 and older at least, maybe in those cases we should make it mandatory? Martin Gräßlin wrote: I have a feeling that the MacOS and Windows maintainer won't like a required XCB dependency :-) Alex Merry wrote: How about making it recommended? At least on non-Apple Unix systems. Aleix Pol Gonzalez wrote: MacOS or Windows shouldn't be running KDE's QPlatformTheme. Actually, we should probably call it Plasma QPlatformTheme. Which raises the question: does this belong in workspace? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/#review58445 --- On May 30, 2014, 6:15 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/ --- (Updated May 30, 2014, 6:15 a.m.) Review request for KDE Frameworks and Àlex Fiestas. Bugs: 334858 https://bugs.kde.org/show_bug.cgi?id=334858 Repository: frameworkintegration Description --- Ensure the xcb connection gets flushed before the event dispatcher blocks This is a workaround for Qt versions which do not yet have the change https://codereview.qt-project.org/85654 It is important to have this workaround as applications can get stalled when a framework uses xcb and doesn't flush the connection manually. BUG: 334858 Diffs - src/platformtheme/main.cpp 21d9aa0864e1887f5efbe4a05d264968af6e7e73 src/platformtheme/config-platformtheme.h.cmake PRE-CREATION src/platformtheme/CMakeLists.txt da77cf816fe5f63e8eb9277d5d81d957b89c7966 Diff: https://git.reviewboard.kde.org/r/118234/diff/ Testing --- 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 118234: [frameworksintegration] Ensure the xcb connection gets flushed before the event dispatcher blocks
On May 26, 2014, 8:11 a.m., Àlex Fiestas wrote: src/platformtheme/CMakeLists.txt, line 4 https://git.reviewboard.kde.org/r/118234/diff/1/?file=273903#file273903line4 This workaround is quite important for 5.3.0 and older at least, maybe in those cases we should make it mandatory? Martin Gräßlin wrote: I have a feeling that the MacOS and Windows maintainer won't like a required XCB dependency :-) Alex Merry wrote: How about making it recommended? At least on non-Apple Unix systems. Aleix Pol Gonzalez wrote: MacOS or Windows shouldn't be running KDE's QPlatformTheme. Actually, we should probably call it Plasma QPlatformTheme. Alex Merry wrote: Which raises the question: does this belong in workspace? Martin Gräßlin wrote: could we move that discussion somewhere else? Right now we need to implement it in a way that it doesn't break the build of MacOS and Windows and this fix is super important to run the workspace at all, I really want to get it in fast. It's bad enough that it missed the beta release again. Sure. Sorry for derailing. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/#review58445 --- On May 30, 2014, 6:15 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/ --- (Updated May 30, 2014, 6:15 a.m.) Review request for KDE Frameworks and Àlex Fiestas. Bugs: 334858 https://bugs.kde.org/show_bug.cgi?id=334858 Repository: frameworkintegration Description --- Ensure the xcb connection gets flushed before the event dispatcher blocks This is a workaround for Qt versions which do not yet have the change https://codereview.qt-project.org/85654 It is important to have this workaround as applications can get stalled when a framework uses xcb and doesn't flush the connection manually. BUG: 334858 Diffs - src/platformtheme/main.cpp 21d9aa0864e1887f5efbe4a05d264968af6e7e73 src/platformtheme/config-platformtheme.h.cmake PRE-CREATION src/platformtheme/CMakeLists.txt da77cf816fe5f63e8eb9277d5d81d957b89c7966 Diff: https://git.reviewboard.kde.org/r/118234/diff/ Testing --- 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 118234: [frameworksintegration] Ensure the xcb connection gets flushed before the event dispatcher blocks
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/#review58947 --- What happens with X11 on OS/X? I suspect this is a possible setup. Is it one we care about? - Alex Merry On May 30, 2014, 6:15 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/ --- (Updated May 30, 2014, 6:15 a.m.) Review request for KDE Frameworks and Àlex Fiestas. Bugs: 334858 https://bugs.kde.org/show_bug.cgi?id=334858 Repository: frameworkintegration Description --- Ensure the xcb connection gets flushed before the event dispatcher blocks This is a workaround for Qt versions which do not yet have the change https://codereview.qt-project.org/85654 It is important to have this workaround as applications can get stalled when a framework uses xcb and doesn't flush the connection manually. BUG: 334858 Diffs - src/platformtheme/main.cpp 21d9aa0864e1887f5efbe4a05d264968af6e7e73 src/platformtheme/config-platformtheme.h.cmake PRE-CREATION src/platformtheme/CMakeLists.txt da77cf816fe5f63e8eb9277d5d81d957b89c7966 Diff: https://git.reviewboard.kde.org/r/118234/diff/ Testing --- 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 118234: [frameworksintegration] Ensure the xcb connection gets flushed before the event dispatcher blocks
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/#review58953 --- Ship it! Given the only objection so far has been that maybe we should make it mandatory, I'm going to say ship it. Making it required later on is easy if that turns out to be the best thing to do. - Alex Merry On May 30, 2014, 6:15 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118234/ --- (Updated May 30, 2014, 6:15 a.m.) Review request for KDE Frameworks and Àlex Fiestas. Bugs: 334858 https://bugs.kde.org/show_bug.cgi?id=334858 Repository: frameworkintegration Description --- Ensure the xcb connection gets flushed before the event dispatcher blocks This is a workaround for Qt versions which do not yet have the change https://codereview.qt-project.org/85654 It is important to have this workaround as applications can get stalled when a framework uses xcb and doesn't flush the connection manually. BUG: 334858 Diffs - src/platformtheme/main.cpp 21d9aa0864e1887f5efbe4a05d264968af6e7e73 src/platformtheme/config-platformtheme.h.cmake PRE-CREATION src/platformtheme/CMakeLists.txt da77cf816fe5f63e8eb9277d5d81d957b89c7966 Diff: https://git.reviewboard.kde.org/r/118234/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118474: Move networkstatus kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/ --- Review request for KDE Frameworks. Repository: kdelibs4support Description --- Move networkstatus kded module to kf5/kded subdirectory This makes sure the installed file is properly versioned. Diffs - src/solid-networkstatus/kded/CMakeLists.txt ca6bef21d4a8450651eaa8326898b6bfecdbb51a src/solid-networkstatus/kded/networkstatus.desktop b792b3cbe1b4dc95e74426ca358ea7005f8c7998 Diff: https://git.reviewboard.kde.org/r/118474/diff/ Testing --- Module is properly loaded when accessing the org.kde.kded5/modules/networkstatus D-Bus interface. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118476: Move kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/ --- Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Move kded module to kf5/kded subdirectory This makes sure the module is properly versioned. Diffs - src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 src/platformstatus/kded_platformstatus.desktop 9976fa3041a6c12fc4b3ce2146fb481181fa65b8 Diff: https://git.reviewboard.kde.org/r/118476/diff/ Testing --- qdbus org.kde.kded5 /kded loadModule kded_platformstatus succeeds Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118472: Version the kded module files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118472/ --- (Updated June 2, 2014, 4:52 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Repository: kio Description --- Version the kded module files This puts KIO's KDED modules in a kf5/kded subdirectory of the plugin directory. Diffs - src/ioslaves/http/kcookiejar/CMakeLists.txt 48c2df8748b6587cd5b09ee9c73c098856fdd03c src/ioslaves/http/kcookiejar/kcookiejar.desktop f5cc5f3baf6d865c6ee4b7e1b13b9967faed2413 src/kpac/CMakeLists.txt a7502dc6343aeedec5c43f12ae8b222e9beba256 src/kpac/proxyscout.desktop ada3db6a5e066012afb284503924d08596ba796c src/kpasswdserver/CMakeLists.txt c49d056e91c64f2450cf150458a69a431d574ed3 src/kpasswdserver/kpasswdserver.desktop 486c5d5e6f61397f1ca927d78c74a6c87aae8197 src/kssld/CMakeLists.txt 06fccaa69e9c46d896d45f7d5dd993d2b84615ff src/kssld/kssld.desktop bc3630f0e23d1de7c274d8329b3dac7ed5e0ec8e Diff: https://git.reviewboard.kde.org/r/118472/diff/ Testing --- All the modules are properly loaded on demand when attempting to access their D-Bus interface. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118484: Move katepart into a kf5/parts subdir of plugin dir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118484/ --- Review request for KDE Frameworks and Christoph Cullmann. Repository: ktexteditor Description --- Move katepart into a kf5/parts subdir of plugin dir This makes sure the file is properly versioned. Diffs - src/part/CMakeLists.txt b4d1fe9e4c60f5fbf30fb9315ea134faebcaec9c src/part/katepart.desktop fa760e9d1257c1d613d4fc59e7798202cde25fdd Diff: https://git.reviewboard.kde.org/r/118484/diff/ Testing --- kwrite still works. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118486: Put transcript plugin in kf5 subdir and simplify search logic
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/ --- Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Put transcript plugin in kf5 subdir and simplify search logic Putting the plugin in a versioned subdirectory ensures there will not be any coinstallability issues with KF6 if it is Qt5-based. Using QPluginLoader to find the plugin makes for simpler, more reliable code. Diffs - src/CMakeLists.txt 3099d2d29e43e40ac761694019f06aec41e54147 src/klocalizedstring.cpp 4a4f6f8c1a79d4b101ae3df5e589cb629f4bd596 Diff: https://git.reviewboard.kde.org/r/118486/diff/ Testing --- Tests pass, but not sure how to test the installed location. Chusslove: do you have a standard way to test the loading and operation of the transcript plugin? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118385: Remove name prefix from autotests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118385/ --- (Updated June 1, 2014, 9:30 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs4support Description --- Remove name prefix from autotests This is (a) unnecessary, now that it is not part of kdelibs, and (b) breaks a couple of the tests, which make assumptions about the executable name. Diffs - autotests/CMakeLists.txt 4b230b2128406be806e9250aeef532ed698dff7c Diff: https://git.reviewboard.kde.org/r/118385/diff/ Testing --- Builds, tests pass. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118384: Improve the kgendesignerplugin man page
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118384/ --- (Updated June 1, 2014, 10:14 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Alex Merry. Repository: kdesignerplugin Description --- Improve the kgendesignerplugin man page The options now match what is actually accepted, the authors section properly reflects the authors of the man page, rather than the tool, the description is more comprehensive and the input file format is documented. Diffs - docs/kgendesignerplugin/man-kgendesignerplugin.1.docbook c1c349dafdd064e0ffa27b0dc65b413c17aa06c7 Diff: https://git.reviewboard.kde.org/r/118384/diff/ Testing --- Built, installed, viewed with the `man` command. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118211: Add autotests for designer plugin generation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118211/ --- (Updated June 1, 2014, 10:16 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Alex Merry. Repository: kdesignerplugin Description --- Add autotests for designer plugin generation Diffs - CMakeLists.txt e7464fc9036e3c4b8664d601b06d02f5bd924fdb autotests/CMakeLists.txt PRE-CREATION autotests/minimal.widgets PRE-CREATION autotests/minimaltest.cpp PRE-CREATION autotests/plugintest.cpp PRE-CREATION autotests/qpushbuttonview.png PRE-CREATION autotests/sth.png PRE-CREATION autotests/testplugin.qrc PRE-CREATION autotests/testplugin.widgets PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118211/diff/ Testing --- The test passes. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118193: Use the macros provided by KDesignerPlugin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118193/ --- (Updated June 1, 2014, 12:55 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs4support Description --- Use the macros provided by KDesignerPlugin This also removes the RPath settings, as they are entirely unnecessary (and arguably not correct, since we are linking against a library that is locally built). Diffs - src/CMakeLists.txt 6c3a0f3c974c5f48dc8b9a8874440424f0f7d0f0 Diff: https://git.reviewboard.kde.org/r/118193/diff/ Testing --- Builds, installs, added an arrow button to a form in designer. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Maintainers: Please get ready for release
On 26/04/14 23:32, Kevin Ottens wrote: Please get back to us when you're done checking your framework, if you get any issue or if something is unclear. Alex Merry: - kdesignerplugin - kimageformats - kmediaplayer (porting aid) All checked. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118340: Allow the kactivitymanagerd daemon to be disabled.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118340/#review58850 --- I'm happy with this. I think this should go in, even if there isn't another release, because packagers can pick it up as an official patch if they want. But I'll let Ivan have the final say. - Alex Merry On May 31, 2014, 3:42 a.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118340/ --- (Updated May 31, 2014, 3:42 a.m.) Review request for KDE Frameworks, Plasma and Ivan Čukić. Repository: kactivities Description --- To allow for the library to be co-installed with the frameworks version, allow the daemon to be disabled. I'm not sure if this is the best way to do this. If there is any better way, I'll update the patch as appropriate. Diffs - src/CMakeLists.txt b4733648fd53ebd681694f185cc5b23f51dfd3f9 Diff: https://git.reviewboard.kde.org/r/118340/diff/ Testing --- Visually inspected install directories. Seems to remove only what is necessary. Thanks, Matthew Dawson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118403: Use correct name in KDE4_ADD_KDEINIT_EXECUTABLE compat macro
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118403/#review58790 --- Ship it! Ship It! - Alex Merry On May 29, 2014, 4:09 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118403/ --- (Updated May 29, 2014, 4:09 p.m.) Review request for KDE Frameworks. Repository: kdelibs4support Description --- s/KDE4_ADD_KDEINIT_EXECUTABLE/kf5_add_kdeinit_executable/ is easy, but there's no reason properties should have wrong names ;-) Diffs - cmake/modules/KDE4Macros.cmake 94a630a Diff: https://git.reviewboard.kde.org/r/118403/diff/ Testing --- Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118404: make sure krossqtsplugin is really treated as plugin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118404/#review58791 --- Ship it! Ship It! - Alex Merry On May 29, 2014, 4:21 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118404/ --- (Updated May 29, 2014, 4:21 p.m.) Review request for KDE Frameworks. Repository: kross Description --- installing (so)versioned plugin doesn't make much sense. also added missing MODULE argument. Diffs - src/qts/CMakeLists.txt 8bfd83d Diff: https://git.reviewboard.kde.org/r/118404/diff/ Testing --- Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Issues for Qt5/KF5/KDE/CI system on OSX/MacPorts
On 29/05/14 08:05, Ben Cooksley wrote: On Thu, May 29, 2014 at 8:08 AM, Marko Käning mk-li...@email.de wrote: Could not locate file kf5/kdoctools/customization in (/Users/kdeci/Library/Application Support, /Library/Application Support) --- which leaves me a little puzzled now. It is nothing to do with the installation parameters now. What needs to be adjusted is kdoctools - we'll need to help it find things within the install prefix. Is there a environment variable which we can set which the code in question (which I assume uses QStandardPaths) will follow? In terms of the value of DATA_INSTALL_DIR, I suggest you examine the install jail (located at $WORKSPACE/install/) to determine where the files are actually being placed and act accordingly. Ah. This. QStandardPaths appears to have no way to provide custom prefixes on Mac or Windows, which is an issue for setups like this. Submitting a Qt patch to provide a cross-platform, Qt-specific equivalent to the XDG_* variables has been suggested, but I don't what came of that. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117974: Some more KArchive examples
On May 12, 2014, 3:16 p.m., Kevin Ottens wrote: examples/bzip2gzip/main.cpp, line 74 https://git.reviewboard.kde.org/r/117974/diff/1/?file=271400#file271400line74 Maybe a better idea to use a loop to avoid the readAll? I know that's an example which needs to be kept simple, but that would be a bit more realistic to read a few bytes from the input and write them in the output to not load the memory. Any progress on this? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117974/#review57784 --- On May 3, 2014, 2:54 p.m., Maarten De Meyer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117974/ --- (Updated May 3, 2014, 2:54 p.m.) Review request for KDE Frameworks. Repository: karchive Description --- 2 extra examples on how to use the KArchive framework. Together with the 2 we already have most of the basic usage is covered. unzipper: Shows how to extract files from an archive bzip2gzip: Example about KCompressionDevice They are BSD licensed because I consider that a good license for examples. I wouldn't mind changing to GPL. Please let me know how they could be improved. Readability, comments, variable naming, too simple? Diffs - examples/bzip2gzip/CMakeLists.txt PRE-CREATION examples/bzip2gzip/main.cpp PRE-CREATION examples/unzipper/CMakeLists.txt PRE-CREATION examples/unzipper/main.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117974/diff/ Testing --- Builds and runs ok. Easy to read and comprehend. (imho) Thanks, Maarten De Meyer ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117985: frameworks/kjs: add missing man page docbook
On May 5, 2014, 7:17 a.m., Kevin Ottens wrote: IIRC that was intentional as to not have kjs depend on kdoctools. Hrm. Distros like Debian aren't going to be super-happy about this. And KJS is officially a porting aid, so I'm not sure bumping it to tier 3 is that big of an issue (especially since the practical dependencies are still minimal). - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117985/#review57287 --- On May 4, 2014, 7:21 a.m., Burkhard Lück wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117985/ --- (Updated May 4, 2014, 7:21 a.m.) Review request for Documentation and KDE Frameworks. Repository: kjs Description --- add missing man page docbook, which got lost in the transition from kdelibs to frameworks Diffs - CMakeLists.txt 60ee824 docs/CMakeLists.txt PRE-CREATION docs/kjs/CMakeLists.txt PRE-CREATION docs/kjs/man-kjs5.1.docbook PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117985/diff/ Testing --- build + installed, viewed with man kjs5 Thanks, Burkhard Lück ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118128: Use actual bytes formatter for sizes in KDirModel
On May 14, 2014, 2:24 p.m., David Faure wrote: It is correct that this is about a string representation of the filesize, to displaying in a column of the model. For machine processing one can use KFileItem::size() after getting the KFileItem out of the KDirModel. However, do we really want that the detailed listview in dolphin? I can more easily recognize the biggest number by being the one with most digits, than having to go through a list of kB, MB, and GB values. It might even break the sorting by size. Let's ask Frank Reininghaus, but I'm not sure this is a good idea. Frank Reininghaus wrote: I might not have fully understood what exactly this patch will change in the GUI - both the file dialog (which uses KDirModel) and Dolphin already do show B/KiB/MiB/etc. values in the detailed view here (on a system with the rather old KDE SC 4.10.5 - I currently don't have anything else to test here), but maybe something changed about that in frameworks? If QSortFilterProxyModel uses the value returned by the function that is modified by this patch (I think it does?), then this might really break sorting by size in the dialog. Have you checked this, Martin? Martin Klapetek wrote: Frank - what this changes is returning bytes formatted by your locale to returning the human readable size + the unit itself. So for example a file with 2000 bytes would return 20,000,000 for locales with , as thousands delimiter before this patch, whereas now it would return 20 MB (including the unit). I'm not sure if Dolphin even uses this though; I also think this already does not work with sorting (as also Mark G. noted) because it does not do locale-aware sort. David - well if your file is 2487651687 bytes big, isn't it just easier to read that as 2.3 GiB? ;) Plus, Dolphin indeed already does this. -- I just think that it's better to actually return a locale-formatted human-readable number rather than just locale-formatted number, which is what this patch does :) Unless this should break things... Alternatively, if this is actually wanted, I can add another role like Mark suggested. David Faure wrote: OK, first step, then is: figuring out why sorting works in KFileDialog right now in kdelibs4, and where the user-readable strings come from. KDirModel, also in kdelibs4, only returns a locale-formatted number, not a human-readable one. Looks like my choice in KDirModel got overriden in the layers above it anyway :-) David Faure wrote: Ah, it's done in the delegate, see KFileItemDelegate::Private::itemSize(). So actually KDirModel should just return a plain number, for sorting to work out of the box :-) Now I'm curious, Martin, where did this patch make any visual change? Surely not in anything that uses KFileItemDelegate Martin Klapetek wrote: Yeah, another way is to make it return just bytes and use the KFormat on the UI side, that also makes sense. As for the visual change - Plasma folder view's tooltip, which shows file sizes, currently in bytes (folderview is based on kdirmodel) Any progress on this? It looks like the concensus is to not do any formatting on the model side. Can you upload a patch that does that? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118128/#review57923 --- On May 14, 2014, 2:01 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118128/ --- (Updated May 14, 2014, 2:01 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- Now I'm not sure if returning the unit together with the size won't break some things, but as it returns string already, I thought returning it in the human readable form would be better than always returning bytes. Diffs - src/widgets/kdirmodel.cpp 70d5ee4 Diff: https://git.reviewboard.kde.org/r/118128/diff/ Testing --- Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118155: adapt to ecm_add_tests so that tests can be found
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118155/#review58704 --- Ship it! Builds and tests pass for me on Linux. I think we don't need the NAME_PREFIX arguments any more, but I'll wait to see what people think of https://git.reviewboard.kde.org/r/118385/ first. - Alex Merry On May 15, 2014, 3 p.m., Patrick Spendrin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118155/ --- (Updated May 15, 2014, 3 p.m.) Review request for KDE Frameworks, kdewin and Plasma. Repository: plasma-framework Description --- adapt to ecm_add_tests so that tests can be found Diffs - autotests/CMakeLists.txt dcee37f0771753d3e381e9d77f351cff16531e93 Diff: https://git.reviewboard.kde.org/r/118155/diff/ Testing --- mingw 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 117125: start_kdeinit: Use capabilities instead of SUID
On April 11, 2014, 4:46 p.m., Commit Hook wrote: This review has been submitted with commit e898d13b430692e775060d49342181192e122fdf by Hrvoje Senjan to branch master. Hrvoje Senjan wrote: i've reverted the commit now. capabilities break LD_LIBRARY_PATH, so this is a no-go. apologies for potentially caused troubles =( Hrvoje Senjan wrote: hm, but we have worse situation with SUID (and LD_LIBRARY_PATH is also not propagated there). the process would terminate, as i wrote in diff2 changes. i wonder should OOM protection be removed entirely? at least with distribution side of things, it looks like we had it SUID on openSUSE; and from what i found, none of e.g. Arch, Fedora, Debian/Kubuntu, Gentoo has it this way... I assume the same can be done with kcheckpass at some point too? missed this one. it would appear so, but i've just tried removing the sticky bits, and unlock works correctly (with KF5 based locker). so maybe not :) Actually, ArchLinux does have start_kdeinit setuid. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117125/#review55468 --- On May 15, 2014, 9:12 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117125/ --- (Updated May 15, 2014, 9:12 p.m.) Review request for KDE Frameworks, Andreas Hartmetz and David Faure. Bugs: https://bugzilla.novell.com/show_bug.cgi?id=862953 https://bugs.kde.org/show_bug.cgi?id=https://bugzilla.novell.com/show_bug.cgi?id=862953 Repository: kinit Description --- The issue came up on security review of kinit package (yes, same is valid for kdelibs4...) SUSE security team is not happy with kdeinit being SUID helper, thus capabilities are utilized first (if available) I've just tried to integrate the suggested patch (from the report) with the CMake bits Diffs - CMakeLists.txt 8bd43d8 cmake/FindLibcap.cmake PRE-CREATION src/config-kdeinit.h.cmake c89c713 src/start_kdeinit/CMakeLists.txt 6bfc496 src/start_kdeinit/start_kdeinit.c 3c733e7 Diff: https://git.reviewboard.kde.org/r/117125/diff/ Testing --- Built: with setcap libcap present - installed as advertised; without one/both of them - the old procedure is in place (using SUID for the helper) I am not sure how to test the OOM killer, fortunately it never kicked in kdelibs4 variant, so can't also say did it work as planned before... Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117125: start_kdeinit: Use capabilities instead of SUID
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117125/#review58706 --- What's the plan with this? Does Andreas' fix for the setuid case also fix the capabilities case? - Alex Merry On May 15, 2014, 9:12 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117125/ --- (Updated May 15, 2014, 9:12 p.m.) Review request for KDE Frameworks, Andreas Hartmetz and David Faure. Bugs: https://bugzilla.novell.com/show_bug.cgi?id=862953 https://bugs.kde.org/show_bug.cgi?id=https://bugzilla.novell.com/show_bug.cgi?id=862953 Repository: kinit Description --- The issue came up on security review of kinit package (yes, same is valid for kdelibs4...) SUSE security team is not happy with kdeinit being SUID helper, thus capabilities are utilized first (if available) I've just tried to integrate the suggested patch (from the report) with the CMake bits Diffs - CMakeLists.txt 8bd43d8 cmake/FindLibcap.cmake PRE-CREATION src/config-kdeinit.h.cmake c89c713 src/start_kdeinit/CMakeLists.txt 6bfc496 src/start_kdeinit/start_kdeinit.c 3c733e7 Diff: https://git.reviewboard.kde.org/r/117125/diff/ Testing --- Built: with setcap libcap present - installed as advertised; without one/both of them - the old procedure is in place (using SUID for the helper) I am not sure how to test the OOM killer, fortunately it never kicked in kdelibs4 variant, so can't also say did it work as planned before... Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118162: Make sure to use the absolute file path for test files.
On May 16, 2014, 11:49 a.m., Alex Merry wrote: This sounds like an issue that needs to be fixed in KArchive... Where, exactly, were the files appearing? And which files were appearing there? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118162/#review58053 --- On May 16, 2014, 11:27 a.m., Patrick von Reth wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118162/ --- (Updated May 16, 2014, 11:27 a.m.) Review request for KDE Frameworks and kdewin. Repository: karchive Description --- The test files apeared at random file locations on Windows. Using the absolute file path fixed that issue. Diffs - autotests/karchivetest.cpp 6b50f32a6d99756ba246262067dd71573f129256 Diff: https://git.reviewboard.kde.org/r/118162/diff/ Testing --- Windows Thanks, Patrick von Reth ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 117125: start_kdeinit: Use capabilities instead of SUID
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117125/#review58714 --- Ship it! Given that the original issue seems to be fixed, I think this can go in again. - Alex Merry On May 15, 2014, 9:12 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117125/ --- (Updated May 15, 2014, 9:12 p.m.) Review request for KDE Frameworks, Andreas Hartmetz and David Faure. Bugs: https://bugzilla.novell.com/show_bug.cgi?id=862953 https://bugs.kde.org/show_bug.cgi?id=https://bugzilla.novell.com/show_bug.cgi?id=862953 Repository: kinit Description --- The issue came up on security review of kinit package (yes, same is valid for kdelibs4...) SUSE security team is not happy with kdeinit being SUID helper, thus capabilities are utilized first (if available) I've just tried to integrate the suggested patch (from the report) with the CMake bits Diffs - CMakeLists.txt 8bd43d8 cmake/FindLibcap.cmake PRE-CREATION src/config-kdeinit.h.cmake c89c713 src/start_kdeinit/CMakeLists.txt 6bfc496 src/start_kdeinit/start_kdeinit.c 3c733e7 Diff: https://git.reviewboard.kde.org/r/117125/diff/ Testing --- Built: with setcap libcap present - installed as advertised; without one/both of them - the old procedure is in place (using SUID for the helper) I am not sure how to test the OOM killer, fortunately it never kicked in kdelibs4 variant, so can't also say did it work as planned before... Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118211: Add autotests for designer plugin generation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118211/ --- (Updated May 29, 2014, 1:52 p.m.) Review request for KDE Frameworks and Alex Merry. Changes --- Add a test for the least info you can get away with in a widgets file. Summary (updated) - Add autotests for designer plugin generation Repository: kdesignerplugin Description (updated) --- Add autotests for designer plugin generation Diffs (updated) - CMakeLists.txt e7464fc9036e3c4b8664d601b06d02f5bd924fdb autotests/CMakeLists.txt PRE-CREATION autotests/minimal.widgets PRE-CREATION autotests/minimaltest.cpp PRE-CREATION autotests/plugintest.cpp PRE-CREATION autotests/qpushbuttonview.png PRE-CREATION autotests/sth.png PRE-CREATION autotests/testplugin.qrc PRE-CREATION autotests/testplugin.widgets PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118211/diff/ Testing --- The test passes. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118377: Add an autotest for designer plugin generation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118377/ --- (Updated May 29, 2014, 1:59 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kdesignerplugin Description --- Add an autotest for designer plugin generation Diffs - CMakeLists.txt e7464fc9036e3c4b8664d601b06d02f5bd924fdb autotests/CMakeLists.txt PRE-CREATION autotests/plugintest.cpp PRE-CREATION autotests/qpushbuttonview.png PRE-CREATION autotests/sth.png PRE-CREATION autotests/testplugin.qrc PRE-CREATION autotests/testplugin.widgets PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118377/diff/ Testing --- Builds, test passes. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118384: Improve the kgendesignerplugin man page
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118384/ --- (Updated May 29, 2014, 2:01 p.m.) Review request for KDE Frameworks and Alex Merry. Changes --- Fix the first example, which didn't work (KConfig requires you to put at least one key in a group for it to notice the group). Repository: kdesignerplugin Description --- Improve the kgendesignerplugin man page The options now match what is actually accepted, the authors section properly reflects the authors of the man page, rather than the tool, the description is more comprehensive and the input file format is documented. Diffs (updated) - docs/kgendesignerplugin/man-kgendesignerplugin.1.docbook c1c349dafdd064e0ffa27b0dc65b413c17aa06c7 Diff: https://git.reviewboard.kde.org/r/118384/diff/ Testing --- Built, installed, viewed with the `man` command. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118384: Improve the kgendesignerplugin man page
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118384/ --- (Updated May 29, 2014, 2:35 p.m.) Review request for KDE Frameworks and Alex Merry. Changes --- Remove two incorrect keys from the man page. I misread the code - you can't set Class or PluginName for individual classes. Repository: kdesignerplugin Description --- Improve the kgendesignerplugin man page The options now match what is actually accepted, the authors section properly reflects the authors of the man page, rather than the tool, the description is more comprehensive and the input file format is documented. Diffs (updated) - docs/kgendesignerplugin/man-kgendesignerplugin.1.docbook c1c349dafdd064e0ffa27b0dc65b413c17aa06c7 Diff: https://git.reviewboard.kde.org/r/118384/diff/ Testing --- Built, installed, viewed with the `man` command. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
