D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > tfry wrote in krandomtest.cpp:178 > If the results are not unique, some of the results will already be in the > set, and so results.insert() does not increase the size of the set. Only if > each thread result is unique, the set size will correspon

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread Thomas Friedrichsmeier
tfry added inline comments. INLINE COMMENTS > dfaure wrote in krandomtest.cpp:168 > variable size array, which is not in the standard. > https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard > > Make size static to fix it. Sorry. Should have waited

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread David Faure
dfaure added a comment. No - not with this syntax. See the stackoverflow discussion I posted, or https://stackoverflow.com/questions/31645309/can-i-use-a-c-variable-length-array-in-c03-and-c11?rq=1 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D5966 To: tfry, d

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-29 Thread Tobias C. Berner
tcberner updated this revision to Diff 14965. tcberner added a comment. Use QStorageInfo. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5784?vs=14334&id=14965 BRANCH master REVISION DETAIL https://phabricator.kde.org/D5784 AFFECTED FILES src/engine/

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread Thiago Macieira
thiago added a comment. In https://phabricator.kde.org/D5865#112747, @rjvbb wrote: > In https://phabricator.kde.org/D5865#112743, @thiago wrote: > > > Fix the source code. > > > > If certain third-party libraries can't compile under MSVC 2015 (or even 2017!) with default compiler

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread René J . V . Bertin
rjvbb added a comment. In https://phabricator.kde.org/D5865#112743, @thiago wrote: > Fix the source code. > > If certain third-party libraries can't compile under MSVC 2015 (or even 2017!) with default compiler options, they don't deserve to be used. Blacklist them. That may

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread Albert Astals Cid
aacid added inline comments. INLINE COMMENTS > dfaure wrote in krandomtest.cpp:168 > variable size array, which is not in the standard. > https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard > > Make size static to fix it. They are in C++11, no? R

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread Thiago Macieira
thiago added a comment. In https://phabricator.kde.org/D5865#112737, @rjvbb wrote: > In https://phabricator.kde.org/D5865#112697, @thiago wrote: > > > Why are we spending time in named operators? It's much easier to fix the source code not to use them and then the problem goes away.

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread René J . V . Bertin
rjvbb added a comment. In https://phabricator.kde.org/D5865#112697, @thiago wrote: > Why are we spending time in named operators? It's much easier to fix the source code not to use them and then the problem goes away. This is about adding a convenience macro for projects that have

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > krandomtest.cpp:168 > +const int size = 5; > +KRandomTestThread* threads[size]; > +for (int i=0; i < size; ++i) { variable size array, which is not in the standard. https://stackoverflow.com/questions/1887097/why-arent-variable-length-a

D6017: Don't leak MimeData object

2017-05-29 Thread David Edmundson
davidedmundson created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY A DeclarativeDropArea creates a new DeclarativeDragDropEvent on every enter/move/leave event. The getter me

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread Thiago Macieira
thiago added a comment. In https://phabricator.kde.org/D5865#112679, @rjvbb wrote: > > That option only exists in MSVC 2017. > > Should we have deduce that from the docs I cite in the CMake comments? I'm willing to believe that I misread as far as support for named operators is conce

D5784: Add support for FreeBSD in FSUtils::getDirectoryFileSystem().

2017-05-29 Thread Tobias C. Berner
tcberner added a comment. Ok, I think I found the issue in QStorageInfo -- it calls getmntinfo() with `flags=0` [1] . Instead one of the valid arguments 1-4 [2]. [1] https://github.com/qt/qtbase/blob/dev/src/corelib/io/qstorageinfo_unix.cpp#L199 [2] https://svnweb.freebsd.org/base/hea

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread René J . V . Bertin
rjvbb added a comment. I have no official reference for Qt requiring C++11, but I'm pretty sure I've seen the remark made on a mailing list or one of their code review tickets. I would have guessed that was with 5.7 but it can just as well be 5.8 and later. A GCC >= 4.8 *probably* correspond

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread René J . V . Bertin
rjvbb added a comment. > That option only exists in MSVC 2017. Should we have deduce that from the docs I cite in the CMake comments? I'm willing to believe that I misread as far as support for named operators is concerned, but they do mention 2015 Update 3 specifically as the appearance

D4911: add Baloo DBus signals for moved or removed files

2017-05-29 Thread Matthieu Gallien
mgallien updated this revision to Diff 14953. mgallien added a comment. add a new method to register applications wanting to watch moved files and define (via xml) the interface to implement for applications REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D49

KDE CI: Frameworks kcoreaddons kf5-qt5 XenialQt5.7 - Build # 12 - Still Unstable!

2017-05-29 Thread no-reply
BUILD UNSTABLE Build URL https://build-sandbox.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20XenialQt5.7/12/ Project: Frameworks kcoreaddons kf5-qt5 XenialQt5.7 Date of build: Mon, 29 May 2017 18:22:08 + Build duration: 3 min 40 sec and counting JUn

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread Thomas Friedrichsmeier
This revision was automatically updated to reflect the committed changes. Closed by commit R244:26a262180155: Ensure proper per thread seeding in KRandom. (authored by tfry). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D5966?vs=14863&id=14952#toc REPOSITORY R244 KCoreAddons CHANGES

D5966: Fix race-condition in KRandom-seeding.

2017-05-29 Thread Michael Pyne
mpyne added a comment. The changeset is fine to go in (once the duplicated semicolon is fixed). As for C++11, I'm not aware that Qt requires C++11 as a blanket policy. Instead they require specific compilers, and then we can use the C++11 features supported by that compiler. Accord

D6016: Don't warn when attempting to remove a MainWindow container

2017-05-29 Thread Elvis Angelaccio
elvisangelaccio added a comment. hmm, this is what I did in ark: https://phabricator.kde.org/R36:e932b113b2d9ee7b792c23057ea2e15f8ea4f9c5 REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D6016 To: elvisangelaccio, dfaure Cc: #frameworks

D6016: Don't warn when attempting to remove a MainWindow container

2017-05-29 Thread David Faure
dfaure added a comment. Containers are menus and toolbars, how can we possibly get here with a mainwindow as container? The fix doesn't looks horrible but I'm curious as to why this never happened before, what does ark do that nobody else ever did? Got a bt? REPOSITORY R263 KXmlGui REVIS

D6016: Don't warn when attempting to remove a MainWindow container

2017-05-29 Thread Elvis Angelaccio
elvisangelaccio created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY MainWindow is a valid element that can contain other elements. We shouldn't print a warning in this case. TEST PLAN latest ark g

D5983: Check error status after every PolKitAuthority usage

2017-05-29 Thread Sergey Kalinichev
skalinichev added a comment. In https://phabricator.kde.org/D5983#112386, @aacid wrote: > Have you had these errors happen to you? > > How can one reproduce them? > > Or is it more of a "this should be the right code but i don't know how to trigger it" case? Yes, for me it

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread Thiago Macieira
thiago added a comment. In https://phabricator.kde.org/D5865#112549, @rjvbb wrote: > If a compiler is problematic in general it may not be worth it trying to account for it in a macro, trying to coerce it into doing something it cannot handle. I don't think that's a reason not to impleme

D6014: Small improvements in IconItem

2017-05-29 Thread Aleix Pol Gonzalez
This revision was automatically updated to reflect the committed changes. Closed by commit R242:3b303d736317: Small improvements in IconItem (authored by apol). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6014?vs=14943&id=14949 REVISION

D6014: Small improvements in IconItem

2017-05-29 Thread Marco Martin
mart accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D6014 To: apol, #frameworks, #plasma, mart Cc: plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-moha

D6014: Small improvements in IconItem

2017-05-29 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 14943. apol added a comment. Few additions REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6014?vs=14939&id=14943 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6014 AFFECTED FILES sr

D5611: [RenameDialog] Enforce plain text format

2017-05-29 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes. Closed by commit R241:6b7115ce7016: [RenameDialog] Enforce plain text format (authored by broulik). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5611?vs=13859&id=14941 REVISION DETAIL h

D6014: Small improvements in IconItem

2017-05-29 Thread Aleix Pol Gonzalez
apol created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Don't construct a QUrl for every source strings. Check it's a file url first (which is the only kind of url we support at the moment)

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread René J . V . Bertin
rjvbb marked an inline comment as done. rjvbb added a comment. So, how do we proceed? When I asked other devs what compiler they used to build their MSWin bundles they all (both) said they can't use MSVC anyway because it's too buggy. If a compiler is problematic in general it may no

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread Fabian Vogt
fvogt added a comment. In https://phabricator.kde.org/D6002#112526, @dfaure wrote: > Then isExecutable() is wrong API, it misses the check for the executable bit. But that's non trivial to fix (check if callers would have the info, deprecate method and add replacement which takes more in

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread David Faure
dfaure added a comment. Then isExecutable() is wrong API, it misses the check for the executable bit. But that's non trivial to fix (check if callers would have the info, deprecate method and add replacement which takes more info as input...). REPOSITORY R241 KIO REVISION DETAIL https:/

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread Fabian Vogt
This revision was automatically updated to reflect the committed changes. Closed by commit R241:4122b52fee54: Identify PIE binaries (application/x-sharedlib) as executable files (authored by fvogt). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6002?vs=14928&id=1

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread David Faure
dfaure accepted this revision. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D6002 To: fvogt, dfaure, aacid Cc: asturmlechner, #frameworks

D6008: Give a parent to KMoreToolsMenuFactory menus

2017-05-29 Thread Elvis Angelaccio
elvisangelaccio added a comment. Note: we could also deprecate the current createMenuFromGroupingNames() in favor of a new createMenuFromGroupingNames() that takes an addition QWidget* argument. Not sure which way is the best. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricato

D6008: Give a parent to KMoreToolsMenuFactory menus

2017-05-29 Thread Elvis Angelaccio
elvisangelaccio created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY A QMenu without a parent will be wrongly positioned on Wayland. TEST PLAN KMoreToolsMenuFactory menu in Dolphin status bar now wor

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-29 Thread Kevin Funk
kfunk requested changes to this revision. kfunk added a comment. This revision now requires changes to proceed. Can't go in as-is, as it breaks compilation on MSVC 2015. But interesting to see that `/Za` is actually problematic. Which proves my point: Just don't use named operators in cod

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread Fabian Vogt
fvogt added a comment. In https://phabricator.kde.org/D6002#112407, @dfaure wrote: > This is a workaround for https://bugs.freedesktop.org/show_bug.cgi?id=97226 but indeed, there seems to be no other way. > > Maybe add a link to that bug report in one of the uses of x-sharedlib in yo

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread Fabian Vogt
fvogt updated this revision to Diff 14928. fvogt added a comment. Add a comment referencing the fdo bug report REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6002?vs=14914&id=14928 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6002 AFFECTED

D6002: Identify PIE binaries (application/x-sharedlib) as executable files

2017-05-29 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. This is a workaround for https://bugs.freedesktop.org/show_bug.cgi?id=97226 but indeed, there seems to be no other way. Maybe add a link to that bug report in one of the uses of x-share