Re: Information regarding upcoming Gitlab Migration: clarifications
On Fri, May 01, 2020 at 09:25:18AM -0400, Allen Winter wrote: > On Thursday, April 30, 2020 5:15:43 PM EDT Albert Astals Cid wrote: > > El dijous, 30 d’abril de 2020, a les 21:31:02 CEST, Ben Cooksley va > > escriure: > > > On Fri, May 1, 2020 at 6:04 AM Ivan Čukić wrote: > > > > > > > > > We have made a big fuss in the past about having different projects > > > > > that do the same thing and now we'll have that but also we'll have > > > > > several projects with the same name? > > > > > It really feels off to me and I wonder if this is related to the move > > > > > to > > > > > gitlab. > > > > > > > > +1 to both sentiments - that projects should have different names and > > > > that > > > > this is a bit off topic for the gitlab migration. > > > > > > The projects *DO* have very different names. That *HAS NOT* changed. > > > To use the example Bhushan gave above, one is called Plasma Mobile > > > Dialer and the other one is called Maui Dialer. > > > > > > With the current git.kde.org setup, we have a flat namespace, so all > > > repositories if the name appears to be generic (as dialer is) have to > > > be namespaced by prefixing of the repository name. > > > > > > With Gitlab however we will now namespaces that group repositories, > > > making the prefixing unnecessary and as some projects have complained > > > about, duplicative. > > > > > > Otherwise you end up with plasma-mobile/plasma-mobile-dialer as your > > > path, which just looks silly. > > > > Am I the only person that just has all the repos on the same folder? I > > thought it was the common thing to do :? > > > I use kdesrc-build and I see the repos in a hierarchy. > In particular, I like frameworks in frameworks in kdepim in kde/pim > > I don't see that I'm setting any special "layout in a hierarchy" option in my > kdesrc-buildrc So it's been a few months since we had switched the default, but since it's clearly an invasive change, the way we addressed it was to make the flat hierarchy a default for new users (who use either of the 'quick config' schemes like kdesrc-build-setup or kdesrc-build --initial-setup), but to leave the built-in default unchanged. So in essence, existing kdesrc-build users (who had a folder-based layout by default unless they went out of their way to find the right option) saw no change, but new users would have that option pre-set for them in the config. Regards, - Michael Pyne
Re: Notice of upcoming changes to the behaviour of the anongit network
On Sat, Apr 11, 2020 at 10:14:38PM +1200, Ben Cooksley wrote: > Hi all, > > As part of the preparations for the move to Gitlab, and the rewrite of > our anongit tooling, one of the things we have looked into is how the > anongit network in general operates. > > As part of this, it has been observed that the git:// protocol is > unencrypted, and thus vulnerable to intercept and manipulation by > hostile actors. > > We have therefore decided that support for the git:// protocol to > access KDE Git repositories will cease following our migration to > Gitlab. > > Going forward, all anonymous access should take place instead over > https, which is encrypted, and has the added benefit of offering > support for redirects (should those be needed) For kdesrc-build users, as Johan Ouwerkerk noted on the other Gitlab thread, kdesrc-build since January 2020 has already switched over to using https for KDE-based source repositories in anticipation of this change. Thanks to Ben and the sysadmin team for coordinating ahead of time on this, it's allowed us to have it deployed for 3 months now and we haven't heard of any breakage as a result of this change. One important note is that if you have any git repositories that you have manually checked out using KDE's anongit, you may need to either manually adjust it to use a https:// git remote (if it uses git:// now), or adjust it to use a kde: remote (as explained at https://community.kde.org/Sysadmin/GitKdeOrgManual#Let_Git_rewrite_URL_prefixes). kdesrc-build configures git to understand this "kde:" prefix by default. Regards, - Michael Pyne
Re: Update on Status of Gitlab Migration
On Sat, Apr 11, 2020 at 09:25:11PM +0200, Johan Ouwerkerk wrote: > On Sat, Apr 11, 2020 at 8:39 PM Ben Cooksley wrote: > > > > Yes, the hostname git.kde.org will be fully retired as part of this > > transition. > > > > From my understanding kdesrc-build will automatically pick this up > > once we update sysadmin/repo-metadata to show the new repository > > paths. > > This is something we'll confirm with mpyne though to ensure we can > > make the cutover as smooth as possible. > > > > Just to be clear, my understanding based on reading the > `Updated/Git.pm` module is that KDE repo paths are abstracted via > ~/.gitconfig URL remapping using `insteadOf`and `pushInsteadOf`. > Currently the code manipulates the user's ~/.gitconfig to bind the > correct mappings to the `kde:` prefix (this happens even before > cloning sysadmin repos for metadata). > > So if my understanding of the code is correct, the entire switch over > is transparent provided that kdesrc-build is updated beforehand to set > the updated value for `pushInsteadOf`. We already have the same > mechanism in place in kdesrc-build for ensuring that people use > https://anongit.kde.org instead of git://anongit.kde.org when > cloning/fetching. Yeah, when Ben asked me a couple of months back about it, this was the same conclusion I reached after reviewing the existing code. But I need to note that the way this works right now is that a module is cloned via a URL such as 'kde:juk' (not kde:kde/kdemultimedia/juk!). This is transparent to how git operates when setup with ~/.gitconfig, so you won't notice it in `git remote -v`, you need to actually look at the repository's .git/config, or run `git config --local --get remote.origin.url` in the source directory, to see whether it uses a kde: URL or a full URL. Either way, once the switchover happens, then *in theory* it can be as easy as running kdesrc-build once (to update ~/.gitconfig) and from there Git will rewrite to the updated URL automatically. We could add the switchover logic before that and guard it with a date check, that way we can do some testing early. > > Depending on how things look we may also make available a script that > > will update the configuration of a repository to reflect both the > > change in hostname as well as the change in path. > > > > A cleanup script could be handy. I think kdesrc-build will > automatically pick up new repo paths from metadata and that should > work transparently, but the old clones may get left behind as well. > People who use the kdesrc-build option to ignore KDE repo structure > shouldn't be affected at all. I don't know that we'll even necessarily need a cleanup script (though it couldn't hurt). In my case, my entire source repository contains only one repository directly referencing anongit (or git.kde.org), all others are non-KDE or kde: 1 git://anongit.kde.org/scratch/ 1 git://cmake.org/ 16 git://code.qt.io/ 1 git://git.freedesktop.org/ 1 git://git.gnupg.org/ 3 git://github.com/ 23 https://code.qt.io/ 7 https://github.com/ 1 https://gitlab.com/ 344 kde: All of the kde: repositories use the kde:foo syntax, where the 'foo' comes from the 'repopath' parameter of the sysadmin/repo-metadata YAML files. We may need to do on-the-fly conversion of the kde: repo paths if they won't be expressible as 'kde:foo' in the future, but we should have the information needed to do this in kdesrc-build to make this happen on-the-fly. Regards, - Michael Pyne
D14984: Explicitly request Qt 5.7's QtQuick to make use of Connections.enabled
This revision was automatically updated to reflect the committed changes. Closed by commit R169:b23e642e5ac4: Explicitly request Qt 5.7s QtQuick to make use of Connections.enabled (authored by mpyne). REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14984?vs=40179=40265 REVISION DETAIL https://phabricator.kde.org/D14984 AFFECTED FILES src/controls/private/RefreshableScrollView.qml src/controls/templates/private/ScrollView.qml To: mpyne, #frameworks, #plasma, mart, davidedmundson Cc: plasma-devel, apol, davidedmundson, mart, hein
D14984: Explicitly request Qt 5.7's QtQuick to make use of Connections.enabled
mpyne created this revision. mpyne added reviewers: Frameworks, Plasma, mart. Herald added a project: Kirigami. Herald added a subscriber: plasma-devel. mpyne requested review of this revision. REVISION SUMMARY I recently rebuilt my Qt, KF5, and Plasma 5 environments from scratch. I noticed that System Settings no longer started. It took me a while to figure out the error message but the proximate issue ended up being: file:///home/kde-svn/kde-5/lib64/qml/org/kde/kirigami.2/templates/private/ScrollView.qml:95:9: "Connections.enabled" is not available in QtQuick 2.5. enabled: !Settings.tabletMode ^ I imagine that Qt 5 has stricter checking on the use of QML properties. In this case we ask for QtQuick as of Qt 5.5, but Connections.enabled is only supported in Qt 5.7 on. I fixed this (and one other such usage) by changing the import statement to ask for QtQuick 5.7. We already require at least Qt 5.7 anyways so this isn't a new dependency for current KF5. TEST PLAN Installed the updated Kirigami, successfully launched System Settings. Additionally the Plasma desktop wallpaper started working again too, though that wasn't why I'd made this fix. I verified by manual inspection of all uses of the 'Connection' QML item that there were no other uses of the "enabled" property from Qt 5.7. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D14984 AFFECTED FILES src/controls/private/RefreshableScrollView.qml src/controls/templates/private/ScrollView.qml To: mpyne, #frameworks, #plasma, mart Cc: plasma-devel, apol, davidedmundson, mart, hein
Re: Closing old Plasma 4 bugs
On Sun, Feb 11, 2018 at 07:29:01PM +1300, Ben Cooksley wrote: > On Sun, Feb 11, 2018 at 4:06 PM, Nate Graham <pointedst...@zoho.com> wrote: > > On 02/10/2018 05:48 PM, Nicolás Alvarez wrote: > >> > >> Meanwhile... maybe you can do some loud blog posts calling for triagers? > >> :) > > > > > > Sounds good. Before then, we need to clean up the wiki page for this: > > https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging > > > > It's thorough and mostly up to date, but huge and intimidating. I've been > > working to improve it, but assistance would be appreciated > > > > > > Problem #2: the fact that you need to ask permission before gaining write > > access to other people's hugs is a gigantic blockade to getting more > > community bug triaging. A LibreOffice bug triager came by recently and > > explained that they got an enormous amount more community bug triaging by > > moving away from this policy and only locking down a few things (like the > > priority and assignee, I think) and leaving the rest open. He said that in > > particular it was a big boost to the number of bugs (correctly) marked as > > duplicates and closed as fixed. > > Interesting. If the broader community is okay with this I see no > reason why we couldn't rearrange the permissions and how they're > currently setup. I can only speak for myself but I'd be very much in favor or loosening restrictions on bug permissions to at least allow for community bug triaging. It's the kind of task that requires community involvement anyways (for testing in environments devs can't easily reproduce) and I think anything we do that gives the broader user community more 'ownership' into the bug database would lead to a more accurate and useful bug database. Regards, - Michael Pyne
D6658: Fix mdstat parsing of inactive RAID arrays, remove dead code
This revision was automatically updated to reflect the committed changes. Closed by commit R106:3588e85cd8a6: softraid: Remove dead code and associated warning. (authored by mpyne). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6658?vs=16597=17265#toc REPOSITORY R106 KSysguard CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6658?vs=16597=17265 REVISION DETAIL https://phabricator.kde.org/D6658 AFFECTED FILES ksysguardd/Linux/softraid.c To: mpyne, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6658: Fix mdstat parsing of inactive RAID arrays, remove dead code
mpyne added a comment. If anyone has objections please let me know in the next few days. REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D6658 To: mpyne, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6631: Fix compilation with strict libc (such as musl)
mpyne added a comment. In https://phabricator.kde.org/D6631#126937, @bcooksley wrote: > Sorry, looks like it's still unhappy (another different error). This time it's complaining about gettimeofday() > According to https://github.com/kinetiknz/cubeb/issues/6 defining XOPEN_SOURCE will sort this, but i'm not sure if musl is okay with that? musl is fine with `XOPEN_SOURCE` as long as it's defined properly. It implies `_POSIX_C_SOURCE` in the FreeBSD headers anyways, though musl amusingly doesn't care at all for `gettimeofday`. musl *did* care for signals-related functions, but it defines those under `_XOPEN_SOURCE` as well (see https://anonscm.debian.org/cgit/collab-maint/musl.git/tree/include/signal.h). I'll take a look. REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D6631 To: mpyne, #plasma, mart Cc: bcooksley, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6631: Fix compilation with strict libc (such as musl)
mpyne added a comment. In https://phabricator.kde.org/D6631#126366, @bcooksley wrote: > Thanks for the quick fix. > > Getting closer - got a different error this time... > https://build.kde.org/job/Plasma%20ksysguard%20kf5-qt5%20FreeBSDQt5.7/7/ This fix wasn't as quick, I had to regenerate my musl environment, but I've pushed a commit which should hopefully make it to the end by removing an unneeded internal include (sys/file.h). REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D6631 To: mpyne, #plasma, mart Cc: bcooksley, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6631: Fix compilation with strict libc (such as musl)
mpyne added a comment. In https://phabricator.kde.org/D6631#126101, @bcooksley wrote: > Clang doesn't seem to like this: https://build.kde.org/job/Plasma%20ksysguard%20kf5-qt5%20FreeBSDQt5.7/4/console Thanks. Looks from the build log like it's not good enough to define the `_POSIX_C_SOURCE` macro and let it take a default meaning, it needs to be defined to a specific value. I've pushed a fix which I hope will make it through CI. REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D6631 To: mpyne, #plasma, mart Cc: bcooksley, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6658: Fix mdstat parsing of inactive RAID arrays, remove dead code
mpyne created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Something I noticed while doing the musl libc fixes: 1. A GCC warning is generated for `in_devs`. It turns out the warning was referring to a Coverity entry (but the ID was wrong, it's 253228, not 3228), the Coverity entry noted that the `in_devs` code is dead code. And so it is. Whatever it was supposed to do before (warn about RAID entries using another RAID device as a sub-device?), it doesn't do it now --> removed. 2. A boolean check for whether the raid array was active or not was miscoded (and therefore was always true). The compiler warning for this was actually what led to me looking through this code, but Coverity also flagged it as well. From inspection, the check actually is necessary since the "level" data we're extracting here is only present if the array is active, so I fixed by removing the errorneous condition. TEST PLAN Builds fine on glibc and musl libc. However since I don't have a RAID array I have no good way of testing the change itself, though it should at least not break anything worse than it's already broken. REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D6658 AFFECTED FILES ksysguardd/Linux/softraid.c To: mpyne, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6631: Fix compilation with strict libc (such as musl)
This revision was automatically updated to reflect the committed changes. Closed by commit R106:257591cf462d: Fix compilation with strict libc (such as musl). (authored by mpyne). REPOSITORY R106 KSysguard CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6631?vs=16520=16590 REVISION DETAIL https://phabricator.kde.org/D6631 AFFECTED FILES ksysguardd/Command.c ksysguardd/Linux/ProcessList.c ksysguardd/Linux/diskstat.c ksysguardd/Linux/netstat.c ksysguardd/Linux/softraid.c ksysguardd/PWUIDCache.c ksysguardd/conf.c ksysguardd/ksysguardd.c To: mpyne, #plasma, mart Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6631: Fix compilation with strict libc (such as musl)
mpyne edited the summary of this revision. mpyne removed a reviewer: Frameworks. mpyne changed the repository for this revision from R111 KSysguard Library to R106 KSysguard. REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D6631 To: mpyne, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6631: Fix compilation with strict libc (such as musl)
mpyne created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Many of the functions we use implicitly are not part of the C standard library so we have to specific that we're pulling from POSIX, X/Open, BSD, or whatever standard we're using before we can rely on the definition being available. There's plenty of room for refactoring here since I mostly stopped at just getting the functions being called to show up from their includes, although I did replace rindex with strrchr since the latter is standardized, and replaced `uint` with `unsigned` rather than trying to figure out what header includes those. TEST PLAN Now compiles against musl libc Still compiles against glibc. REPOSITORY R111 KSysguard Library REVISION DETAIL https://phabricator.kde.org/D6631 AFFECTED FILES ksysguardd/Command.c ksysguardd/Linux/ProcessList.c ksysguardd/Linux/diskstat.c ksysguardd/Linux/netstat.c ksysguardd/Linux/softraid.c ksysguardd/PWUIDCache.c ksysguardd/conf.c ksysguardd/ksysguardd.c To: mpyne, #plasma, #frameworks Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6327: [ksshaskpass] Support mercurial (hg) ssh prompts
mpyne accepted this revision. mpyne added a comment. This revision is now accepted and ready to land. Looks good to me, and compiles/tests fine here. Though I'm not using it on any hg repos so I can't actually test the new feature, but as long as it works for you I'd say go ahead and commit. REPOSITORY R105 KDE SSH Password Dialog REVISION DETAIL https://phabricator.kde.org/D6327 To: cfeck, whiting, mpyne Cc: plasma-devel, #plasma, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
[Differential] [Accepted] D4540: fix bug with git after update to Plasma 5.9 in neon
mpyne accepted this revision. mpyne added a comment. The change is fine as far as privacy impact, it shouldn't add any issues that aren't already present. REPOSITORY R105 KDE SSH Password Dialog REVISION DETAIL https://phabricator.kde.org/D4540 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: idekels, whiting, bbuch, mpyne Cc: bbuch, cfeck, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
Re: Review Request for KWayland for inclusion in frameworks
On Sat, May 7, 2016 13:50:05 David Faure wrote: > Hi guys, > > Can you check the CI for KWayland? ASAN detects an invalid memory usage > in KWayland::Client::OutputDevice::Private::doneCallback() called after > KWayland::Client::OutputDevice::~OutputDevice(). > > https://build.kde.org/view/Frameworks%20kf5-qt5/job/kwayland%20master%20kf5-> > qt5/28/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/kwa > yland_testWaylandOutputDevice/ I find that report kind of confusing. The line 323 of test_wayland_outputdevice.cpp is just a QSignalSpy constructor, it shouldn't involve destruction of an OutputDevice (what ASAN is warning about here). I wonder if it maybe has something to do with the modern signal/slot connection syntax in use? For what it's worth there's a relevant Coverity entry (CID 1340943), noting that the TestWaylandOutputDevice class does not have an initializer for m_queue within the constructor (or in any of the other class members it looked at). But I don't see how that would be related either. Regards, - Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 127837: oxygen_windeco: Fix potential use-after-free from improper use of QCache
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127837/ --- (Updated May 7, 2016, 4:24 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Changes --- Submitted with commit b595f47f021cbcf1cee23cd0bee1df57a2e2f1b9 by Michael Pyne to branch master. Repository: oxygen Description --- oxygen windeco: Fix use-after-free for pixmap in QCache. Coverity identified (in CID 1336171) a usage of QPixmap after it's been the parameter to QCache::insert. This is unsafe since QCache gains ownership of whatever object is passed in, and can even (at least in theory) delete the object you pass in. Instead give the QCache its own copy of the pixmap. To make this work without leaking our existing pixmap we need to work with a QPixmap instead of QPixmap* in the non-cached case. This leads to whitespace changes; use git diff -b to filter pure whitespace changes out. Diffs - kdecoration/oxygendecohelper.cpp a7e9306 Diff: https://git.reviewboard.kde.org/r/127837/diff/ Testing --- The new code compiles but I'm currently unable to test since kcmshell5 kwindecoration (and any other KCM viewer) is currently crashing for me in QML, due to what I'm sure is a local misconfiguration... Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 127837: oxygen_windeco: Fix potential use-after-free from improper use of QCache
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127837/ --- Review request for KDE Frameworks and Plasma. Repository: oxygen Description --- oxygen windeco: Fix use-after-free for pixmap in QCache. Coverity identified (in CID 1336171) a usage of QPixmap after it's been the parameter to QCache::insert. This is unsafe since QCache gains ownership of whatever object is passed in, and can even (at least in theory) delete the object you pass in. Instead give the QCache its own copy of the pixmap. To make this work without leaking our existing pixmap we need to work with a QPixmap instead of QPixmap* in the non-cached case. This leads to whitespace changes; use git diff -b to filter pure whitespace changes out. Diffs - kdecoration/oxygendecohelper.cpp a7e9306 Diff: https://git.reviewboard.kde.org/r/127837/diff/ Testing --- The new code compiles but I'm currently unable to test since kcmshell5 kwindecoration (and any other KCM viewer) is currently crashing for me in QML, due to what I'm sure is a local misconfiguration... Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 126539: Check sockaddr_un buffer size before strcpy()ing into it.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126539/ --- Review request for KDE Frameworks and Plasma. Repository: kwallet-pam Description --- Coverity strikes again, and notes in CID 1335116 that copying the socket name into a fixed-size buffer here could overflow the buffer. I don't see any reason it would be wrong in all cases, so best to double-check. Submitting for review mostly because I don't use pam_kwallet, otherwise the check is simple enough that I'd feel comfortable committing directly. Note that the len that is already calculated includes the null terminator already. Diffs - pam_kwallet.c 345aa03 Diff: https://git.reviewboard.kde.org/r/126539/diff/ Testing --- Code still compiles. Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126539: Check sockaddr_un buffer size before strcpy()ing into it.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126539/ --- (Updated Dec. 28, 2015, 1:34 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Changes --- Submitted with commit 9543cc4058b24e4e5bfe8d324de309ca7050058b by Michael Pyne to branch master. Repository: kwallet-pam Description --- Coverity strikes again, and notes in CID 1335116 that copying the socket name into a fixed-size buffer here could overflow the buffer. I don't see any reason it would be wrong in all cases, so best to double-check. Submitting for review mostly because I don't use pam_kwallet, otherwise the check is simple enough that I'd feel comfortable committing directly. Note that the len that is already calculated includes the null terminator already. Diffs - pam_kwallet.c 345aa03 Diff: https://git.reviewboard.kde.org/r/126539/diff/ Testing --- Code still compiles. Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 126512: Use proper deleters for libxcb structs in xembed-sni-proxy.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126512/ --- Review request for Plasma. Repository: plasma-workspace Description --- Coverity noted we were mismatching new Foo with free(foo), which is undefined behavior (CID 1340556). While I was fixing that I noticed we have the same issue with QScopedPointer<>: when using QSP to track objects returned by libxcb, we must use free() to release memory, not C++ delete. (e.g. see the [XCB API](http://xcb.freedesktop.org/manual/group__XCBAPI.html#ga6727f2bfb24769655e52d1f1c50f58fe)). QScopedPointer will do this if we use QScopedPointerPodDeleter. Diffs - xembed-sni-proxy/sniproxy.cpp ae6eab7 Diff: https://git.reviewboard.kde.org/r/126512/diff/ Testing --- I've only compiled and verified no warnings at this point. I didn't see any relevant crash bugs after a quick web search either. Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126443: Fix memleak in Plasma framework's PackageUrlInterceptor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126443/ --- (Updated Dec. 21, 2015, 7:39 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Changes --- Submitted with commit 5c1d2ed01c9eb4f5c56990de8de6373f4f50c785 by Michael Pyne to branch master. Repository: plasma-framework Description --- Fixes a minor memleak in PackageUrlInterceptor (noted by Coverity, CID #1332370), by deleting d-ptr in existing destructor. Diffs - src/plasmaquick/packageurlinterceptor.cpp e49fc35 Diff: https://git.reviewboard.kde.org/r/126443/diff/ Testing --- Still builds. Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 126443: Fix memleak in Plasma framework's PackageUrlInterceptor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126443/ --- Review request for Plasma. Repository: plasma-framework Description --- Fixes a minor memleak in PackageUrlInterceptor (noted by Coverity, CID #1332370), by deleting d-ptr in existing destructor. Diffs - src/plasmaquick/packageurlinterceptor.cpp e49fc35 Diff: https://git.reviewboard.kde.org/r/126443/diff/ Testing --- Still builds. Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 126411: KF5::Plasma: Fix potential use-after-free in FrameSVG
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126411/ --- Review request for Plasma. Repository: plasma-framework Description --- Plasma framework's FrameSVG class uses cached regions for efficiency. However Coverity caught a mis-use of QCache in FrameSvg::mask(), which could lead to a use-after-free situation. (CID 1291560) Basically, any pointer passed into QCache::insert must be assumed to be deleted after insert() has been called -- we can't then return that pointer to the caller. Moreover we were simply returning a pointer to calling code that had been (and still would be) owned by QCache, which is unsafe as it can be deleted at any time. The fix in both cases is to make a local copy of the QRegion from out of the cache and return that. I didn't thoroughly look for other cases where we return cached pointers, or evaluate whether the performance impact from making local copies makes the cache less useful, this patch just fixes the immediate bug. Diffs - src/plasma/framesvg.cpp 107e0e6 Diff: https://git.reviewboard.kde.org/r/126411/diff/ Testing --- Everything compiles, Plasma shell boots up with the new code without issue, switching Plasma themes works fine. I checked for open bugs but didn't see anything obvious (that wasn't already closed years ago, at least). Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126411: KF5::Plasma: Fix potential use-after-free in FrameSVG
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126411/ --- (Updated Dec. 18, 2015, 2:36 a.m.) Status -- This change has been marked as submitted. Review request for Plasma. Changes --- Submitted with commit 600bdda04508fab512ed97c9874353378c7cb3fa by Michael Pyne to branch master. Repository: plasma-framework Description --- Plasma framework's FrameSVG class uses cached regions for efficiency. However Coverity caught a mis-use of QCache in FrameSvg::mask(), which could lead to a use-after-free situation. (CID 1291560) Basically, any pointer passed into QCache::insert must be assumed to be deleted after insert() has been called -- we can't then return that pointer to the caller. Moreover we were simply returning a pointer to calling code that had been (and still would be) owned by QCache, which is unsafe as it can be deleted at any time. The fix in both cases is to make a local copy of the QRegion from out of the cache and return that. I didn't thoroughly look for other cases where we return cached pointers, or evaluate whether the performance impact from making local copies makes the cache less useful, this patch just fixes the immediate bug. Diffs - src/plasma/framesvg.cpp 107e0e6 Diff: https://git.reviewboard.kde.org/r/126411/diff/ Testing --- Everything compiles, Plasma shell boots up with the new code without issue, switching Plasma themes works fine. I checked for open bugs but didn't see anything obvious (that wasn't already closed years ago, at least). Thanks, Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: KF 5.8 changelog
On Fri, March 13, 2015 19:36:29 Marco Martin wrote: On Friday 13 March 2015 19:15:21 Mark Gaiser wrote: To everybody(and myself): please follow this advice, on any mayor change especially if needs a bit of back and forth in reviews, let's try to add CHANGELOG: in the commit, mkay? Not just major changes. Basically all commits! not basically all commits case by case those that make sense. Or the noise makes the thinkg absolutely useless Isn't the feature intended to aid those preparing release announcements, as opposed to being fed directly to a webpage? In this case the noise would be annoying perhaps, but still better than the situation previously where it was difficult to come up with a promo-style release announcement in the first place. Our promo team can at least filter out the noise when they make the release announcement, and interested power users would likely appreciate the fuller list of changes. But it's hard to recover signal if we don't provide it, especially weeks or even months after the feature/bugfix was first committed. Still, it's a good point that we have to put thought into our CHANGELOG entries. CHANGELOG entries on every commit would be just as useless as not having those entries on any commit. Regards, - Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: kf5 alpha 1 : modules, versions
On Tue, February 11, 2014 20:59:24 David Faure wrote: On Monday 10 February 2014 15:43:13 Marco Martin wrote: On Monday 10 February 2014, Ivan Čukić wrote: the above points should be done.. only thing, in kactivities frameworks should still be merged in master (Ivan, would this be ok?) Fine by me :) (even more than 'fine') ok. i suppose kdesrc-build has to be updated beforehand tough to pick up the correct ones? (how?) Normally (in the future) this would be change kde-build-metadata/logical-module-structure. However kactivities was actually missing there, its branch name was hardcoded in the kdesrc-build file. So just update the file kf5-frameworks-build-include in the kdesrc-build repo. But still, adding an entry into logical-module-structure would be good for the future. I would argue we should do both: migrate the kf5-*-build-include in kdesrc- build to use branch-groups consistently and ensure that logical-module- structure is up to date. The kde-build-metadata module has a script to verify that you get the right branch name so that you don't have to run kdesrc-build to validate the JSON or ensure you got what you expected. I might do this myself in the next day or so if there's no opposition, as now is the easiest this transition is ever going to be, at least until KDE 4 development has well and truly stopped. Regards, - Michael Pyne ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Proposal for branching policy towards KF5
On Fri, July 19, 2013 00:21:21 you wrote: After more live discussion with Sebas and Marco plus Aaron over a video chat, we came up with the following setup for the workspace repos (*) : - the development branch for their next feature release (based on Qt5/KF5) will be master. - *before* this happens, however, kdesrc-build / kde-build-metadata / projects.kde.org will need to be improved so that tools (kdesrc-build and possibly build.kde.org) can automatically select the latest Qt4-based branch (i.e. master everywhere and 4.11 for the workspace repos), on demand. This would also be the opportunity to implement latest *stable* branch which is 4.11 for most modules right now, but could be at some point 4.12 for most and 4.11 for workspace repos. Adding a similar generic selection for qt5/kf5, we would end up giving 3 options to people who compile from sources: stable, latest-qt4, or qt5/kf5- based. First note: There's a lot of different mailing lists with at least some interest in this discussion, so I've mailed them all for informational purposes... but let's keep the discussion limited to the kde-core-devel mailing list! Back on topic, I have made an initial draft specification [1] for what this logical module group layer would look like. In addition, there is a sample JSON file in the kde-build-metadata git repository, called logical-module-structure that one can view to get a feel for the proposed syntax/semantics. I didn't want to write another parser, but JSON has no native comment support, so the documentation [1] is on community.kde.org (though perhaps that's for the best). For those with no clue what I'm talking about, the original thread from kde- core-devel is available from [2]. A point of concern is that currently we already have a concept similar to this, for i18n. It's possible to specify in the projects.kde.org management interface a stable or trunk branch for translation purposes. I don't know the translation infrastructure well enough to see how this proposal would impact that feature; I assume we'd want to move scripty ( friends) over to using this at some point if we go through with it, but it's probably easy enough to keep both techniques on whatever release checklist we're using now. At this point I think this still needs a green light from the release team, though. They are now CC'ed for review. One clarification I should make is that I also received a recommendation to investigate migrating our current dependency data into this new JSON file if possible. I put the effort into doing this as it would also help make the implementation of some kdesrc-build misfeatures relating to dependency-data additions a bit easier, as there's no need to construct an AST and a parser. Additionally it would permit 'soft' dependencies, which are useful for modules that can utilize optional features but don't have required dependencies on other git modules. However that can, and probably should, be considered separately (though I'll take comments now, if you have them). [1]: http://community.kde.org/Infrastructure/Project_Metadata [2]: http://markmail.org/message/4l3yqerga7mfiit5 Anyways, thanks for your interest and please let me know if this will work to solve the problem at hand. If so I will start on integrating within kdesrc- build, and working with the sysadmins to support within the continuous integration infrastructure. Regards, - Michael Pyne signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: ksplashx: Adjust erroneous CMake calls.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105468/#review16237 --- Ship it! If no one objects by Monday I'd just make the commit. It can always be reverted later if it causes an issue, and it sounds like it fixes several other problems that haven't become issues yet. - Michael Pyne On July 7, 2012, 3:07 a.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105468/ --- (Updated July 7, 2012, 3:07 a.m.) Review request for Build System and Plasma. Description --- ksplashx: Adjust erroneous CMake calls. Since its inception, ksplashx's CMakeLists.txt was using `include(FindFOO)' instead of `find_package(FOO)' to find libpng and libjpeg. While it has indeed worked so far, it is not the right way to find dependencies. Adjust this by correctly looking for libpng and libjpeg via find_package(). The calls are in the top-level CMakeLists.txt since putting all find_package() and macro_optional_find_package() calls in the same file seems to appease some packagers. While here, also add libjpeg's include directory to the compiler's include path and use a FindPNG variable that's not internal to do the same for libpng. Diffs - CMakeLists.txt ded5a2995b3d25c1c7c891afe93795fb0dfcfb87 ksplash/ksplashx/CMakeLists.txt 65447939dc9ac17c69f1e94afe935b37c7dceba4 Diff: http://git.reviewboard.kde.org/r/105468/diff/ Testing --- kde-workspace built fine both before and after this change Thanks, Raphael Kubo da Costa ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: [PATCH] Request review for KSharedDataCache (Round 2)
On Thursday, April 29, 2010 21:12:42 Michael Pyne wrote:
Well that [changeable layout] isn't what *I* wanted, but I see your point
given that precondition, and it occurs to me that the one having to touch
my code afterwards may want to change the layout, so I'll move it around
indeed.
For everyone's information, KSharedDataCache and KImageCache are now in trunk.
I'll give it a day or two for any last minute comments and port KIconLoader
over shortly.
For my Plasma dev friends, I also intend to port Plasma::Theme over to use
KImageCache, using the attached patch. Please let me know if there are
problems with this.
Regards,
- Michael Pyne
Index: plasma/theme.cpp
===
--- plasma/theme.cpp (revision 1118716)
+++ plasma/theme.cpp (working copy)
@@ -36,7 +36,7 @@
#include kglobal.h
#include kglobalsettings.h
#include kmanagerselection.h
-#include kpixmapcache.h
+#include kimagecache.h
#include ksharedconfig.h
#include kstandarddirs.h
#include kwindowsystem.h
@@ -149,7 +149,7 @@
QString defaultWallpaperSuffix;
int defaultWallpaperWidth;
int defaultWallpaperHeight;
-KPixmapCache *pixmapCache;
+KImageCache *pixmapCache;
KSharedConfigPtr svgElementsCache;
QHashQString, QSetQString invalidElements;
QHashQString, QPixmap pixmapsToCache;
@@ -176,9 +176,8 @@
bool ThemePrivate::useCache()
{
if (cacheTheme !pixmapCache) {
-pixmapCache = new KPixmapCache(plasma_theme_ + themeName);
ThemeConfig config;
-pixmapCache-setCacheLimit(config.themeCacheKb());
+pixmapCache = new KImageCache(plasma_theme_ + themeName, config.themeCacheKb() * 1024);
}
return cacheTheme;
@@ -232,7 +231,7 @@
void ThemePrivate::discardCache()
{
-KPixmapCache::deleteCache(plasma_theme_ + themeName);
+KSharedDataCache::deleteCache(plasma_theme_ + themeName);
delete pixmapCache;
pixmapCache = 0;
invalidElements.clear();
@@ -253,7 +252,7 @@
QHashIteratorQString, QPixmap it(pixmapsToCache);
while (it.hasNext()) {
it.next();
-pixmapCache-insert(idsToCache[it.key()], it.value());
+pixmapCache-insertPixmap(idsToCache[it.key()], it.value());
}
pixmapsToCache.clear();
@@ -480,7 +479,7 @@
QFileInfo info(f);
-if (useCache() info.lastModified().toTime_t() pixmapCache-timestamp()) {
+if (useCache() info.lastModified().toTime_t() pixmapCache-lastModifiedTime()) {
discardCache();
} else {
QString svgElementsFile = KStandardDirs::locateLocal(cache, plasma-svgelements- + themeName);
@@ -700,7 +699,12 @@
return true;
}
-return d-pixmapCache-find(key, pix);
+QPixmap temp = d-pixmapCache-findPixmap(key);
+if(!temp.isNull()) {
+pix = temp;
+return true;
+}
+return false;
}
return false;
@@ -709,7 +713,7 @@
// BIC FIXME: Should be merged with the other findInCache method above when we break BC
bool Theme::findInCache(const QString key, QPixmap pix, unsigned int lastModified)
{
-if (d-useCache() lastModified d-pixmapCache-timestamp()) {
+if (d-useCache() lastModified d-pixmapCache-lastModifiedTime()) {
return false;
}
@@ -719,7 +723,7 @@
void Theme::insertIntoCache(const QString key, const QPixmap pix)
{
if (d-useCache()) {
-d-pixmapCache-insert(key, pix);
+d-pixmapCache-insertPixmap(key, pix);
}
}
@@ -815,7 +819,9 @@
void Theme::setCacheLimit(int kbytes)
{
if (d-useCache()) {
-d-pixmapCache-setCacheLimit(kbytes);
+;
+// Too late for you bub.
+// d-pixmapCache-setCacheLimit(kbytes);
}
}
signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[PATCH] Make man:foo, info:foo, etc. work again from KRunner
Hi all,
Attached is a patch to fix bug 221371 (man: and info: syntax no longer
working) which is caused by a strengthening of the syntax checks done in
RunnerContext in kdelibs/plasma. It used to be that near the end of the chain
of checks if a context hadn't been defined that a KUrl would be constructed
and made either a network path or a local path.
This was changed a few weeks ago to only assume a path if there was a slash or
backslash, but this was done before the network check was made to.
This caused the context passed to the location runner to change from a
NetworkContext to an UnknownContext which stopped any matches from being
emitted (since these URLs don't match the possibilities allowed in
LocationRunner for UnknownContext URLs).
The attached patch changes the ambiguity test to be performed after the
network URL check to restore the old behavior while still not ambiguously
assuming local paths.
Note that it was always possible to workaround with man:/foo and info:/foo,
but the other style is frequently used as examples of cool things you can do
in KDE and are valid URLs anyways so I'd like that syntax to also work. I'd
also like to get this in before 4.4 so I would appreciate a prompt review if
possible so I can contact release-team.
Thanks!
Regards,
- Michael Pyne
Index: plasma/runnercontext.cpp
===
--- plasma/runnercontext.cpp (revision 1070191)
+++ plasma/runnercontext.cpp (working copy)
@@ -188,16 +188,17 @@
// that it has arguments!
type = (space 0) ? RunnerContext::ShellCommand :
RunnerContext::Executable;
-} else if (path.indexOf('/') == -1 path.indexOf('\\') == -1 ) {
-//if a path doesn't have any slashes is a single word or
-//sentence: is too ambiguous to be sure we're in a filesystem context
-return;
} else {
KUrl url(term);
QString correctCasePath;
+// check if we have a network location first, otherwise assume a path,
+// but if a path doesn't have any slashes is a single word or
+// sentence: it's too ambiguous to be sure we're in a filesystem context
if (!url.protocol().isEmpty() !url.isLocalFile()) {
type = RunnerContext::NetworkLocation;
-} else if (correctPathCase(path, correctCasePath)) {
+} else if ((path.indexOf('/') != -1 || path.indexOf('\\') != -1)
+ correctPathCase(path, correctCasePath))
+{ // multiline if, drop opening brace to make scope clear
path = correctCasePath;
QFileInfo info(path);
signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Incompatible change to libs/plasmaclock?
Hi all, Revision 942893 (http://websvn.kde.org/?view=revrevision=942893) broke the kdebase build for me (a rename of a .ui file). I would have just fixed it, but I noticed the offending line is a INSTALL entry in CMakeLists.txt. If we were installing the ui_.h file before (i.e. it's some kind of public interface) is it OK to just change the name all of a sudden? Or does this ui_calendarConfig.h file need installed at all (if not I'd just remove it I think). If you need any more info from me please CC me as I'm not subscribed. Regards, - Michael Pyne signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: Make web shortcut icons look better in krunner
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/47/ --- Review request for Plasma. Summary --- Right now the icons on some web shortcut searches look poor, (for instance, our love kde: shortcut gives me a question mark icon). I see that there is code to give a sane default for icons with no favicon, but it doesn't seem to work here. Using the kdelibs KMimeType instead of direct DBus calls seems to fix that. In addition since most favicons are small and look poor scaled up by the runner, I added the ability to obey the Icon= setting of the .desktop files defining a web shortcut. (For instance, try adding Icon=kde to the kde.desktop file in kdebase/runtime/kurifilter-plugins/ikws/searchproviders or installed at `kde4-config --install services`/searchproviders/kde.desktop. Now you get the very nice glassy KDE logo when you type kde:KFoo in krunner). Diffs - kdebase/workspace/plasma/runners/webshortcuts/webshortcutrunner.cpp 923467 Diff: http://reviewboard.kde.org/r/47/diff Testing --- I've tested gg: (which has a favicon), kde: (whose Icon= overrides) and imdb: (no favicon, nice default used instead of the question mark). Thanks, Michael ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Make web shortcut icons look better in krunner
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/47/ --- (Updated 2009-02-08 18:44:31.168430) Review request for Plasma. Summary --- Right now the icons on some web shortcut searches look poor, (for instance, our love kde: shortcut gives me a question mark icon). I see that there is code to give a sane default for icons with no favicon, but it doesn't seem to work here. Using the kdelibs KMimeType instead of direct DBus calls seems to fix that. In addition since most favicons are small and look poor scaled up by the runner, I added the ability to obey the Icon= setting of the .desktop files defining a web shortcut. (For instance, try adding Icon=kde to the kde.desktop file in kdebase/runtime/kurifilter-plugins/ikws/searchproviders or installed at `kde4-config --install services`/searchproviders/kde.desktop. Now you get the very nice glassy KDE logo when you type kde:KFoo in krunner). Diffs - kdebase/workspace/plasma/runners/webshortcuts/webshortcutrunner.cpp 923467 Diff: http://reviewboard.kde.org/r/47/diff Testing --- I've tested gg: (which has a favicon), kde: (whose Icon= overrides) and imdb: (no favicon, nice default used instead of the question mark). Screenshots --- How icons look with .desktop support http://reviewboard.kde.org/r/47/s/4/ Sites with favicons http://reviewboard.kde.org/r/47/s/5/ Sites without favicons http://reviewboard.kde.org/r/47/s/6/ Just noticed that dragging the KRunner gives a different icon with this patch though :( http://reviewboard.kde.org/r/47/s/7/ Thanks, Michael ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
