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: Web Shortcuts KCM
On Tuesday 15 July 2014 15:16:20 Kevin Ottens wrote: (ie at most a widget would be enough for the app related settings, we should talk to the underlying platform for the other ones). I don't want users to have to configure their search engines in 10 KDE apps one after the other by hand. A centralized configuration is much more convenient. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119316: Fix minor memory leak in Wallpaper interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119316/ --- Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Fix minor memory leak in Wallpaper interface Diffs - src/scriptengines/qml/plasmoid/wallpaperinterface.cpp f1da571 Diff: https://git.reviewboard.kde.org/r/119316/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119316: Fix minor memory leak in Wallpaper interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119316/#review62493 --- Ship it! Ship It! - Marco Martin On July 16, 2014, 11:57 a.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119316/ --- (Updated July 16, 2014, 11:57 a.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Fix minor memory leak in Wallpaper interface Diffs - src/scriptengines/qml/plasmoid/wallpaperinterface.cpp f1da571 Diff: https://git.reviewboard.kde.org/r/119316/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119316: Fix minor memory leak in Wallpaper interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119316/ --- (Updated July 16, 2014, 1:39 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Fix minor memory leak in Wallpaper interface Diffs - src/scriptengines/qml/plasmoid/wallpaperinterface.cpp f1da571 Diff: https://git.reviewboard.kde.org/r/119316/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Call for help: possible race conditions in KAuth
When submitting KAuth to openSUSE, the SUSE security team found possible race conditions that could lead to security issues[1]- I'm writing here because until these issues are solved, KAuth will not be accepted into openSUSE. The second reason I'm posting this here is because it seems people involved with KAuth are not reachable: - security@ko was contacted without an answer; - other KDE people including drf were contacted without a response; Some discussion was raised with Martin Briza (CC'ed just in case, so he may provide some insight, at least) with regards to polkit-qt-1 issues which were (to my understanding) fixed. I can say I cannot fix this at all (I can write C++, but I have neither the skill nor the time to fix what's needed here), and therefore this is a cry for help to see at least the identification of the issue and a fix or workaround, or just an explanation why this is not an issue. I think this is quite important as KAuth is a security-related framework. [1] https://bugzilla.novell.com/show_bug.cgi?id=864716#c41 -- Luca Beltrame - KDE Forums team KDE Science supporter GPG key ID: 6E1A4E79 signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119321: kio-extras: Install KIO and KDED modules into correct folders
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119321/ --- Review request for KDE Frameworks. Repository: kio-extras Description --- The KIO Frameworks installs the ioslaves into ```${PLUGIN_INSTALL_DIR}/kf5/kio```, so kio-extras should follow. This patch also drops the ```kio_``` prefix from the binary name - it made sense when all the libraries were in /usr/lib, but does not make sense when they have their own folder now (also the ones installed by KIO Framework don't have the prefix) Same for KDED modules and KParts installed by kio-extras: KDED Framework installs KDED modules to ```${PLUGIN_INSTALL_DIR}/kf5/kded``` and binaries don't have the ```kded_``` prefix, KParts are installed to ```${PLUGIN_INSTALL_DIR}/kf5/parts```. Diffs - desktop/CMakeLists.txt f7bd670 recentdocuments/recentdocuments.protocol c0bf1eb desktop/desktopnotifier.desktop a32ee6f filter/CMakeLists.txt 240cc7b filter/bzip.protocol 7d3cb57 filter/bzip2.protocol 1baaf7d filter/gzip.protocol 8ed55ec filter/lzma.protocol 13bb28e filter/xz.protocol f4fb7f0 fish/CMakeLists.txt 24136d4 fish/fish.protocol ff5784e info/CMakeLists.txt 0163e82 info/info.protocol fa8cbb4 man/CMakeLists.txt 3343ed8 man/kmanpart.desktop cc5df64 man/man.protocol cc100ce network/ioslave/CMakeLists.txt 67fc482 network/ioslave/network.protocol 1e10cc6 network/kded/CMakeLists.txt 3be676e network/kded/networkwatcher.desktop f9fdce3 nfs/CMakeLists.txt dfc6eae nfs/nfs.protocol 85cf203 recentdocuments/CMakeLists.txt bc2b9db bookmarks/bookmarks.protocol 0642bd2 archive/tar.protocol 19447a5 archive/zip.protocol ce7c54b bookmarks/CMakeLists.txt 703b109 archive/CMakeLists.txt ec2cf7a archive/ar.protocol 7a848e5 recentdocuments/recentdocumentsnotifier.desktop 096b14b settings/CMakeLists.txt 5a57a18 settings/settings.protocol efb03e8 sftp/CMakeLists.txt defb7dd sftp/sftp.protocol ec15eeb smb/CMakeLists.txt a3a772f smb/smb.protocol e585978 thumbnail/CMakeLists.txt f3733d4 thumbnail/thumbnail.protocol eef743a trash/CMakeLists.txt 4ee0358 trash/trash.protocol 2776985 Diff: https://git.reviewboard.kde.org/r/119321/diff/ Testing --- Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119323: fix auth race condition
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- Review request for KDE Frameworks and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119323: fix auth race condition
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- (Updated July 16, 2014, 4:05 p.m.) Review request for KDE Frameworks, Hrvoje Senjan, Luca Beltrame, and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119323: fix auth race condition
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/#review62520 --- I ran the (few) autotests, and those also pass. I can't test it runtime, will do so later hopefully. - Luca Beltrame On Lug. 16, 2014, 4:05 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- (Updated Lug. 16, 2014, 4:05 p.m.) Review request for KDE Frameworks, Hrvoje Senjan, Luca Beltrame, and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119323: fix auth race condition
On July 16, 2014, 6:06 p.m., Luca Beltrame wrote: I ran the (few) autotests, and those also pass. I can't test it runtime, will do so later hopefully. a few runtime checks (e.g. org.kde.powerdevil.backlighthelper) seem to work =) also polkit reports system-bus-name is used - Hrvoje --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/#review62520 --- On July 16, 2014, 6:05 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- (Updated July 16, 2014, 6:05 p.m.) Review request for KDE Frameworks, Hrvoje Senjan, Luca Beltrame, and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119321: kio-extras: Install KIO and KDED modules into correct folders
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119321/#review62522 --- The kioslaves part seems to be more or less like my patch here: https://git.reviewboard.kde.org/r/119081/ , just without the renaming of the output file. - Alexander Richardson On Juli 16, 2014, 5:20 nachm., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119321/ --- (Updated Juli 16, 2014, 5:20 nachm.) Review request for KDE Frameworks. Repository: kio-extras Description --- The KIO Frameworks installs the ioslaves into ```${PLUGIN_INSTALL_DIR}/kf5/kio```, so kio-extras should follow. This patch also drops the ```kio_``` prefix from the binary name - it made sense when all the libraries were in /usr/lib, but does not make sense when they have their own folder now (also the ones installed by KIO Framework don't have the prefix) Same for KDED modules and KParts installed by kio-extras: KDED Framework installs KDED modules to ```${PLUGIN_INSTALL_DIR}/kf5/kded``` and binaries don't have the ```kded_``` prefix, KParts are installed to ```${PLUGIN_INSTALL_DIR}/kf5/parts```. Diffs - desktop/CMakeLists.txt f7bd670 recentdocuments/recentdocuments.protocol c0bf1eb desktop/desktopnotifier.desktop a32ee6f filter/CMakeLists.txt 240cc7b filter/bzip.protocol 7d3cb57 filter/bzip2.protocol 1baaf7d filter/gzip.protocol 8ed55ec filter/lzma.protocol 13bb28e filter/xz.protocol f4fb7f0 fish/CMakeLists.txt 24136d4 fish/fish.protocol ff5784e info/CMakeLists.txt 0163e82 info/info.protocol fa8cbb4 man/CMakeLists.txt 3343ed8 man/kmanpart.desktop cc5df64 man/man.protocol cc100ce network/ioslave/CMakeLists.txt 67fc482 network/ioslave/network.protocol 1e10cc6 network/kded/CMakeLists.txt 3be676e network/kded/networkwatcher.desktop f9fdce3 nfs/CMakeLists.txt dfc6eae nfs/nfs.protocol 85cf203 recentdocuments/CMakeLists.txt bc2b9db bookmarks/bookmarks.protocol 0642bd2 archive/tar.protocol 19447a5 archive/zip.protocol ce7c54b bookmarks/CMakeLists.txt 703b109 archive/CMakeLists.txt ec2cf7a archive/ar.protocol 7a848e5 recentdocuments/recentdocumentsnotifier.desktop 096b14b settings/CMakeLists.txt 5a57a18 settings/settings.protocol efb03e8 sftp/CMakeLists.txt defb7dd sftp/sftp.protocol ec15eeb smb/CMakeLists.txt a3a772f smb/smb.protocol e585978 thumbnail/CMakeLists.txt f3733d4 thumbnail/thumbnail.protocol eef743a trash/CMakeLists.txt 4ee0358 trash/trash.protocol 2776985 Diff: https://git.reviewboard.kde.org/r/119321/diff/ Testing --- Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On Wednesday, 2014-07-16, 10:33:43, David Faure wrote: On Tuesday 15 July 2014 15:16:20 Kevin Ottens wrote: (ie at most a widget would be enough for the app related settings, we should talk to the underlying platform for the other ones). I don't want users to have to configure their search engines in 10 KDE apps one after the other by hand. A centralized configuration is much more convenient. Hmm, what if KDE applications outside a KDE workspace are seen as separate entities by users of those other workspaces? Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On Wednesday 16 July 2014 21:12:25 Kevin Krammer wrote: On Wednesday, 2014-07-16, 10:33:43, David Faure wrote: On Tuesday 15 July 2014 15:16:20 Kevin Ottens wrote: (ie at most a widget would be enough for the app related settings, we should talk to the underlying platform for the other ones). I don't want users to have to configure their search engines in 10 KDE apps one after the other by hand. A centralized configuration is much more convenient. Hmm, what if KDE applications outside a KDE workspace are seen as separate entities by users of those other workspaces? Exactly... They definitely are in such a case. Cheers. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On Wednesday 16 July 2014 10:33:43 David Faure wrote: On Tuesday 15 July 2014 15:16:20 Kevin Ottens wrote: (ie at most a widget would be enough for the app related settings, we should talk to the underlying platform for the other ones). I don't want users to have to configure their search engines in 10 KDE apps one after the other by hand. A centralized configuration is much more convenient. Maybe I was unclear, but I didn't mean to imply that for something like the web shortcuts having a centralized configuration was wrong. I think that shipping a KCM is wrong. But I'm fine with a widget which provide mean to save in a centralized configuration. Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On Tuesday 15 July 2014 16:59:29 Eike Hein wrote: On 07/15/2014 03:16 PM, Kevin Ottens wrote: Well, for a single entry menu, really? :-) Yeah, I do care about every single menu entry not being broken, no matter where users use my app :). Sorry, I was unclear there. I meant having to deal with a single menu entry to kick out if running outside of a plasma session. Of course admittedly the situation in KDE 4 wasn't good either since it actually meant a runtime dep on kde-baseapps. Indeed. I'm OK with spawning our own dialog and using a widget class, but I don't have time to work on that personally (I'm busy with Plasma and Konversation). This is a roadblock to keeping feature continuity in KF5 releases, so we do need to ship the KCM for now. Let me know where you guys want it put in the end. My opinion is that for the time being it should be shipped by the workspace. Once the widget I mentioned in that thread would be available then the KCM could be ported to use it, and Konversation would be able to use said widget directly. Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119323: fix auth race condition
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/#review62546 --- src/backends/polkit-1/Polkit1Backend.cpp https://git.reviewboard.kde.org/r/119323/#comment43392 i guess we can also have QDBusConnection::systemBus().baseService() here? - Hrvoje Senjan On July 16, 2014, 6:05 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- (Updated July 16, 2014, 6:05 p.m.) Review request for KDE Frameworks, Hrvoje Senjan, Luca Beltrame, and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119323: fix auth race condition
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/#review62547 --- FYI, this probably needs to go in kdelibs too as the issue is there too. I and Hrvoje are currently testing also a version for 4.x. - Luca Beltrame On Lug. 16, 2014, 4:05 p.m., Martin Tobias Holmedahl Sandsmark wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119323/ --- (Updated Lug. 16, 2014, 4:05 p.m.) Review request for KDE Frameworks, Hrvoje Senjan, Luca Beltrame, and Martin Bříza. Repository: kauth Description --- pid based auth is racy because of pid reuse, don't use it. Diffs - src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/119323/diff/ Testing --- it builds Thanks, Martin Tobias Holmedahl Sandsmark ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel