Re: Review Request 120287: [OS X] make kde-workspace build

[Rolf Eike Beer]

Am Freitag, 19. September 2014, 22:57:40 schrieb René J.V. Bertin:
> > On Sept. 20, 2014, 12:26 a.m., Christoph Feck wrote:
> > > You added APPLE to the if() but not always to the matching endif()...
> 
> True. But that's optional, no?

The endif needs to have either a matching argument to the last if or elseif, 
or an entirely empty argument.

Eike

signature.asc
Description: This is a digitally signed message part.

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 12:26 a.m., Christoph Feck wrote:
> > You added APPLE to the if() but not always to the matching endif()...
> 
> René J.V. Bertin wrote:
> True. But that's optional, no?

On Saturday September 20 2014, Rolf Eike Beer wrote regarding "Re: Review 
Request 120287: [OS X] make kde-workspace build"

>The endif needs to have either a matching argument to the last if or elseif, 
>or an entirely empty argument.

OK, I'd never noticed issues when not doing so, I'm pretty sure not even when 
testing the changed files on Linux (which is a bit why I stopped being strict 
about it).

Does the same apply to `else`? I'm not sure that repeating the same complex 
argument list improves readability all the time, so would it b acceptable if I 
put an empty argument in places that get too cluttered otherwise?


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

> On Sept. 19, 2014, 10:24 nachm., Martin Gräßlin wrote:
> > overall I rather tend to -1 for these changes. I consider changing the 
> > build system in a long term release as way too risky considering that the 
> > core development doesn't use this iteration any more. Any unintended 
> > breakage (e.g. a component also not being built on X11) would probably not 
> > be spotted and that's something we certainly don't want to do in a minor 
> > version.
> > 
> > Given that we are already in a late state of the bug fix only development 
> > it should not be difficult to keep the patch downstream as a 
> > distro-specific patch.
> 
> René J.V. Bertin wrote:
> That's re: the changes to the CMakefile? I frankly don't see the risk. I 
> took care to piggyback most changes to those files on WIN32 conditionals, and 
> there are no APPLEs running Qt/X11 that could run this kde-workspace versin. 
> But checking for false positives on Linux is easy enough (I can try to do 
> that over the weekend). Besides, core development may not be using "this 
> iteration" anymore, but that doesn't mean no one else would spot a 
> hypothetical regression...
> 
> Ben Cooksley wrote:
> Martin, can you please clarify how changing an if( NOT WIN32 ) to if( NOT 
> WIN32 AND NOT APPLE ) would make a difference to platforms which are not 
> Windows or Apple based?
> 
> We've been trying to encourage upstreaming of patches from both the 
> Windows and Mac builds to make it easier to build these directly from source. 
> This is particularly crucial with Plasma Next / KF5 / etc.

> can you please clarify how changing an if( NOT WIN32 ) to if( NOT WIN32 AND 
> NOT APPLE ) would make a difference to platforms which are not Windows or 
> Apple based?

which it isn't. There are also changes in the structure with now more if-else. 
To me each of them is a risky change as the whole thing is no longer tested.

Personally I always weight the risk of regression against the probably gain for 
each patch going into the stable branch. For our main platforms in this case 
there is no gain, only the risk for regression. So with my Linux developer hat 
on, I don't like the idea of it being touched.

Also I do not understand why one would want kde-workspace on MacOS. With 
Windows maintainers we just concluded that it doesn't make sense to offer it 
and the new split repositories don't support building for Windows any more. 
Given that I just fail to see what would be the gain of having kde-workspace 
being built on MacOS or what advantages this would give to users on said 
platform. E.g. there is now the launch KCM being built on MacOS. Who will 
benefit by that? It's a KCM to configure the behavior of X11 startup 
notifications. The settings are read by KRunner, the taskmanager and KWin. All 
three are excluded from building, though. Fonts KCM is not being built, but 
fontinstall is. Again it probably will build, but it's completely useless - I 
have seen the X11 dependency of it when trying to use it on Wayland. krdb is 
being built, although it's so X11 specific that there is now a huge #ifdef 
Q_WS_X11.

While I appreciate the work on making our software work on more systems, I 
highly doubt there is need in making the LTS kde-workspace module build on 
MacOS. I think it would be better to look at the now split repositories and 
provide those which make sense without needing to introduce risky changes.


- Martin


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


On Sept. 20, 2014, 12:05 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

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



CMakeLists.txt


tabs instead of whitespaces



CMakeLists.txt


tabs instead of whitespaces



CMakeLists.txt


tabs instead of whitespaces



kcontrol/krdb/krdb.cpp


added newline



libs/CMakeLists.txt


why is that Darwin and all other cases are APPLE?


- Martin Gräßlin


On Sept. 20, 2014, 12:05 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

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


While we're discussing krdb's function on non-X11 systems:

I seem to recall that the colour theme selector kcm had an effect like it has 
under X11: applying the new theme to running applications instantly. I can no 
longer reproduce this. My work on getting this package to OS X has been a very 
informal approach. It's only been the other day that I realised it could have 
some value for the transition to KF5, and rounded up everything, tested with 
the latest kde-workspace version and then made this RR. 
So my recollection can be wrong, and it could even be that it's expectable that 
KDE/Mac doesn't even respect all colour choices for all GUI elements.

For instance, I'm now seeing that simple applications like systemsettings or 
kwalletmanager ignore (almost) all colour choices, kate and kdevelop use them 
for the edit views but not lists/frames and digikam uses almost all selected 
colours (but that app has a builtin theme selector).

I've reverted to kde-workspace 4.11.9 (the one from KDE 4.12.5; MacPorts keeps 
snapshots one can switch back and forth between) and as far as I can tell those 
observations are not the result of my changes to krdb.cpp .

Thomas seemed knowledgable about how Qt4 works on OS X; to what extent is this 
to be expected, and to what extent might we be able to improve this? (If 
improvement it is on a platform that basically allows only the text selection 
highlight colour to be customised...)
What (missing) things might I look for aside updates to kdeglobal.rc (which I 
realise I haven't even done yet)?

- René J.V. Bertin


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > 
> >
> > tabs instead of whitespaces

I looked into those yesterday, and cannot find tabs on my end. No idea where 
they'd come from.


> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > libs/CMakeLists.txt, line 10
> > 
> >
> > why is that Darwin and all other cases are APPLE?

Good one. Look just a bit down from that location, you'll see another 
"Darwinism" that was there when I set out changing. I guess the line 10 change 
was one of the 1st I made, and I never got around to changing it to an APPLE 
when I made my later changes.

It's probably better actually to standardise to APPLE, because there *is* 
another OS that uses (or might use) the Darwin keyword (OpenDarwin, 
PureDarwin), and that OS will probably use X11. Not sure it'll ever gain 
momentum or even run Qt, but best pick a single keyword right now, no?


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

> On Sept. 20, 2014, 10:20 vorm., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > 
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.

I'd guess the editor you use changes it somehow. I'd suggest to use Kate and 
enable show all whitespaces.


- Martin


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


On Sept. 20, 2014, 12:05 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

> On Sept. 20, 2014, 10:25 vorm., René J.V. Bertin wrote:
> > While we're discussing krdb's function on non-X11 systems:
> > 
> > I seem to recall that the colour theme selector kcm had an effect like it 
> > has under X11: applying the new theme to running applications instantly. I 
> > can no longer reproduce this. My work on getting this package to OS X has 
> > been a very informal approach. It's only been the other day that I realised 
> > it could have some value for the transition to KF5, and rounded up 
> > everything, tested with the latest kde-workspace version and then made this 
> > RR. 
> > So my recollection can be wrong, and it could even be that it's expectable 
> > that KDE/Mac doesn't even respect all colour choices for all GUI elements.
> > 
> > For instance, I'm now seeing that simple applications like systemsettings 
> > or kwalletmanager ignore (almost) all colour choices, kate and kdevelop use 
> > them for the edit views but not lists/frames and digikam uses almost all 
> > selected colours (but that app has a builtin theme selector).
> > 
> > I've reverted to kde-workspace 4.11.9 (the one from KDE 4.12.5; MacPorts 
> > keeps snapshots one can switch back and forth between) and as far as I can 
> > tell those observations are not the result of my changes to krdb.cpp .
> > 
> > Thomas seemed knowledgable about how Qt4 works on OS X; to what extent is 
> > this to be expected, and to what extent might we be able to improve this? 
> > (If improvement it is on a platform that basically allows only the text 
> > selection highlight colour to be customised...)
> > What (missing) things might I look for aside updates to kdeglobal.rc (which 
> > I realise I haven't even done yet)?

just wondering: shouldn't the color settings be picked from the MacOS settings?


- Martin


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


On Sept. 20, 2014, 12:05 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

> On Sept. 20, 2014, 10:20 vorm., Martin Gräßlin wrote:
> > libs/CMakeLists.txt, line 10
> > 
> >
> > why is that Darwin and all other cases are APPLE?
> 
> René J.V. Bertin wrote:
> Good one. Look just a bit down from that location, you'll see another 
> "Darwinism" that was there when I set out changing. I guess the line 10 
> change was one of the 1st I made, and I never got around to changing it to an 
> APPLE when I made my later changes.
> 
> It's probably better actually to standardise to APPLE, because there *is* 
> another OS that uses (or might use) the Darwin keyword (OpenDarwin, 
> PureDarwin), and that OS will probably use X11. Not sure it'll ever gain 
> momentum or even run Qt, but best pick a single keyword right now, no?

yes, I think standardise on one keywoards would be good. Though I do not know 
which one is the best.


- Martin


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


On Sept. 20, 2014, 12:05 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline

Ah, the famous German rigor ... is this really an issue, shouldn't that empty 
line have been there in the 1st place?!


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

> On Sept. 20, 2014, 10:20 vorm., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline
> 
> René J.V. Bertin wrote:
> Ah, the famous German rigor ... is this really an issue, shouldn't that 
> empty line have been there in the 1st place?!

yes of course. It's a change which has nothing to do with the overall change. 
It's not atomic any more.


- Martin


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


On Sept. 20, 2014, 12:05 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 8:20 vorm., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline
> 
> René J.V. Bertin wrote:
> Ah, the famous German rigor ... is this really an issue, shouldn't that 
> empty line have been there in the 1st place?!
> 
> Martin Gräßlin wrote:
> yes of course. It's a change which has nothing to do with the overall 
> change. It's not atomic any more.

> shouldn't that empty line have been there in the 1st place?!

c++98 newline @ EOF, later versions don't, ie. it's not required to add that 
line despite you may get warnings about it (and actually kate has an option to 
automatically add it on saving)

Do you use xcode?


- Thomas


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


On Sept. 19, 2014, 10:05 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 19, 2014, 10:05 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 8:20 vorm., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline
> 
> René J.V. Bertin wrote:
> Ah, the famous German rigor ... is this really an issue, shouldn't that 
> empty line have been there in the 1st place?!
> 
> Martin Gräßlin wrote:
> yes of course. It's a change which has nothing to do with the overall 
> change. It's not atomic any more.
> 
> Thomas Lübking wrote:
> > shouldn't that empty line have been there in the 1st place?!
> 
> c++98 newline @ EOF, later versions don't, ie. it's not required to add 
> that line despite you may get warnings about it (and actually kate has an 
> option to automatically add it on saving)
> 
> Do you use xcode?

c++98 *required* newline @ EOF ...


- Thomas


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


On Sept. 19, 2014, 10:05 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 19, 2014, 10:05 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:25 a.m., René J.V. Bertin wrote:
> > While we're discussing krdb's function on non-X11 systems:
> > 
> > I seem to recall that the colour theme selector kcm had an effect like it 
> > has under X11: applying the new theme to running applications instantly. I 
> > can no longer reproduce this. My work on getting this package to OS X has 
> > been a very informal approach. It's only been the other day that I realised 
> > it could have some value for the transition to KF5, and rounded up 
> > everything, tested with the latest kde-workspace version and then made this 
> > RR. 
> > So my recollection can be wrong, and it could even be that it's expectable 
> > that KDE/Mac doesn't even respect all colour choices for all GUI elements.
> > 
> > For instance, I'm now seeing that simple applications like systemsettings 
> > or kwalletmanager ignore (almost) all colour choices, kate and kdevelop use 
> > them for the edit views but not lists/frames and digikam uses almost all 
> > selected colours (but that app has a builtin theme selector).
> > 
> > I've reverted to kde-workspace 4.11.9 (the one from KDE 4.12.5; MacPorts 
> > keeps snapshots one can switch back and forth between) and as far as I can 
> > tell those observations are not the result of my changes to krdb.cpp .
> > 
> > Thomas seemed knowledgable about how Qt4 works on OS X; to what extent is 
> > this to be expected, and to what extent might we be able to improve this? 
> > (If improvement it is on a platform that basically allows only the text 
> > selection highlight colour to be customised...)
> > What (missing) things might I look for aside updates to kdeglobal.rc (which 
> > I realise I haven't even done yet)?
> 
> Martin Gräßlin wrote:
> just wondering: shouldn't the color settings be picked from the MacOS 
> settings?

Well, that's a question one could ask. Apart from the fact that leaves us with 
about just the text highlight colour setting, it all depends to what extent we 
want to impose the OS X look and feel. If we do, though, we also ought to make 
it impossible to pick a style other than the "Macintosh" style Qt offers, even 
if the user selected another style in qtconfig or whatever replacement Qt5 has.
And if we do, that also raises the question to what extent we should or should 
not drop my request to make Qt's menu-meddling heuristics optional (= guessing 
what menu actions should go under the Preferences (and About) menu items of 
Apple's application menu instead of in the author-specified menu).

To be honest, I think we should not. I am a firm believer in user choice and 
not in knowing better what kind of look and feel s/he prefers. I also think 
that one of the (perceived) benefits of running KDE applications on OS X could 
well be the very fact that they allow more adjusting to personal taste than 
native OS X apps. And since we're not imposing anything (in my vision), nothing 
keeps a user from making KDE applications look like OS X applications. When I 
submitted a MacPorts port for the QtCurve style I even threw in theme and 
colour settings files to help achieve that.

Oh, and concerning the Macintosh style: I don't use it. Somehow it doesn't feel 
right, at least not on OS X 10.6 . With that style, Qt applications look ... 
dated, more so than native OS X applications. And the layout is much less 
compact, which may explain why I find the look dated, but which is also a waste 
of screen space (even if you have a Retina display ...)


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline
> 
> René J.V. Bertin wrote:
> Ah, the famous German rigor ... is this really an issue, shouldn't that 
> empty line have been there in the 1st place?!
> 
> Martin Gräßlin wrote:
> yes of course. It's a change which has nothing to do with the overall 
> change. It's not atomic any more.
> 
> Thomas Lübking wrote:
> > shouldn't that empty line have been there in the 1st place?!
> 
> c++98 newline @ EOF, later versions don't, ie. it's not required to add 
> that line despite you may get warnings about it (and actually kate has an 
> option to automatically add it on saving)
> 
> Do you use xcode?
> 
> Thomas Lübking wrote:
> c++98 *required* newline @ EOF ...

But this is not @EOF, it's a newline inserted to detach the 1st function from 
the last #include statement (or rather, the last conditional #ifdef Q_WS_X11 
include).

Given that that's an X11 conditional include, I don't think that cosmetic 
change (adding a newline) is completely irrelevant either.

@Thomas: yes, I use Xcode, but not here. CMake can generate Xcode projects, but 
that only works for relatively simple ones, so it's not even feasible to use 
Xcode. Most of the time I use KDevelop (git/kde4-legacy) because of the access 
to the documentation it gives. I can give less guarantee that I didn't edit the 
file in vi at some point, though ;)

[OT]: there's a Google plugin for Xcode that removes white space from empty 
lines and I think also the final newline. And I do use Xcode for my OS X 
Keychain work from time to time, because KDevelop can't access the native SDK 
documentation.[/OT]


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)

[Thomas Lübking]

> On Sept. 18, 2014, 9:19 vorm., Thomas Lübking wrote:
> > drkonqi/main.cpp, line 47
> > 
> >
> > this sounds fishy - at least the comment to be incorrect?
> > i hope that OSX does not just actually abort() when you call setuid() 
> > but that indeed the tests fail and the applications exits(255)?
> > 
> > In case of the latter, does the process itself run with suid until this 
> > point?
> > 
> > I assume if we've to consider that drkonqi does not (require) to run 
> > suid, the test should be omitted if "geteuid()" (notice the "e"!) isn't 0.
> > 
> > Skipping this altogether only makes sense for broken by design 
> > operating systems which fail to confirm to posix standards (windows ;-)
> 
> Ian Wadham wrote:
> I am pretty sure Apple OS X does just abort Dr Konqi. It considers use of 
> setuid/setgid a security breach (it calls it "setugid"). It is part of new 
> security rules that came into OS X about 4 versions ago.
> 
> The question is moot, because I am not attempting to run Dr Konqi via 
> kdeinit4 any more, only by forking (see review 119497).
> 
> So I propose to settle the issue by removing the Q_OS_MAC condition. I 
> intend to leave in the comment, however, to remind me to do something at this 
> point if I ever get the help I have asked for with the many problems in 
> kdeinit4 and friends on Apple OS X, or if the methods of running Dr K from 
> KCrash change in KF5. I heard a rumour that kdeinit is to be dropped in KF5.
> 
> All the tests I did on this in July showed that the crashing app, 
> kdeinit4 and Dr Konqi were all running as the logged-in user and no actual 
> setting of uid or gid was needed. They would just set things to what they 
> were before. Also none of the executable files had any special permission 
> bits set. Nevertheless, a few lines later, Apple OS X kicks Dr Konqi off the 
> machine somehow.
> 
> FWIW the Apple OS X console log said, back in July:
> 
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: ENTERING 
> KCrash::defaultCrashHandler (1623294600)...
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: crashing... 
> crashRecursionCounter = 2
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Application Name 
> = palapeli path = /Applications/kde4.13/palapeli.app/Contents/MacOS pid = 
> 14165
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Arguments: 
> /Applications/kde4.13/palapeli.app/Contents/MacOS/palapeli --nocrashhandler 
> -psn_0_327760 
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Attempting to 
> start 
> /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi from 
> kdeinit
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: Connect 
> sock_file=/kdedev/kde4.13/home/.kde4.13/socket-Tara.local/kdeinit4__tmp_launch-KdDfgS_org.x_0
> 22/07/14 4:30:34.451 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: Got 
> EXEC_NEW 
> '/kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi' 
> from wrapper.
> 22/07/14 4:30:34.451 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: 
> preparing to launch 
> /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi
> 22/07/14 4:30:34.545 PM [0x0-0x4f04f].org.kde.kdeinit4: objc[14167]: 
> Object 0x7fc6cb64e5e0 of class NSPathStore2 autoreleased with no pool in 
> place - just leaking - break on objc_autoreleaseNoPool() to debug
> 22/07/14 4:30:34.545 PM [0x0-0x4f04f].org.kde.kdeinit4: objc[14167]: 
> Object 0x7fc6cb64e660 of class NSPathStore2 autoreleased with no pool in 
> place - just leaking - break on objc_autoreleaseNoPool() to debug
> 22/07/14 4:30:34.546 PM drkonqi: The application with bundle ID  is 
> running setugid(), which is not allowed.
> 22/07/14 4:30:34.546 PM [0x0-0x4f04f].org.kde.kdeinit4: 2014-07-22 
> 16:30:34.545 drkonqi[14167:2503] The application with bundle ID  is running 
> setugid(), which is not allowed.
> 22/07/14 4:30:34.549 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: PID 
> 14167 terminated.
> 
> René J.V. Bertin wrote:
> Ian, do you think this could in any way be related to the fact that one 
> must do "certain permissions-related things" in order the be able to use a 
> non-Apple-provided debugger? (Which in turn might have something to do with 
> preventing too easy reverse-engineering and other hacker business?)
> 
> Ian Wadham wrote:
> No. Dr K works fine if you start it by forking, including the uid/gid 
> stuff. And I am pretty sure MacPorts implements a way to provide access to 
> debuggers of all stripes. That came up on macports-dev list a few months ago.
> 
> René J.V. Bertin wrote:
> Yes, it (MacPorts) does. But one that involves something like a 
> code-signing certificate, IIRC. In other words, it seems to be linked to the 
> executable. OTOH, Dr K launches a standalone debugger, so as long a

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 8:20 vorm., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline
> 
> René J.V. Bertin wrote:
> Ah, the famous German rigor ... is this really an issue, shouldn't that 
> empty line have been there in the 1st place?!
> 
> Martin Gräßlin wrote:
> yes of course. It's a change which has nothing to do with the overall 
> change. It's not atomic any more.
> 
> Thomas Lübking wrote:
> > shouldn't that empty line have been there in the 1st place?!
> 
> c++98 newline @ EOF, later versions don't, ie. it's not required to add 
> that line despite you may get warnings about it (and actually kate has an 
> option to automatically add it on saving)
> 
> Do you use xcode?
> 
> Thomas Lübking wrote:
> c++98 *required* newline @ EOF ...
> 
> René J.V. Bertin wrote:
> But this is not @EOF, it's a newline inserted to detach the 1st function 
> from the last #include statement (or rather, the last conditional #ifdef 
> Q_WS_X11 include).
> 
> Given that that's an X11 conditional include, I don't think that cosmetic 
> change (adding a newline) is completely irrelevant either.
> 
> @Thomas: yes, I use Xcode, but not here. CMake can generate Xcode 
> projects, but that only works for relatively simple ones, so it's not even 
> feasible to use Xcode. Most of the time I use KDevelop (git/kde4-legacy) 
> because of the access to the documentation it gives. I can give less 
> guarantee that I didn't edit the file in vi at some point, though ;)
> 
> [OT]: there's a Google plugin for Xcode that removes white space from 
> empty lines and I think also the final newline. And I do use Xcode for my OS 
> X Keychain work from time to time, because KDevelop can't access the native 
> SDK documentation.[/OT]

Indeed - then this change is completely unrelated and pointless.

Yes, there probably *should* have been a newline in the first place, but it's 
actually irrelevant whether there is and such changes don't belong into this 
kind of patch.

> I don't think that cosmetic change (adding a newline) is completely 
> irrelevant either.

The point is that this is a purely cosmetic change, unrelated to the rest of 
your patch.
You are right, that one would demand a newline in *new code* here, but it 
doesn't belong in your patch.

If you wish to tidy up aged code for KF5, you're very welcome =)

Here's for vim:
https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Vim

For kate, "configure editor..."
/"Appearance"
   o) Highlight Tabs
   o) Highlight trailign spaces
   o) Show indention lines
/"editing"/"indention"
   o) Spaces
   Tab width: 4 chars
   Indention width: 4 chars
/"Open/Save"
   Remove Trailing space: on modified lines
) Append newline at end of file


- Thomas


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


On Sept. 19, 2014, 10:05 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 19, 2014, 10:05 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)

[René J . V . Bertin]

> On Sept. 18, 2014, 11:19 a.m., Thomas Lübking wrote:
> > drkonqi/main.cpp, line 47
> > 
> >
> > this sounds fishy - at least the comment to be incorrect?
> > i hope that OSX does not just actually abort() when you call setuid() 
> > but that indeed the tests fail and the applications exits(255)?
> > 
> > In case of the latter, does the process itself run with suid until this 
> > point?
> > 
> > I assume if we've to consider that drkonqi does not (require) to run 
> > suid, the test should be omitted if "geteuid()" (notice the "e"!) isn't 0.
> > 
> > Skipping this altogether only makes sense for broken by design 
> > operating systems which fail to confirm to posix standards (windows ;-)
> 
> Ian Wadham wrote:
> I am pretty sure Apple OS X does just abort Dr Konqi. It considers use of 
> setuid/setgid a security breach (it calls it "setugid"). It is part of new 
> security rules that came into OS X about 4 versions ago.
> 
> The question is moot, because I am not attempting to run Dr Konqi via 
> kdeinit4 any more, only by forking (see review 119497).
> 
> So I propose to settle the issue by removing the Q_OS_MAC condition. I 
> intend to leave in the comment, however, to remind me to do something at this 
> point if I ever get the help I have asked for with the many problems in 
> kdeinit4 and friends on Apple OS X, or if the methods of running Dr K from 
> KCrash change in KF5. I heard a rumour that kdeinit is to be dropped in KF5.
> 
> All the tests I did on this in July showed that the crashing app, 
> kdeinit4 and Dr Konqi were all running as the logged-in user and no actual 
> setting of uid or gid was needed. They would just set things to what they 
> were before. Also none of the executable files had any special permission 
> bits set. Nevertheless, a few lines later, Apple OS X kicks Dr Konqi off the 
> machine somehow.
> 
> FWIW the Apple OS X console log said, back in July:
> 
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: ENTERING 
> KCrash::defaultCrashHandler (1623294600)...
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: crashing... 
> crashRecursionCounter = 2
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Application Name 
> = palapeli path = /Applications/kde4.13/palapeli.app/Contents/MacOS pid = 
> 14165
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Arguments: 
> /Applications/kde4.13/palapeli.app/Contents/MacOS/palapeli --nocrashhandler 
> -psn_0_327760 
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Attempting to 
> start 
> /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi from 
> kdeinit
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: Connect 
> sock_file=/kdedev/kde4.13/home/.kde4.13/socket-Tara.local/kdeinit4__tmp_launch-KdDfgS_org.x_0
> 22/07/14 4:30:34.451 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: Got 
> EXEC_NEW 
> '/kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi' 
> from wrapper.
> 22/07/14 4:30:34.451 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: 
> preparing to launch 
> /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi
> 22/07/14 4:30:34.545 PM [0x0-0x4f04f].org.kde.kdeinit4: objc[14167]: 
> Object 0x7fc6cb64e5e0 of class NSPathStore2 autoreleased with no pool in 
> place - just leaking - break on objc_autoreleaseNoPool() to debug
> 22/07/14 4:30:34.545 PM [0x0-0x4f04f].org.kde.kdeinit4: objc[14167]: 
> Object 0x7fc6cb64e660 of class NSPathStore2 autoreleased with no pool in 
> place - just leaking - break on objc_autoreleaseNoPool() to debug
> 22/07/14 4:30:34.546 PM drkonqi: The application with bundle ID  is 
> running setugid(), which is not allowed.
> 22/07/14 4:30:34.546 PM [0x0-0x4f04f].org.kde.kdeinit4: 2014-07-22 
> 16:30:34.545 drkonqi[14167:2503] The application with bundle ID  is running 
> setugid(), which is not allowed.
> 22/07/14 4:30:34.549 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: PID 
> 14167 terminated.
> 
> René J.V. Bertin wrote:
> Ian, do you think this could in any way be related to the fact that one 
> must do "certain permissions-related things" in order the be able to use a 
> non-Apple-provided debugger? (Which in turn might have something to do with 
> preventing too easy reverse-engineering and other hacker business?)
> 
> Ian Wadham wrote:
> No. Dr K works fine if you start it by forking, including the uid/gid 
> stuff. And I am pretty sure MacPorts implements a way to provide access to 
> debuggers of all stripes. That came up on macports-dev list a few months ago.
> 
> René J.V. Bertin wrote:
> Yes, it (MacPorts) does. But one that involves something like a 
> code-signing certificate, IIRC. In other words, it seems to be linked to the 
> executable. OTOH, Dr K launches a standalone debugger, so as long a

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > kcontrol/krdb/krdb.cpp, line 56
> > 
> >
> > added newline
> 
> René J.V. Bertin wrote:
> Ah, the famous German rigor ... is this really an issue, shouldn't that 
> empty line have been there in the 1st place?!
> 
> Martin Gräßlin wrote:
> yes of course. It's a change which has nothing to do with the overall 
> change. It's not atomic any more.
> 
> Thomas Lübking wrote:
> > shouldn't that empty line have been there in the 1st place?!
> 
> c++98 newline @ EOF, later versions don't, ie. it's not required to add 
> that line despite you may get warnings about it (and actually kate has an 
> option to automatically add it on saving)
> 
> Do you use xcode?
> 
> Thomas Lübking wrote:
> c++98 *required* newline @ EOF ...
> 
> René J.V. Bertin wrote:
> But this is not @EOF, it's a newline inserted to detach the 1st function 
> from the last #include statement (or rather, the last conditional #ifdef 
> Q_WS_X11 include).
> 
> Given that that's an X11 conditional include, I don't think that cosmetic 
> change (adding a newline) is completely irrelevant either.
> 
> @Thomas: yes, I use Xcode, but not here. CMake can generate Xcode 
> projects, but that only works for relatively simple ones, so it's not even 
> feasible to use Xcode. Most of the time I use KDevelop (git/kde4-legacy) 
> because of the access to the documentation it gives. I can give less 
> guarantee that I didn't edit the file in vi at some point, though ;)
> 
> [OT]: there's a Google plugin for Xcode that removes white space from 
> empty lines and I think also the final newline. And I do use Xcode for my OS 
> X Keychain work from time to time, because KDevelop can't access the native 
> SDK documentation.[/OT]
> 
> Thomas Lübking wrote:
> Indeed - then this change is completely unrelated and pointless.
> 
> Yes, there probably *should* have been a newline in the first place, but 
> it's actually irrelevant whether there is and such changes don't belong into 
> this kind of patch.
> 
> > I don't think that cosmetic change (adding a newline) is completely 
> irrelevant either.
> 
> The point is that this is a purely cosmetic change, unrelated to the rest 
> of your patch.
> You are right, that one would demand a newline in *new code* here, but it 
> doesn't belong in your patch.
> 
> If you wish to tidy up aged code for KF5, you're very welcome =)
> 
> Here's for vim:
> https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Vim
> 
> For kate, "configure editor..."
> /"Appearance"
>o) Highlight Tabs
>o) Highlight trailign spaces
>o) Show indention lines
> /"editing"/"indention"
>o) Spaces
>Tab width: 4 chars
>Indention width: 4 chars
> /"Open/Save"
>Remove Trailing space: on modified lines
> ) Append newline at end of file

Well, if said German rigor has it come to that, I'll remove the newline. But 
than I'll also keep from changing the Darwin references to APPLE in the CMake 
files.


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 119497: Report crashes of KDE apps in Apple OS X (1) (fix kcrash)

[Thomas Lübking]

> On Juli 29, 2014, 3:33 nachm., Aleix Pol Gonzalez wrote:
> > kinit/kinit.cpp, line 1481
> > 
> >
> > do you need $DISPLAY in OS X?
> 
> René J.V. Bertin wrote:
> Nope. It can be set if the user has XQuartz installed and running, but 
> that shouldn't make a difference.
> 
> In fact, $DISPLAY shouldn't be used on OS X because we wouldn't want 
> things like socket names change when the user starts or quits XQuartz with 
> KDE apps and/or services running.
> 
> Ian Wadham wrote:
> Perish the thought ($DISPLAY fluctuating between a value and an empty 
> string). On my system, OS X 10.7.5 (Lion), XQuartz was installed by Apple and 
> $DISPLAY is always set to a value, even if XQuartz (X11.app) is not running 
> (i.e. no X11 server running). I presume installing X11 somehow "doctors" the 
> OS X (Darwin) startup procedures so that DISPLAY is already defined in every 
> user's ~/.profile. I do not know if that would be the case if a FOSS version 
> of XQuartz would be installed (e.g. on OS X 10.9.x Mavericks).
> 
> The big thing is that $DISPLAY -is- used (non-portably) in KDE to help 
> name a socket in kdeinit4 (kinit.cpp), wrapper.c, kwrapper and KCrash - and 
> FAIK it may be used in this way in several other places. If $DISPLAY is not 
> used consistently in all those places, communication with kdeinit4 can break, 
> as it does already in KCrash/kdeinit4 on Apple OS X. And THAT is what we are 
> trying to fix here. KDE apps on Apple OS X MUST be able to report a crash 
> reliably.
> 
> This is why I keep asking for help from a KDE core developer. How 
> widespread is this problem in KDE on Apple OS X? What are the implication for 
> KDE apps? Should references to $DISPLAY in KDE be eliminated from the OS X 
> version? What do kdeinit4, kwrapper and klauncher really do? Is whatever they 
> do actually portable to Apple OS X? Or are we all, KDE guys and MacPorts guys 
> alike, just crossing our fingers and hoping for the best?
> 
> I tried using lxr.kde.org to find further references, but there are 
> thousands: mostly because "display" is a commonly-used English word in 
> programming. And I did tick the case-sensitive box, but it does not seem to 
> work.
> 
> René J.V. Bertin wrote:
> Hmm, interesting. I should find some time to play with this in my 10.9 VM.
> Know however that even if $DISPLAY is always set, it will not always have 
> the same value. At least for me, XQuartz has the annoying habit of increasing 
> the display number after a restart.
> 
> If it's too complicated to remove all references to $DISPLAY from KDE 
> code (which wouldn't surprise me at all) there remains another approach. 
> Identify an appropriate location in the startup/initialisation code that all 
> KDE applications and services share, and set $DISPLAY to something sensible 
> (but preferably NOT a valid X11 display specifier). The only possible 
> undesirable side-effect I can see from here would be that shells in konsole 
> risk to inherit that value.
> This probably isn't the most elegant solution, but then again that's 
> because KDE apparently never imposed the use of its own internal 
> variable/function (or one from Qt) instead of directly querying $DISPLAY.
> 
> Ian Wadham wrote:
> Restart of what? My system (Lion) has Apple OS X's XQuartz and $DISPLAY 
> has a random temp-file path prepended every time Apple OS X starts of 
> restarts. And that path is different every time. But so what? $DISPLAY keeps 
> the same value no matter how many times I start XQuartz (X11) by running Gimp 
> or whatever. And that value, determined immediately after boot or restart, 
> should be picked by all KDE components, which come into existence later.
> 
> René J.V. Bertin wrote:
> I meant restarts of the X server - it does crash sometimes, sometimes I 
> have to restart it for other reasons, like after logging off and back in. I 
> recall that I used to have those weird (socket based?) $DISPLAY values too, 
> but now they're of a perfectly normal host:X.Y form on my systems. Except 
> that X tends to increase each time I start XQuartz. I ignore how common this 
> is, but I think that if you're looking into the use of $DISPLAY on OS X, you 
> could just as well take all possible situations into account. I'd suggest to 
> use the fact that the actual value is irrelevant and without important to 
> clamp it to something appropriate (why not Qt4:Mac) in such a way that all 
> those younger components pick up that value. And not the actual, current 
> value of $DISPLAY, which may or may not have remained unchanged. (I 
> specifically used the term clamp to draw an analogy signal acquisition, where 
> unconnected inputs can carry all kinds of bogus signals.)
> 
> René J.V. Bertin wrote:
> I've looked into the $DISPLAY value variations mentioned (= described 
> from memory) above. The

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > 
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.

No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in vi 
(which doesn't modify anything unless I ask for it) and there are no tabs to be 
found. Same in KDevelop when I open the patch review tool. I wonder if your 
editor isn't the culprit, or something on reviewboard?

I copied the .reviewboardrc file from another KDE project and edited the 
obvious settings. I see nothing concerning tabs in there, but maybe there ought 
to be?


- René J.V.


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


On Sept. 20, 2014, 12:05 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:05 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)

[Thomas Lübking]

> On Sept. 18, 2014, 9:19 vorm., Thomas Lübking wrote:
> > drkonqi/main.cpp, line 47
> > 
> >
> > this sounds fishy - at least the comment to be incorrect?
> > i hope that OSX does not just actually abort() when you call setuid() 
> > but that indeed the tests fail and the applications exits(255)?
> > 
> > In case of the latter, does the process itself run with suid until this 
> > point?
> > 
> > I assume if we've to consider that drkonqi does not (require) to run 
> > suid, the test should be omitted if "geteuid()" (notice the "e"!) isn't 0.
> > 
> > Skipping this altogether only makes sense for broken by design 
> > operating systems which fail to confirm to posix standards (windows ;-)
> 
> Ian Wadham wrote:
> I am pretty sure Apple OS X does just abort Dr Konqi. It considers use of 
> setuid/setgid a security breach (it calls it "setugid"). It is part of new 
> security rules that came into OS X about 4 versions ago.
> 
> The question is moot, because I am not attempting to run Dr Konqi via 
> kdeinit4 any more, only by forking (see review 119497).
> 
> So I propose to settle the issue by removing the Q_OS_MAC condition. I 
> intend to leave in the comment, however, to remind me to do something at this 
> point if I ever get the help I have asked for with the many problems in 
> kdeinit4 and friends on Apple OS X, or if the methods of running Dr K from 
> KCrash change in KF5. I heard a rumour that kdeinit is to be dropped in KF5.
> 
> All the tests I did on this in July showed that the crashing app, 
> kdeinit4 and Dr Konqi were all running as the logged-in user and no actual 
> setting of uid or gid was needed. They would just set things to what they 
> were before. Also none of the executable files had any special permission 
> bits set. Nevertheless, a few lines later, Apple OS X kicks Dr Konqi off the 
> machine somehow.
> 
> FWIW the Apple OS X console log said, back in July:
> 
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: ENTERING 
> KCrash::defaultCrashHandler (1623294600)...
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: crashing... 
> crashRecursionCounter = 2
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Application Name 
> = palapeli path = /Applications/kde4.13/palapeli.app/Contents/MacOS pid = 
> 14165
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Arguments: 
> /Applications/kde4.13/palapeli.app/Contents/MacOS/palapeli --nocrashhandler 
> -psn_0_327760 
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: KCrash: Attempting to 
> start 
> /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi from 
> kdeinit
> 22/07/14 4:30:34.451 PM [0x0-0x50050].palapeli: Connect 
> sock_file=/kdedev/kde4.13/home/.kde4.13/socket-Tara.local/kdeinit4__tmp_launch-KdDfgS_org.x_0
> 22/07/14 4:30:34.451 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: Got 
> EXEC_NEW 
> '/kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi' 
> from wrapper.
> 22/07/14 4:30:34.451 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: 
> preparing to launch 
> /kdedev/kde4.13/kde4/lib/kde4/libexec/drkonqi.app/Contents/MacOS/drkonqi
> 22/07/14 4:30:34.545 PM [0x0-0x4f04f].org.kde.kdeinit4: objc[14167]: 
> Object 0x7fc6cb64e5e0 of class NSPathStore2 autoreleased with no pool in 
> place - just leaking - break on objc_autoreleaseNoPool() to debug
> 22/07/14 4:30:34.545 PM [0x0-0x4f04f].org.kde.kdeinit4: objc[14167]: 
> Object 0x7fc6cb64e660 of class NSPathStore2 autoreleased with no pool in 
> place - just leaking - break on objc_autoreleaseNoPool() to debug
> 22/07/14 4:30:34.546 PM drkonqi: The application with bundle ID  is 
> running setugid(), which is not allowed.
> 22/07/14 4:30:34.546 PM [0x0-0x4f04f].org.kde.kdeinit4: 2014-07-22 
> 16:30:34.545 drkonqi[14167:2503] The application with bundle ID  is running 
> setugid(), which is not allowed.
> 22/07/14 4:30:34.549 PM [0x0-0x4f04f].org.kde.kdeinit4: kdeinit4: PID 
> 14167 terminated.
> 
> René J.V. Bertin wrote:
> Ian, do you think this could in any way be related to the fact that one 
> must do "certain permissions-related things" in order the be able to use a 
> non-Apple-provided debugger? (Which in turn might have something to do with 
> preventing too easy reverse-engineering and other hacker business?)
> 
> Ian Wadham wrote:
> No. Dr K works fine if you start it by forking, including the uid/gid 
> stuff. And I am pretty sure MacPorts implements a way to provide access to 
> debuggers of all stripes. That came up on macports-dev list a few months ago.
> 
> René J.V. Bertin wrote:
> Yes, it (MacPorts) does. But one that involves something like a 
> code-signing certificate, IIRC. In other words, it seems to be linked to the 
> executable. OTOH, Dr K launches a standalone debugger, so as long a

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

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

(Updated Sept. 20, 2014, 12:51 p.m.)


Review request for KDE Software on Mac OS X and kde-workspace.


Changes
---

includes the latest fixes


Repository: kde-workspace


Description
---

A few rather straightforward patches to make the relevant bits of KDE4's 
kde-workspace build and function on OS X.
The main interest is having the systemsettings control panel to control the 
various relevant KDE settings among which desktop search, fonts, colours and 
even style.
The oxygen style builds and looks good but shows some updating glitches due to 
compositing.

I'm submitting this patch partly in hope it may be useful in bringing 
kf5-workspace to OS X, one day.


Diffs (updated)
-

  CMakeLists.txt 195f99c 
  kcontrol/CMakeLists.txt fc666b1 
  kcontrol/krdb/krdb.cpp 36fc99c 
  kcontrol/style/CMakeLists.txt d832b20 
  libs/CMakeLists.txt c0576fe 

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


Testing
---

On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
git/master, 4.14.1).


Thanks,

René J.V. Bertin

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

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

(Updated Sept. 20, 2014, 12:54 p.m.)


Review request for KDE Software on Mac OS X and kde-workspace.


Repository: kde-workspace


Description
---

A few rather straightforward patches to make the relevant bits of KDE4's 
kde-workspace build and function on OS X.
The main interest is having the systemsettings control panel to control the 
various relevant KDE settings among which desktop search, fonts, colours and 
even style.
The oxygen style builds and looks good but shows some updating glitches due to 
compositing.

I'm submitting this patch partly in hope it may be useful in bringing 
kf5-workspace to OS X, one day.


Diffs
-

  CMakeLists.txt 195f99c 
  kcontrol/CMakeLists.txt fc666b1 
  kcontrol/krdb/krdb.cpp 36fc99c 
  kcontrol/style/CMakeLists.txt d832b20 
  libs/CMakeLists.txt c0576fe 

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


Testing
---

On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
git/master, 4.14.1).


File Attachments


copy of the diff file saved locally, which had no tabs when I uploaded it. 
Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch


Thanks,

René J.V. Bertin

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

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

(Updated Sept. 20, 2014, 12:53 p.m.)


Review request for KDE Software on Mac OS X and kde-workspace.


Changes
---

in reaction to the tab character mystery: proof they are not on my end. Just to 
be sure, the SHA256 checksum is 
3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 .


Repository: kde-workspace


Description
---

A few rather straightforward patches to make the relevant bits of KDE4's 
kde-workspace build and function on OS X.
The main interest is having the systemsettings control panel to control the 
various relevant KDE settings among which desktop search, fonts, colours and 
even style.
The oxygen style builds and looks good but shows some updating glitches due to 
compositing.

I'm submitting this patch partly in hope it may be useful in bringing 
kf5-workspace to OS X, one day.


Diffs
-

  CMakeLists.txt 195f99c 
  kcontrol/CMakeLists.txt fc666b1 
  kcontrol/krdb/krdb.cpp 36fc99c 
  kcontrol/style/CMakeLists.txt d832b20 
  libs/CMakeLists.txt c0576fe 

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


Testing
---

On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
git/master, 4.14.1).


File Attachments (updated)


copy of the diff file saved locally, which had no tabs when I uploaded it
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch


Thanks,

René J.V. Bertin

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
> No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in 
> vi (which doesn't modify anything unless I ask for it) and there are no tabs 
> to be found. Same in KDevelop when I open the patch review tool. I wonder if 
> your editor isn't the culprit, or something on reviewboard?
> 
> I copied the .reviewboardrc file from another KDE project and edited the 
> obvious settings. I see nothing concerning tabs in there, but maybe there 
> ought to be?

See my local copy of the diff I just added.


- René J.V.


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


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 8:20 vorm., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
> No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in 
> vi (which doesn't modify anything unless I ask for it) and there are no tabs 
> to be found. Same in KDevelop when I open the patch review tool. I wonder if 
> your editor isn't the culprit, or something on reviewboard?
> 
> I copied the .reviewboardrc file from another KDE project and edited the 
> obvious settings. I see nothing concerning tabs in there, but maybe there 
> ought to be?
> 
> René J.V. Bertin wrote:
> See my local copy of the diff I just added.

There're no tabs in the diff (checked teh only reliable way: okteta ;-)

Indention in those files is crazy (2 or 3 chars, never 4), what makes it look 
like WS/tab intermix, but ">>>" seems only a RB indication that the line was 
altered only by shifting.


- Thomas


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


On Sept. 20, 2014, 10:54 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 10:54 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

levant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
> No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in 
> vi (which doesn't modify anything unless I ask for it) and there are no tabs 
> to be found. Same in KDevelop when I open the patch review tool. I wonder if 
> your editor isn't the culprit, or something on reviewboard?
> 
> I copied the .reviewboardrc file from another KDE project and edited the 
> obvious settings. I see nothing concerning tabs in there, but maybe there 
> ought to be?
> 
> René J.V. Bertin wrote:
> See my local copy of the diff I just added.
> 
> Thomas Lübking wrote:
> There're no tabs in the diff (checked teh only reliable way: okteta ;-)
> 
> Indention in those files is crazy (2 or 3 chars, never 4), what makes it 
> look like WS/tab intermix, but ">>>" seems only a RB indication that the line 
> was altered only by shifting.

Just a question: would I still have triggered `-pedantic` mode if I had 
included something like "and relevant code tidy-ups" in my RR title? :P

Also, is one supposed to go through an RR for each and every tidy-up one might 
want to commit (now that I have commit access ...)?


- René J.V.


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


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 8:20 vorm., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
> No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in 
> vi (which doesn't modify anything unless I ask for it) and there are no tabs 
> to be found. Same in KDevelop when I open the patch review tool. I wonder if 
> your editor isn't the culprit, or something on reviewboard?
> 
> I copied the .reviewboardrc file from another KDE project and edited the 
> obvious settings. I see nothing concerning tabs in there, but maybe there 
> ought to be?
> 
> René J.V. Bertin wrote:
> See my local copy of the diff I just added.
> 
> Thomas Lübking wrote:
> There're no tabs in the diff (checked teh only reliable way: okteta ;-)
> 
> Indention in those files is crazy (2 or 3 chars, never 4), what makes it 
> look like WS/tab intermix, but ">>>" seems only a RB indication that the line 
> was altered only by shifting.
> 
> René J.V. Bertin wrote:
> Just a question: would I still have triggered `-pedantic` mode if I had 
> included something like "and relevant code tidy-ups" in my RR title? :P
> 
> Also, is one supposed to go through an RR for each and every tidy-up one 
> might want to commit (now that I have commit access ...)?

You can stuff as many patches (commits) into one review as you like, just that 
one patch may delay the "ship it" to all others than and at some point ppl. 
will refuse to look at 3MB of patches at once ;-)

I don't think that pure tidy up commits belong into KDE4 at all, if you really 
consider one necessary, better have it reviewed.

For KF5/master you ideally clean up code while editing the area anyway.
Global astyle fixes should only be done when things become too much of a mess - 
and be shown to the maintainer.
(Reason is that you introduce "fences" to "git blame")


- Thomas


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


On Sept. 20, 2014, 10:54 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 10:54 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Marko Käning]

> On Sept. 20, 2014, 10:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
> No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in 
> vi (which doesn't modify anything unless I ask for it) and there are no tabs 
> to be found. Same in KDevelop when I open the patch review tool. I wonder if 
> your editor isn't the culprit, or something on reviewboard?
> 
> I copied the .reviewboardrc file from another KDE project and edited the 
> obvious settings. I see nothing concerning tabs in there, but maybe there 
> ought to be?
> 
> René J.V. Bertin wrote:
> See my local copy of the diff I just added.
> 
> Thomas Lübking wrote:
> There're no tabs in the diff (checked teh only reliable way: okteta ;-)
> 
> Indention in those files is crazy (2 or 3 chars, never 4), what makes it 
> look like WS/tab intermix, but ">>>" seems only a RB indication that the line 
> was altered only by shifting.
> 
> René J.V. Bertin wrote:
> Just a question: would I still have triggered `-pedantic` mode if I had 
> included something like "and relevant code tidy-ups" in my RR title? :P
> 
> Also, is one supposed to go through an RR for each and every tidy-up one 
> might want to commit (now that I have commit access ...)?
> 
> Thomas Lübking wrote:
> You can stuff as many patches (commits) into one review as you like, just 
> that one patch may delay the "ship it" to all others than and at some point 
> ppl. will refuse to look at 3MB of patches at once ;-)
> 
> I don't think that pure tidy up commits belong into KDE4 at all, if you 
> really consider one necessary, better have it reviewed.
> 
> For KF5/master you ideally clean up code while editing the area anyway.
> Global astyle fixes should only be done when things become too much of a 
> mess - and be shown to the maintainer.
> (Reason is that you introduce "fences" to "git blame")

Reason being that "fences" to "git blame" get introduced is a very good point, 
indeed! (Never thought of that, actually.)
That's somewhat a problem of the SCM's way of handling the code lines then!
Would be cool if white-space commits could be marked as such, so that git could 
omit/bridge them in case of blame...


- Marko


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


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Christoph Feck]

> On Sept. 20, 2014, 8:20 a.m., Martin Gräßlin wrote:
> > CMakeLists.txt, lines 225-234
> > <https://git.reviewboard.kde.org/r/120287/diff/3/?file=313626#file313626line225>
> >
> > tabs instead of whitespaces
> 
> René J.V. Bertin wrote:
> I looked into those yesterday, and cannot find tabs on my end. No idea 
> where they'd come from.
> 
> Martin Gräßlin wrote:
> I'd guess the editor you use changes it somehow. I'd suggest to use Kate 
> and enable show all whitespaces.
> 
> René J.V. Bertin wrote:
> No, really, those tabs aren't there. I re-reopened the CMakeLists.txt in 
> vi (which doesn't modify anything unless I ask for it) and there are no tabs 
> to be found. Same in KDevelop when I open the patch review tool. I wonder if 
> your editor isn't the culprit, or something on reviewboard?
> 
> I copied the .reviewboardrc file from another KDE project and edited the 
> obvious settings. I see nothing concerning tabs in there, but maybe there 
> ought to be?
> 
> René J.V. Bertin wrote:
> See my local copy of the diff I just added.
> 
> Thomas Lübking wrote:
> There're no tabs in the diff (checked teh only reliable way: okteta ;-)
> 
> Indention in those files is crazy (2 or 3 chars, never 4), what makes it 
> look like WS/tab intermix, but ">>>" seems only a RB indication that the line 
> was altered only by shifting.
> 
> René J.V. Bertin wrote:
> Just a question: would I still have triggered `-pedantic` mode if I had 
> included something like "and relevant code tidy-ups" in my RR title? :P
> 
> Also, is one supposed to go through an RR for each and every tidy-up one 
> might want to commit (now that I have commit access ...)?
> 
> Thomas Lübking wrote:
> You can stuff as many patches (commits) into one review as you like, just 
> that one patch may delay the "ship it" to all others than and at some point 
> ppl. will refuse to look at 3MB of patches at once ;-)
> 
> I don't think that pure tidy up commits belong into KDE4 at all, if you 
> really consider one necessary, better have it reviewed.
> 
> For KF5/master you ideally clean up code while editing the area anyway.
> Global astyle fixes should only be done when things become too much of a 
> mess - and be shown to the maintainer.
> (Reason is that you introduce "fences" to "git blame")
> 
> Marko Käning wrote:
> Reason being that "fences" to "git blame" get introduced is a very good 
> point, indeed! (Never thought of that, actually.)
> That's somewhat a problem of the SCM's way of handling the code lines 
> then!
> Would be cool if white-space commits could be marked as such, so that git 
> could omit/bridge them in case of blame...

The >>> markers reviewboard shows are not Tabs, but change in indentation.


- Christoph


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


On Sept. 20, 2014, 10:54 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 10:54 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

E AND Darwin)`.


- René J.V.


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


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

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



kcontrol/CMakeLists.txt
<https://git.reviewboard.kde.org/r/120287/#comment46794>

so what's the point of the if now?

We have:
* Windows
* Apple
* Linux (by fontconfig)



libs/CMakeLists.txt
<https://git.reviewboard.kde.org/r/120287/#comment46793>

why don't you merge with NOT WIN32? In all other cases it's merged.


- Martin Gräßlin


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

n the 
> sole OS X vendor, but OS X is likely to be replaced some day by something 
> that's not `(APPLE AND Darwin)`.

> this patch is for building kde-workspace on OS X

And it builds with APPLE and Darwin intermixed, does it?
I agree that aligning the detection is reasonable, but that's still a different 
patch ("atomic")

About the proper check: I assume one really has to test for (APPLE AND Darwin) 
to half-wise detect OSX (there seems no specific key in cmake).
Half-wise because this applies to iOS as well :-(


- Thomas


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


On Sept. 20, 2014, 10:54 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 10:54 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 12:29 nachm., Martin Gräßlin wrote:
> > kcontrol/CMakeLists.txt, lines 45-48
> > <https://git.reviewboard.kde.org/r/120287/diff/4/?file=313800#file313800line45>
> >
> > so what's the point of the if now?
> > 
> > We have:
> > * Windows
> > * Apple
> > * Linux (by fontconfig)

The actual check seems to be for Linux w/o fontconfig.
Likely possible but rare and strange setup.


- Thomas


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


On Sept. 20, 2014, 10:54 vorm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 10:54 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

to be replaced some day by something 
> that's not `(APPLE AND Darwin)`.
> 
> Thomas Lübking wrote:
> > this patch is for building kde-workspace on OS X
> 
> And it builds with APPLE and Darwin intermixed, does it?
> I agree that aligning the detection is reasonable, but that's still a 
> different patch ("atomic")
> 
> About the proper check: I assume one really has to test for (APPLE AND 
> Darwin) to half-wise detect OSX (there seems no specific key in cmake).
> Half-wise because this applies to iOS as well :-(

Didn't know/realise that.

Guess someone could raise an issue upstream with cmake for that ...


- René J.V.


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


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

> On Sept. 20, 2014, 2:29 p.m., Martin Gräßlin wrote:
> > kcontrol/CMakeLists.txt, lines 45-48
> > <https://git.reviewboard.kde.org/r/120287/diff/4/?file=313800#file313800line45>
> >
> > so what's the point of the if now?
> > 
> > We have:
> > * Windows
> > * Apple
> > * Linux (by fontconfig)
> 
> Thomas Lübking wrote:
> The actual check seems to be for Linux w/o fontconfig.
> Likely possible but rare and strange setup.

Or APPLE without fontconfig, which might be a less rare setup. So I'd prefer to 
keep it like this and figure out in KF5 what the status of the fonts kcm is 
going to be.


> On Sept. 20, 2014, 2:29 p.m., Martin Gräßlin wrote:
> > libs/CMakeLists.txt, lines 9-13
> > <https://git.reviewboard.kde.org/r/120287/diff/4/?file=313803#file313803line9>
> >
> > why don't you merge with NOT WIN32? In all other cases it's merged.

I just changed the Darwin check to one for APPLE, didn't want to do anything 
less atomic :P

Maybe we could introduce a molecule, pardon, macro, that boils down to NOX11 in 
the toplevel cmake file?


- René J.V.


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


On Sept. 20, 2014, 12:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> ----
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 120287: [OS X] make kde-workspace build

[René J . V . Bertin]

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

(Updated Sept. 20, 2014, 2:52 p.m.)


Review request for KDE Software on Mac OS X and kde-workspace.


Repository: kde-workspace


Description
---

A few rather straightforward patches to make the relevant bits of KDE4's 
kde-workspace build and function on OS X.
The main interest is having the systemsettings control panel to control the 
various relevant KDE settings among which desktop search, fonts, colours and 
even style.
The oxygen style builds and looks good but shows some updating glitches due to 
compositing.

I'm submitting this patch partly in hope it may be useful in bringing 
kf5-workspace to OS X, one day.


Diffs (updated)
-

  CMakeLists.txt 195f99c 
  kcontrol/CMakeLists.txt fc666b1 
  kcontrol/krdb/krdb.cpp 36fc99c 
  kcontrol/style/CMakeLists.txt d832b20 
  libs/CMakeLists.txt c0576fe 

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


Testing
---

On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
git/master, 4.14.1).


File Attachments


copy of the diff file saved locally, which had no tabs when I uploaded it. 
Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch


Thanks,

René J.V. Bertin

Re: Review Request 120287: [OS X] make kde-workspace build

[Thomas Lübking]

> On Sept. 20, 2014, 12:29 nachm., Martin Gräßlin wrote:
> > kcontrol/CMakeLists.txt, lines 45-48
> > <https://git.reviewboard.kde.org/r/120287/diff/4/?file=313800#file313800line45>
> >
> > so what's the point of the if now?
> > 
> > We have:
> > * Windows
> > * Apple
> > * Linux (by fontconfig)
> 
> Thomas Lübking wrote:
> The actual check seems to be for Linux w/o fontconfig.
> Likely possible but rare and strange setup.
> 
> René J.V. Bertin wrote:
> Or APPLE without fontconfig, which might be a less rare setup. So I'd 
> prefer to keep it like this and figure out in KF5 what the status of the 
> fonts kcm is going to be.

I meant "why this check was originally introduced".
Someone then stood up and said "but fonconfig is not required on windows" - and 
not on apple either.

Martin then wondered "why is there a check itfp if it ultimaltey just checks 
'if anything'" and me assumed the actual check is whether this is "NOT X11 OR 
FONTCONFIG_FOUND".


- Thomas


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


On Sept. 20, 2014, 12:52 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 12:52 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 119497: Report crashes of KDE apps in Apple OS X (1) (fix kcrash)

[Ian Wadham]

> On July 29, 2014, 3:33 p.m., Aleix Pol Gonzalez wrote:
> > kinit/kinit.cpp, line 1481
> > 
> >
> > do you need $DISPLAY in OS X?
> 
> René J.V. Bertin wrote:
> Nope. It can be set if the user has XQuartz installed and running, but 
> that shouldn't make a difference.
> 
> In fact, $DISPLAY shouldn't be used on OS X because we wouldn't want 
> things like socket names change when the user starts or quits XQuartz with 
> KDE apps and/or services running.
> 
> Ian Wadham wrote:
> Perish the thought ($DISPLAY fluctuating between a value and an empty 
> string). On my system, OS X 10.7.5 (Lion), XQuartz was installed by Apple and 
> $DISPLAY is always set to a value, even if XQuartz (X11.app) is not running 
> (i.e. no X11 server running). I presume installing X11 somehow "doctors" the 
> OS X (Darwin) startup procedures so that DISPLAY is already defined in every 
> user's ~/.profile. I do not know if that would be the case if a FOSS version 
> of XQuartz would be installed (e.g. on OS X 10.9.x Mavericks).
> 
> The big thing is that $DISPLAY -is- used (non-portably) in KDE to help 
> name a socket in kdeinit4 (kinit.cpp), wrapper.c, kwrapper and KCrash - and 
> FAIK it may be used in this way in several other places. If $DISPLAY is not 
> used consistently in all those places, communication with kdeinit4 can break, 
> as it does already in KCrash/kdeinit4 on Apple OS X. And THAT is what we are 
> trying to fix here. KDE apps on Apple OS X MUST be able to report a crash 
> reliably.
> 
> This is why I keep asking for help from a KDE core developer. How 
> widespread is this problem in KDE on Apple OS X? What are the implication for 
> KDE apps? Should references to $DISPLAY in KDE be eliminated from the OS X 
> version? What do kdeinit4, kwrapper and klauncher really do? Is whatever they 
> do actually portable to Apple OS X? Or are we all, KDE guys and MacPorts guys 
> alike, just crossing our fingers and hoping for the best?
> 
> I tried using lxr.kde.org to find further references, but there are 
> thousands: mostly because "display" is a commonly-used English word in 
> programming. And I did tick the case-sensitive box, but it does not seem to 
> work.
> 
> René J.V. Bertin wrote:
> Hmm, interesting. I should find some time to play with this in my 10.9 VM.
> Know however that even if $DISPLAY is always set, it will not always have 
> the same value. At least for me, XQuartz has the annoying habit of increasing 
> the display number after a restart.
> 
> If it's too complicated to remove all references to $DISPLAY from KDE 
> code (which wouldn't surprise me at all) there remains another approach. 
> Identify an appropriate location in the startup/initialisation code that all 
> KDE applications and services share, and set $DISPLAY to something sensible 
> (but preferably NOT a valid X11 display specifier). The only possible 
> undesirable side-effect I can see from here would be that shells in konsole 
> risk to inherit that value.
> This probably isn't the most elegant solution, but then again that's 
> because KDE apparently never imposed the use of its own internal 
> variable/function (or one from Qt) instead of directly querying $DISPLAY.
> 
> Ian Wadham wrote:
> Restart of what? My system (Lion) has Apple OS X's XQuartz and $DISPLAY 
> has a random temp-file path prepended every time Apple OS X starts of 
> restarts. And that path is different every time. But so what? $DISPLAY keeps 
> the same value no matter how many times I start XQuartz (X11) by running Gimp 
> or whatever. And that value, determined immediately after boot or restart, 
> should be picked by all KDE components, which come into existence later.
> 
> René J.V. Bertin wrote:
> I meant restarts of the X server - it does crash sometimes, sometimes I 
> have to restart it for other reasons, like after logging off and back in. I 
> recall that I used to have those weird (socket based?) $DISPLAY values too, 
> but now they're of a perfectly normal host:X.Y form on my systems. Except 
> that X tends to increase each time I start XQuartz. I ignore how common this 
> is, but I think that if you're looking into the use of $DISPLAY on OS X, you 
> could just as well take all possible situations into account. I'd suggest to 
> use the fact that the actual value is irrelevant and without important to 
> clamp it to something appropriate (why not Qt4:Mac) in such a way that all 
> those younger components pick up that value. And not the actual, current 
> value of $DISPLAY, which may or may not have remained unchanged. (I 
> specifically used the term clamp to draw an analogy signal acquisition, where 
> unconnected inputs can carry all kinds of bogus signals.)
> 
> René J.V. Bertin wrote:
> I've looked into the $DISPLAY value variations mentioned (= described 
> from memory) above. The b

Re: Review Request 120287: [OS X] make kde-workspace build

[Martin Gräßlin]

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



CMakeLists.txt
<https://git.reviewboard.kde.org/r/120287/#comment46804>

out of interest: this is now a huge NOT WIN32 block with two NOT APPLE 
blocks. If I see correctly, what remains being built on Apple is:
* kcheckpass
* qguiplatformplugin_kde

Somehow I doubt kcheckpass will work on MacOS (it uses PAM) or is of any 
need (only used by kscreenlocker_greet in ksmserver which is not built). And 
qguiplatformplugin_kde should not be of any use on MacOS either because it's 
the Qt plugin for a Plasma session and shouldn't (TM) be used on other 
platforms.

So if I see correctly you could just merge the whole thing into a
if(NOT WIN32 AND NOT APPLE)


- Martin Gräßlin


On Sept. 20, 2014, 2:52 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120287/
> ---
> 
> (Updated Sept. 20, 2014, 2:52 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kde-workspace.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> A few rather straightforward patches to make the relevant bits of KDE4's 
> kde-workspace build and function on OS X.
> The main interest is having the systemsettings control panel to control the 
> various relevant KDE settings among which desktop search, fonts, colours and 
> even style.
> The oxygen style builds and looks good but shows some updating glitches due 
> to compositing.
> 
> I'm submitting this patch partly in hope it may be useful in bringing 
> kf5-workspace to OS X, one day.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 195f99c 
>   kcontrol/CMakeLists.txt fc666b1 
>   kcontrol/krdb/krdb.cpp 36fc99c 
>   kcontrol/style/CMakeLists.txt d832b20 
>   libs/CMakeLists.txt c0576fe 
> 
> Diff: https://git.reviewboard.kde.org/r/120287/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 and 10.9.4 with KDE/MacPorts (4.12.5 and more recently kdelibs 
> git/master, 4.14.1).
> 
> 
> File Attachments
> 
> 
> copy of the diff file saved locally, which had no tabs when I uploaded it. 
> Checksum: 3989cdd46af3c891e570974d66c330403dcd41c4ee5e17a372fa385080cbabd1 
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/09/20/b212730f-6258-4277-851c-226bc0673aa1__patchreview-20140920.patch
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 119372: Fix Bug 337486 - Should not permit moving of read-only folder to a different directory

[David Faure]

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

Ship it!


Ship It!

- David Faure


On Sept. 19, 2014, 11:56 a.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119372/
> ---
> 
> (Updated Sept. 19, 2014, 11:56 a.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Bugs: 337486
> http://bugs.kde.org/show_bug.cgi?id=337486
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> Fixes Bug 337486
> 
> 
> Diffs
> -
> 
>   lib/konq/konq_operations.cpp 220a90a 
> 
> Diff: https://git.reviewboard.kde.org/r/119372/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Review Request 119497: Report crashes of KDE apps in Apple OS X (1) (fix kcrash)

[Thomas Lübking]

> On Juli 29, 2014, 3:33 nachm., Aleix Pol Gonzalez wrote:
> > kinit/kinit.cpp, line 1481
> > 
> >
> > do you need $DISPLAY in OS X?
> 
> René J.V. Bertin wrote:
> Nope. It can be set if the user has XQuartz installed and running, but 
> that shouldn't make a difference.
> 
> In fact, $DISPLAY shouldn't be used on OS X because we wouldn't want 
> things like socket names change when the user starts or quits XQuartz with 
> KDE apps and/or services running.
> 
> Ian Wadham wrote:
> Perish the thought ($DISPLAY fluctuating between a value and an empty 
> string). On my system, OS X 10.7.5 (Lion), XQuartz was installed by Apple and 
> $DISPLAY is always set to a value, even if XQuartz (X11.app) is not running 
> (i.e. no X11 server running). I presume installing X11 somehow "doctors" the 
> OS X (Darwin) startup procedures so that DISPLAY is already defined in every 
> user's ~/.profile. I do not know if that would be the case if a FOSS version 
> of XQuartz would be installed (e.g. on OS X 10.9.x Mavericks).
> 
> The big thing is that $DISPLAY -is- used (non-portably) in KDE to help 
> name a socket in kdeinit4 (kinit.cpp), wrapper.c, kwrapper and KCrash - and 
> FAIK it may be used in this way in several other places. If $DISPLAY is not 
> used consistently in all those places, communication with kdeinit4 can break, 
> as it does already in KCrash/kdeinit4 on Apple OS X. And THAT is what we are 
> trying to fix here. KDE apps on Apple OS X MUST be able to report a crash 
> reliably.
> 
> This is why I keep asking for help from a KDE core developer. How 
> widespread is this problem in KDE on Apple OS X? What are the implication for 
> KDE apps? Should references to $DISPLAY in KDE be eliminated from the OS X 
> version? What do kdeinit4, kwrapper and klauncher really do? Is whatever they 
> do actually portable to Apple OS X? Or are we all, KDE guys and MacPorts guys 
> alike, just crossing our fingers and hoping for the best?
> 
> I tried using lxr.kde.org to find further references, but there are 
> thousands: mostly because "display" is a commonly-used English word in 
> programming. And I did tick the case-sensitive box, but it does not seem to 
> work.
> 
> René J.V. Bertin wrote:
> Hmm, interesting. I should find some time to play with this in my 10.9 VM.
> Know however that even if $DISPLAY is always set, it will not always have 
> the same value. At least for me, XQuartz has the annoying habit of increasing 
> the display number after a restart.
> 
> If it's too complicated to remove all references to $DISPLAY from KDE 
> code (which wouldn't surprise me at all) there remains another approach. 
> Identify an appropriate location in the startup/initialisation code that all 
> KDE applications and services share, and set $DISPLAY to something sensible 
> (but preferably NOT a valid X11 display specifier). The only possible 
> undesirable side-effect I can see from here would be that shells in konsole 
> risk to inherit that value.
> This probably isn't the most elegant solution, but then again that's 
> because KDE apparently never imposed the use of its own internal 
> variable/function (or one from Qt) instead of directly querying $DISPLAY.
> 
> Ian Wadham wrote:
> Restart of what? My system (Lion) has Apple OS X's XQuartz and $DISPLAY 
> has a random temp-file path prepended every time Apple OS X starts of 
> restarts. And that path is different every time. But so what? $DISPLAY keeps 
> the same value no matter how many times I start XQuartz (X11) by running Gimp 
> or whatever. And that value, determined immediately after boot or restart, 
> should be picked by all KDE components, which come into existence later.
> 
> René J.V. Bertin wrote:
> I meant restarts of the X server - it does crash sometimes, sometimes I 
> have to restart it for other reasons, like after logging off and back in. I 
> recall that I used to have those weird (socket based?) $DISPLAY values too, 
> but now they're of a perfectly normal host:X.Y form on my systems. Except 
> that X tends to increase each time I start XQuartz. I ignore how common this 
> is, but I think that if you're looking into the use of $DISPLAY on OS X, you 
> could just as well take all possible situations into account. I'd suggest to 
> use the fact that the actual value is irrelevant and without important to 
> clamp it to something appropriate (why not Qt4:Mac) in such a way that all 
> those younger components pick up that value. And not the actual, current 
> value of $DISPLAY, which may or may not have remained unchanged. (I 
> specifically used the term clamp to draw an analogy signal acquisition, where 
> unconnected inputs can carry all kinds of bogus signals.)
> 
> René J.V. Bertin wrote:
> I've looked into the $DISPLAY value variations mentioned (= described 
> from memory) above. The

Re: Review Request 119497: Report crashes of KDE apps in Apple OS X (1) (fix kcrash)

[Ian Wadham]

> On July 29, 2014, 3:33 p.m., Aleix Pol Gonzalez wrote:
> > kinit/kinit.cpp, line 1481
> > 
> >
> > do you need $DISPLAY in OS X?
> 
> René J.V. Bertin wrote:
> Nope. It can be set if the user has XQuartz installed and running, but 
> that shouldn't make a difference.
> 
> In fact, $DISPLAY shouldn't be used on OS X because we wouldn't want 
> things like socket names change when the user starts or quits XQuartz with 
> KDE apps and/or services running.
> 
> Ian Wadham wrote:
> Perish the thought ($DISPLAY fluctuating between a value and an empty 
> string). On my system, OS X 10.7.5 (Lion), XQuartz was installed by Apple and 
> $DISPLAY is always set to a value, even if XQuartz (X11.app) is not running 
> (i.e. no X11 server running). I presume installing X11 somehow "doctors" the 
> OS X (Darwin) startup procedures so that DISPLAY is already defined in every 
> user's ~/.profile. I do not know if that would be the case if a FOSS version 
> of XQuartz would be installed (e.g. on OS X 10.9.x Mavericks).
> 
> The big thing is that $DISPLAY -is- used (non-portably) in KDE to help 
> name a socket in kdeinit4 (kinit.cpp), wrapper.c, kwrapper and KCrash - and 
> FAIK it may be used in this way in several other places. If $DISPLAY is not 
> used consistently in all those places, communication with kdeinit4 can break, 
> as it does already in KCrash/kdeinit4 on Apple OS X. And THAT is what we are 
> trying to fix here. KDE apps on Apple OS X MUST be able to report a crash 
> reliably.
> 
> This is why I keep asking for help from a KDE core developer. How 
> widespread is this problem in KDE on Apple OS X? What are the implication for 
> KDE apps? Should references to $DISPLAY in KDE be eliminated from the OS X 
> version? What do kdeinit4, kwrapper and klauncher really do? Is whatever they 
> do actually portable to Apple OS X? Or are we all, KDE guys and MacPorts guys 
> alike, just crossing our fingers and hoping for the best?
> 
> I tried using lxr.kde.org to find further references, but there are 
> thousands: mostly because "display" is a commonly-used English word in 
> programming. And I did tick the case-sensitive box, but it does not seem to 
> work.
> 
> René J.V. Bertin wrote:
> Hmm, interesting. I should find some time to play with this in my 10.9 VM.
> Know however that even if $DISPLAY is always set, it will not always have 
> the same value. At least for me, XQuartz has the annoying habit of increasing 
> the display number after a restart.
> 
> If it's too complicated to remove all references to $DISPLAY from KDE 
> code (which wouldn't surprise me at all) there remains another approach. 
> Identify an appropriate location in the startup/initialisation code that all 
> KDE applications and services share, and set $DISPLAY to something sensible 
> (but preferably NOT a valid X11 display specifier). The only possible 
> undesirable side-effect I can see from here would be that shells in konsole 
> risk to inherit that value.
> This probably isn't the most elegant solution, but then again that's 
> because KDE apparently never imposed the use of its own internal 
> variable/function (or one from Qt) instead of directly querying $DISPLAY.
> 
> Ian Wadham wrote:
> Restart of what? My system (Lion) has Apple OS X's XQuartz and $DISPLAY 
> has a random temp-file path prepended every time Apple OS X starts of 
> restarts. And that path is different every time. But so what? $DISPLAY keeps 
> the same value no matter how many times I start XQuartz (X11) by running Gimp 
> or whatever. And that value, determined immediately after boot or restart, 
> should be picked by all KDE components, which come into existence later.
> 
> René J.V. Bertin wrote:
> I meant restarts of the X server - it does crash sometimes, sometimes I 
> have to restart it for other reasons, like after logging off and back in. I 
> recall that I used to have those weird (socket based?) $DISPLAY values too, 
> but now they're of a perfectly normal host:X.Y form on my systems. Except 
> that X tends to increase each time I start XQuartz. I ignore how common this 
> is, but I think that if you're looking into the use of $DISPLAY on OS X, you 
> could just as well take all possible situations into account. I'd suggest to 
> use the fact that the actual value is irrelevant and without important to 
> clamp it to something appropriate (why not Qt4:Mac) in such a way that all 
> those younger components pick up that value. And not the actual, current 
> value of $DISPLAY, which may or may not have remained unchanged. (I 
> specifically used the term clamp to draw an analogy signal acquisition, where 
> unconnected inputs can carry all kinds of bogus signals.)
> 
> René J.V. Bertin wrote:
> I've looked into the $DISPLAY value variations mentioned (= described 
> from memory) above. The b

Re: Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)

[Thomas Lübking]

> On Sept. 19, 2014, 10:16 vorm., Ben Cooksley wrote:
> > Unless anyone has any objections in the next 24 hours, I think this can go 
> > in as it makes Dr Konqi usable on another platform.

+1 - "Shit It" ... from my side.

Certainly no technical concerns remaining here - but i'm not the code 
owner/maintainer either ;-)


- Thomas


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


On Sept. 18, 2014, 10:57 vorm., Ian Wadham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119498/
> ---
> 
> (Updated Sept. 18, 2014, 10:57 vorm.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and 
> Michael Pyne.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> When a KDE app crashes in Apple OS X, it just disappears from the screen. At 
> the most, the user is invited to report the crash to Apple. AFAIK this has 
> been a problem in KDE on Apple OS X for years, leading to frustration with 
> KDE among Apple users and MacPorts developers and an attitude among KDE 
> developers of "Why does nobody report the problem(s) on bugs.kde.org?"
> 
> It is my strong belief that the failure to report crashes of KDE apps in 
> Apple OS X also exists in Frameworks.
> 
> So far I have identified a number of portability bugs in KDE on Apple OS X: 1 
> in KCrash, 1 in kdeinit4 and 5 in Dr Konqi. Three patches for Dr Konqi are 
> submitted in this review. Patches for KCrash and kdeinit4 are submitted in 
> part 1 of this review, against kdelibs. I am still investigating the other 
> two problems in Dr Konqi - and there could be more than two...
> 
> In this review we have three portability problems:
> 
> 1. On Apple OS X, Dr Konqi's dialog box hides itself underneath the main 
> window of the app that has just crashed, so is effectively useless. This 
> appears to be because Dr Konqi is started by a Linux/Unix method (fork() + 
> exec()?). If an app is started with the Apple OS X "open" command, it always 
> appears on top. The patch just raises the dialog box.
> 
> 2. When formatting the backtrace output, Dr Konqi crashes (with an ASSERT) on 
> the last line. This appears to be an error in the algorithm used (i.e. also a 
> bug in Linux KDE), but the patch is treating it as an Apple OS X portability 
> problem for now.
> 
> 3. Dr Konqi checks whether the user can save cookies in kcookiejar and, if 
> not, stops reporting the crash. On Apple OS X, cookies would be kept in 
> another browser (e.g. Safari or Firefox) and not in KDE's default browser 
> (Konqueror) and cookie jar. IMHO, Dr K should report the crash no matter 
> what, as long as it can connect to bugs.kde.org and log in.
> 
> 
> Diffs
> -
> 
>   drkonqi/gdbhighlighter.cpp 7cd0aa9 
>   drkonqi/main.cpp 75e060e 
>   drkonqi/reportassistantpages_bugzilla.cpp 86ca327 
> 
> Diff: https://git.reviewboard.kde.org/r/119498/diff/
> 
> 
> Testing
> ---
> 
> Using Apple OS X 10.7.5 (Lion) on a MacBook Pro, I have installed KDE libs 
> via MacPorts (at version 4.12.5) and I have adapted kdesrc-build to run in an 
> Apple OS X environment and used it to test against the KDE 4.13 branch. I 
> have been testing with a KDE app that I can crash at will and using stderr 
> and Apple OS X Console log output to determine the outcome.
> 
> Please note that I am the -only- KDE developer who has this kind of setup, 
> but I am NOT a KDE core developer. My experience before now has been in KDE 
> Games. However I used to be a UNIX and database guru before I retired in 1998.
> 
> I NEED HELP from KDE -core- developers to proceed further. These problems 
> will also exist in Dr Konqi for KF 5, but I am as yet unable to build or test 
> Frameworks on Apple OS X and I cannot find Dr Konqi among the Frameworks 
> repositories. I am sure there are many more Apple OS X portability problems 
> in Dr Konqi and other KDE software.
> 
> Without my patches, Dr Konqi, on Apple OS X, remains invisible to the user, 
> often fails to complete the backtrace report and then fails to connect to 
> bugs.kde.org.
> 
> With my patches, Dr Konqi on Apple OS X can generate a full crash report, 
> including the backtrace and the results of the dialog with the user. 
> Sometimes, however, it fails to submit the completed report to bugs.kde.org. 
> This problem is still under investigation.
> 
> I would not have got this far without help from Michael Pyne, Thomas Lübking 
> and several of the MacPorts developers, as well as the unfailing enthusiasm 
> and encouragement of my friend Marko Käning.
> 
> 
> File Attachments
> 
> 
> Log of Dr K ASSERT problem
>   
> https://git.reviewboard.kde.org/media/u

Re: Review Request 119498: Report crashes of KDE apps in Apple OS X (2) (fix drkonqi)

[Ian Wadham]

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

Ship it!


Ship It!

- Ian Wadham


On Sept. 18, 2014, 10:57 a.m., Ian Wadham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119498/
> ---
> 
> (Updated Sept. 18, 2014, 10:57 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and 
> Michael Pyne.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> When a KDE app crashes in Apple OS X, it just disappears from the screen. At 
> the most, the user is invited to report the crash to Apple. AFAIK this has 
> been a problem in KDE on Apple OS X for years, leading to frustration with 
> KDE among Apple users and MacPorts developers and an attitude among KDE 
> developers of "Why does nobody report the problem(s) on bugs.kde.org?"
> 
> It is my strong belief that the failure to report crashes of KDE apps in 
> Apple OS X also exists in Frameworks.
> 
> So far I have identified a number of portability bugs in KDE on Apple OS X: 1 
> in KCrash, 1 in kdeinit4 and 5 in Dr Konqi. Three patches for Dr Konqi are 
> submitted in this review. Patches for KCrash and kdeinit4 are submitted in 
> part 1 of this review, against kdelibs. I am still investigating the other 
> two problems in Dr Konqi - and there could be more than two...
> 
> In this review we have three portability problems:
> 
> 1. On Apple OS X, Dr Konqi's dialog box hides itself underneath the main 
> window of the app that has just crashed, so is effectively useless. This 
> appears to be because Dr Konqi is started by a Linux/Unix method (fork() + 
> exec()?). If an app is started with the Apple OS X "open" command, it always 
> appears on top. The patch just raises the dialog box.
> 
> 2. When formatting the backtrace output, Dr Konqi crashes (with an ASSERT) on 
> the last line. This appears to be an error in the algorithm used (i.e. also a 
> bug in Linux KDE), but the patch is treating it as an Apple OS X portability 
> problem for now.
> 
> 3. Dr Konqi checks whether the user can save cookies in kcookiejar and, if 
> not, stops reporting the crash. On Apple OS X, cookies would be kept in 
> another browser (e.g. Safari or Firefox) and not in KDE's default browser 
> (Konqueror) and cookie jar. IMHO, Dr K should report the crash no matter 
> what, as long as it can connect to bugs.kde.org and log in.
> 
> 
> Diffs
> -
> 
>   drkonqi/gdbhighlighter.cpp 7cd0aa9 
>   drkonqi/main.cpp 75e060e 
>   drkonqi/reportassistantpages_bugzilla.cpp 86ca327 
> 
> Diff: https://git.reviewboard.kde.org/r/119498/diff/
> 
> 
> Testing
> ---
> 
> Using Apple OS X 10.7.5 (Lion) on a MacBook Pro, I have installed KDE libs 
> via MacPorts (at version 4.12.5) and I have adapted kdesrc-build to run in an 
> Apple OS X environment and used it to test against the KDE 4.13 branch. I 
> have been testing with a KDE app that I can crash at will and using stderr 
> and Apple OS X Console log output to determine the outcome.
> 
> Please note that I am the -only- KDE developer who has this kind of setup, 
> but I am NOT a KDE core developer. My experience before now has been in KDE 
> Games. However I used to be a UNIX and database guru before I retired in 1998.
> 
> I NEED HELP from KDE -core- developers to proceed further. These problems 
> will also exist in Dr Konqi for KF 5, but I am as yet unable to build or test 
> Frameworks on Apple OS X and I cannot find Dr Konqi among the Frameworks 
> repositories. I am sure there are many more Apple OS X portability problems 
> in Dr Konqi and other KDE software.
> 
> Without my patches, Dr Konqi, on Apple OS X, remains invisible to the user, 
> often fails to complete the backtrace report and then fails to connect to 
> bugs.kde.org.
> 
> With my patches, Dr Konqi on Apple OS X can generate a full crash report, 
> including the backtrace and the results of the dialog with the user. 
> Sometimes, however, it fails to submit the completed report to bugs.kde.org. 
> This problem is still under investigation.
> 
> I would not have got this far without help from Michael Pyne, Thomas Lübking 
> and several of the MacPorts developers, as well as the unfailing enthusiasm 
> and encouragement of my friend Marko Käning.
> 
> 
> File Attachments
> 
> 
> Log of Dr K ASSERT problem
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/07/30/a3f99f00-94df-4b10-bc47-66b1c966f893__DrKonqiASSERT.kcrash.txt
> 
> 
> Thanks,
> 
> Ian Wadham
> 
>