D5966: Fix race-condition in KRandom-seeding.

[Michael Pyne]

mpyne added a comment.


  Would it be better to use `QThreadStorage` (to record if the thread has 
been seeded) instead of a `QSet`?  It would allow us to more easily adopt 
C++11's `thread_local` once we remove support for older compilers.  I also 
looked into whether we could adopt the higher-quality RNGs that should be 
available in C++11, but getting them seeded is apparently very difficult to do 
without introducing errors.
  
  Also, I'd recommend using `quintptr` instead of `intptr_t` in the 
reinterpret_cast, as that seems more accurate for the semantics of XOR.  The 
shift to XOR is a great idea though.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D5966

To: tfry, dfaure
Cc: mpyne, tfry, rjvbb, #frameworks

D5972: Set (and unset, as necessary) QT_NO_EXCEPTIONS for Clang (and ICC)

[René J . V . Bertin]

rjvbb edited the summary of this revision.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D5972

To: rjvbb, #build_system, #frameworks
Cc: #frameworks, #build_system

D5972: Set (and unset, as necessary) QT_NO_EXCEPTIONS for Clang (and ICC)

[René J . V . Bertin]

rjvbb edited the summary of this revision.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D5972

To: rjvbb, #build_system, #frameworks
Cc: #frameworks, #build_system

Re: Review Request 130139: Set QT_NO_EXCEPTIONS for *.mm files

[René J . V . Bertin]

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



See also https://phabricator.kde.org/D5972

- René J.V. Bertin


On May 25, 2017, 1:37 p.m., Harald Fernengel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130139/
> ---
> 
> (Updated May 25, 2017, 1:37 p.m.)
> 
> 
> Review request for KDE Frameworks and René J.V. Bertin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> Seems that some versions of clang default to build *.mm files with
> exceptions disabled, so compilation would fail with:
> 
> include/QtCore/qlist.h:522:9: error: cannot use 'throw' with exceptions 
> disabled
> 
> 
> Diffs
> -
> 
>   src/kdeinit/CMakeLists.txt f00dd77baa95dbe7583e08760302951446aff387 
> 
> Diff: https://git.reviewboard.kde.org/r/130139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harald Fernengel
> 
>

D5972: Set (and unset, as necessary) QT_NO_EXCEPTIONS for Clang (and ICC)

[René J . V . Bertin]

rjvbb created this revision.
rjvbb added a project: Build System.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Qt currently fails to set `QT_NO_EXCEPTIONS` when building with Clang (and 
thus presumably with ICC too as it apparently masquerades as Clang).
  
  This patch corrects that omission which leads to issues like this one: 
https://git.reviewboard.kde.org/r/130139/
  
  I have attempted to added the corresponding unsetters to the file and target 
specific exceptions enabler macros. These changes have the intended effect on 
the compiler invocation, I have not double-checked if they also have the 
intended effect on the actual compilation.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D5972

AFFECTED FILES
  kde-modules/KDECompilerSettings.cmake

To: rjvbb, #build_system, #frameworks
Cc: #frameworks, #build_system

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

[René J . V . Bertin]

rjvbb added a comment.


  ping?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D5865

To: rjvbb, #frameworks, #build_system, cgilles
Cc: kfunk

D5971: Expose the symbolic 22px kde icon by moving it to kde-symbolic and symlinking to it

[Chris Holland]

Zren updated this revision to Diff 14841.
Zren added a comment.


  Apply to icons-dark too.

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5971?vs=14840&id=14841

REVISION DETAIL
  https://phabricator.kde.org/D5971

AFFECTED FILES
  icons-dark/apps/22/kde-symbolic.svg
  icons-dark/apps/22/kde.svg
  icons/apps/22/kde-symbolic.svg
  icons/apps/22/kde.svg

To: Zren
Cc: #frameworks

D5971: Expose the symbolic 22px kde icon by moving it to kde-symbolic and symlinking to it

[Chris Holland]

Zren created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  This lets panels >23px use the symbolic icon for their launcher. Right now 
the 48px version of kde.svg has a blue rounded rectangle background.
  
  An `apps/44/kde-symbolic.svg` doesn't appear to be necessary.
  
  F3764366: 2017-05-25___09-51-39.png 
  
  Should I create a new `apps/symbolic` folder instead?
  
  I need to duplicate my work in icons-dark too don't I?

TEST PLAN
  Ran
  
cd apps/22
git mv kde.svg kde-symbolic.svg
ln -s kde-symbolic.svg kde.svg
  
  Then restarted plasmashell.
  Then chose kde-symbolic for the launcher.

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D5971

AFFECTED FILES
  icons/apps/22/kde.svg
  icons/apps/symbolic/kde-symbolic.svg

To: Zren
Cc: #frameworks

Re: Review Request 130139: Set QT_NO_EXCEPTIONS for *.mm files

[René J . V . Bertin]

> On May 25, 2017, 2:26 p.m., René J.V. Bertin wrote:
> > src/kdeinit/CMakeLists.txt, line 6
> > 
> >
> > I think you'll need to investigate this issue further first. I've been 
> > building (and patching) KInit since 5.16.0 or so and never run into this 
> > issue (neither have the other testers of my to-be-official MacPorts ports).
> > 
> > I just checked (with AppleClang and macports-clang-4.0): all of KInit 
> > is built with `-fno-exceptions`, which has been the default for quite a 
> > while now (see KDECompilerSettings in the ECM). Yet for me QT_NO_EXCEPTIONS 
> > is NOT defined when I build kinit_mac.mm (nor when I include qlist.h and 
> > presumably not in the other files either).
> > 
> > Which raises a number of questions:
> > 1. why is QT_NO_EXCEPTIONS not required on my end, but it is on your 
> > end (or with HomeBrew's build recipes)?
> > 2. is this related to how Qt is built?
> > 3. why is only kinit_mac.mm concerned, or more generally, why would 
> > this be an ObjC++ issue?
> > 
> > Either way, the real issue here lies in KDE's disabling of exceptions, 
> > and Qt's lacking magic in qglobal.h (around line 632 in Qt 5.8.0) that 
> > defines QT_NO_EXCEPTIONS as needed when GCC is being used. I haven't 
> > verified it that test can be extended to clang too (it works with GCC on 
> > Mac) but it seems much more reasonable to define QT_NO_EXCEPTIONS in the 
> > ECM than as suggested here on a case-by-case basis.
> > 
> > -2 as is thus, as far as I'm concerned.
> 
> René J.V. Bertin wrote:
> To make things more interesting, here's the preprocessed QList method 
> where the error cited above originates:
> 
> ```
> template 
> inline typename QList::iterator QList::insert(iterator before, 
> const T &t)
> {
> do { } while ((false) && (isValidIterator(before)));
> 
> int iBefore = int(before.i - reinterpret_cast(p.begin()));
> Node *n = 0;
> if (d->ref.isShared())
> n = detach_helper_grow(iBefore, 1);
> else
> n = reinterpret_cast(p.insert(iBefore));
> try {
> node_construct(n, t);
> } catch (...) {
> p.remove(iBefore);
> throw;
> }
> return n;
> }
> ```
> 
> which shows that even on my end I should be getting an error when 
> compiling with `-fno-exceptions`.

https://bugreports.qt.io/browse/QTBUG-61034


- René J.V.


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


On May 25, 2017, 1:37 p.m., Harald Fernengel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130139/
> ---
> 
> (Updated May 25, 2017, 1:37 p.m.)
> 
> 
> Review request for KDE Frameworks and René J.V. Bertin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> Seems that some versions of clang default to build *.mm files with
> exceptions disabled, so compilation would fail with:
> 
> include/QtCore/qlist.h:522:9: error: cannot use 'throw' with exceptions 
> disabled
> 
> 
> Diffs
> -
> 
>   src/kdeinit/CMakeLists.txt f00dd77baa95dbe7583e08760302951446aff387 
> 
> Diff: https://git.reviewboard.kde.org/r/130139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harald Fernengel
> 
>

Re: Review Request 130139: Set QT_NO_EXCEPTIONS for *.mm files

[René J . V . Bertin]

> On May 25, 2017, 2:26 p.m., René J.V. Bertin wrote:
> > src/kdeinit/CMakeLists.txt, line 6
> > 
> >
> > I think you'll need to investigate this issue further first. I've been 
> > building (and patching) KInit since 5.16.0 or so and never run into this 
> > issue (neither have the other testers of my to-be-official MacPorts ports).
> > 
> > I just checked (with AppleClang and macports-clang-4.0): all of KInit 
> > is built with `-fno-exceptions`, which has been the default for quite a 
> > while now (see KDECompilerSettings in the ECM). Yet for me QT_NO_EXCEPTIONS 
> > is NOT defined when I build kinit_mac.mm (nor when I include qlist.h and 
> > presumably not in the other files either).
> > 
> > Which raises a number of questions:
> > 1. why is QT_NO_EXCEPTIONS not required on my end, but it is on your 
> > end (or with HomeBrew's build recipes)?
> > 2. is this related to how Qt is built?
> > 3. why is only kinit_mac.mm concerned, or more generally, why would 
> > this be an ObjC++ issue?
> > 
> > Either way, the real issue here lies in KDE's disabling of exceptions, 
> > and Qt's lacking magic in qglobal.h (around line 632 in Qt 5.8.0) that 
> > defines QT_NO_EXCEPTIONS as needed when GCC is being used. I haven't 
> > verified it that test can be extended to clang too (it works with GCC on 
> > Mac) but it seems much more reasonable to define QT_NO_EXCEPTIONS in the 
> > ECM than as suggested here on a case-by-case basis.
> > 
> > -2 as is thus, as far as I'm concerned.

To make things more interesting, here's the preprocessed QList method where the 
error cited above originates:

```
template 
inline typename QList::iterator QList::insert(iterator before, const T &t)
{
do { } while ((false) && (isValidIterator(before)));

int iBefore = int(before.i - reinterpret_cast(p.begin()));
Node *n = 0;
if (d->ref.isShared())
n = detach_helper_grow(iBefore, 1);
else
n = reinterpret_cast(p.insert(iBefore));
try {
node_construct(n, t);
} catch (...) {
p.remove(iBefore);
throw;
}
return n;
}
```

which shows that even on my end I should be getting an error when compiling 
with `-fno-exceptions`.


- René J.V.


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


On May 25, 2017, 1:37 p.m., Harald Fernengel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130139/
> ---
> 
> (Updated May 25, 2017, 1:37 p.m.)
> 
> 
> Review request for KDE Frameworks and René J.V. Bertin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> Seems that some versions of clang default to build *.mm files with
> exceptions disabled, so compilation would fail with:
> 
> include/QtCore/qlist.h:522:9: error: cannot use 'throw' with exceptions 
> disabled
> 
> 
> Diffs
> -
> 
>   src/kdeinit/CMakeLists.txt f00dd77baa95dbe7583e08760302951446aff387 
> 
> Diff: https://git.reviewboard.kde.org/r/130139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harald Fernengel
> 
>

D5970: Fix KSNIs being unable to register service under flatpak

[David Edmundson]

davidedmundson created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Rather than registering a custom service and then passing that to the
  watcher's RegisterItem we can just pass the base service as a name.
  
  As we're not registering random names it works on Flatpak. 
  Replaces https://phabricator.kde.org/D5911

TEST PLAN
  Ran the existing test with --ksni-count 5
  Still had all my items appear

REPOSITORY
  R289 KNotifications

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5970

AFFECTED FILES
  src/kstatusnotifieritemdbus_p.cpp
  src/kstatusnotifieritemdbus_p.h

To: davidedmundson, #frameworks

Re: Review Request 130139: Set QT_NO_EXCEPTIONS for *.mm files

[René J . V . Bertin]

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




src/kdeinit/CMakeLists.txt (line 6)


I think you'll need to investigate this issue further first. I've been 
building (and patching) KInit since 5.16.0 or so and never run into this issue 
(neither have the other testers of my to-be-official MacPorts ports).

I just checked (with AppleClang and macports-clang-4.0): all of KInit is 
built with `-fno-exceptions`, which has been the default for quite a while now 
(see KDECompilerSettings in the ECM). Yet for me QT_NO_EXCEPTIONS is NOT 
defined when I build kinit_mac.mm (nor when I include qlist.h and presumably 
not in the other files either).

Which raises a number of questions:
1. why is QT_NO_EXCEPTIONS not required on my end, but it is on your end 
(or with HomeBrew's build recipes)?
2. is this related to how Qt is built?
3. why is only kinit_mac.mm concerned, or more generally, why would this be 
an ObjC++ issue?

Either way, the real issue here lies in KDE's disabling of exceptions, and 
Qt's lacking magic in qglobal.h (around line 632 in Qt 5.8.0) that defines 
QT_NO_EXCEPTIONS as needed when GCC is being used. I haven't verified it that 
test can be extended to clang too (it works with GCC on Mac) but it seems much 
more reasonable to define QT_NO_EXCEPTIONS in the ECM than as suggested here on 
a case-by-case basis.

-2 as is thus, as far as I'm concerned.


- René J.V. Bertin


On May 25, 2017, 1:37 p.m., Harald Fernengel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130139/
> ---
> 
> (Updated May 25, 2017, 1:37 p.m.)
> 
> 
> Review request for KDE Frameworks and René J.V. Bertin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> Seems that some versions of clang default to build *.mm files with
> exceptions disabled, so compilation would fail with:
> 
> include/QtCore/qlist.h:522:9: error: cannot use 'throw' with exceptions 
> disabled
> 
> 
> Diffs
> -
> 
>   src/kdeinit/CMakeLists.txt f00dd77baa95dbe7583e08760302951446aff387 
> 
> Diff: https://git.reviewboard.kde.org/r/130139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harald Fernengel
> 
>

Review Request 130139: Set QT_NO_EXCEPTIONS for *.mm files

[Harald Fernengel]

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

Review request for KDE Frameworks and René J.V. Bertin.


Repository: kinit


Description
---

Seems that some versions of clang default to build *.mm files with
exceptions disabled, so compilation would fail with:

include/QtCore/qlist.h:522:9: error: cannot use 'throw' with exceptions disabled


Diffs
-

  src/kdeinit/CMakeLists.txt f00dd77baa95dbe7583e08760302951446aff387 

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


Testing
---


Thanks,

Harald Fernengel

Review Request 130138: Set QT_NO_EXCEPTIONS when building *.mm files

[Harald Fernengel]

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

Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.


Repository: sonnet


Description
---

Apparently, exceptions are disabled when building *.mm files.
The alternative would be to set

target_compile_options(sonnet_nsspellchecker PRIVATE "-fexceptions")

in order to force exception to be on for *.mm files. However, since
exceptions aren't used in the code, let's just build without.


Diffs
-

  src/plugins/nsspellchecker/CMakeLists.txt 
f8c99d1ea080b1e28f60c57f8f588ed059c96711 

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


Testing
---


Thanks,

Harald Fernengel

D5912: minimize dialog resizes/moves

[Marco Martin]

mart added a comment.


  In https://phabricator.kde.org/D5912#111657, @davidedmundson wrote:
  
  > but: we're still doing a resize whislt we have the old window management 
min/max hints set from the previous item, so it'll still have an extra resize, 
just at a kwin level not here.
  
  
  where are you getting them? now, i still i get an extra resize when switching 
maintitem in taskbar tooltips, not on other tooltips that share the same 
mainitem

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D5912

To: mart, #plasma, hein, davidedmundson
Cc: broulik, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas

D5912: minimize dialog resizes/moves

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:32e67a591edb: minimize dialog resizes/moves (authored by 
mart).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D5912?vs=14809&id=14834#toc

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5912?vs=14809&id=14834

REVISION DETAIL
  https://phabricator.kde.org/D5912

AFFECTED FILES
  src/declarativeimports/core/tooltipdialog.cpp
  src/plasmaquick/dialog.cpp

To: mart, #plasma, hein, davidedmundson
Cc: broulik, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas

D5966: Fix race-condition in KRandom-seeding.

[Thomas Friedrichsmeier]

tfry updated this revision to Diff 14831.
tfry added a comment.


  Ensure proper per-thread random-seeding in KRandom::random().
  
  As commented, the problem is somewhat different, than I though, originally:
  
  qsrand() and qrand() keep random seeds per QThread, and each thread needs a 
separate call
  to qsrand(). Without this patch, qsrand() would only be called for the first 
thread to call
  KRandom::random(). Any other thread will effectively use qsrand(1).
  
  Note that prior to 
https://phabricator.kde.org/R244:9ae6d765b37135bbfe3a8b936e5a88b8a435e424 
srand() and rand() were used.
  Those are not properly thread-safe, but at least this would not result in 
threads using a
  fixed seed.

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5966?vs=14824&id=14831

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5966

AFFECTED FILES
  src/lib/randomness/krandom.cpp

To: tfry, dfaure
Cc: tfry, rjvbb, #frameworks