[prev in list] [next in list] [prev in thread] [next in thread]
List: calligra-devel
Subject: Re: Review Request 129800: [Mac] : prepare for "linuxy" vs. standalone app bundle builds
From: René J.V. Bertin <rjvbertin () gmail ! com>
Date: 2017-01-17 11:22:25
Message-ID: 20170117112225.31352.58599 () mimi ! kde ! org
[Download RAW message or body]
--===============4209357761992438695==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Jan. 10, 2017, 9:28 p.m., Anthony Fieroni wrote:
> > So when Karbon works, i'm +1, please try words / sheets are they worked as well \
> > as Karbon?
>
> René J.V. Bertin wrote:
> I cannot tell yet, but I'm not expecting any issues because of this patch. As you \
> can see it only touches common/shared code at the moment, and I am NOT building \
> with `-DAPPLE_STANDALONE_BUILD`.
> I *think* I should have most external dependencies installed for Words and Sheets, \
> so I'll be looking at building them in the near future, and update this ticket \
> accordingly.
> I'm my point of view that's icing on the cake that can come after we've reached \
> feature parity with Linux. I can imagine others think different (that's what Macs \
> are for ;)). Hence this patch, it'll make it easier to follow both approaches \
> simultaneously.
> René J.V. Bertin wrote:
> BTW: I do hope for some collaborative brainstorming w.r.t. using info from \
> KoResourcePaths.h more widely, instead of using QStandardPaths types directly. \
> That's a rather delicate intervention which I'd prefer not to take the sole \
> responsibility for.
> René J.V. Bertin wrote:
> I can confirm Words works too with this patch.
>
> (I did have to fix a small issue in the words-odf filter, I've taken the liberty to \
> push the fix: https://cgit.kde.org/calligra.git/commit/?id=a8fd10d8b0a24e581eeb4754b458ba98ddbf0167)
>
> René J.V. Bertin wrote:
> Sheets works nicely too!
>
> Anthony Fieroni wrote:
> I response for only Karbon, wait for Camilla and Dag Andersen.
>
> René J.V. Bertin wrote:
> Evidently.
>
> Plan also works as far as I can tell (I never used it before).
>
> There is an issue with `calligraplanwork` however, but given the error I think it \
> has nothing to do with my changes. If anything, I may have missed a location where \
> a change is required, for instance to install required DBus service files.
> Dag Andersen wrote:
> Afaics this should be ok.
> The defines in KoResourcePaths.h doesn't seem to be used?
> Can't comment on apple stuff, never seen one close up ;)
>
> René J.V. Bertin wrote:
> > The defines in KoResourcePaths.h doesn't seem to be used?
>
> No indeed. As I tried to explain in the description, they're more a proposal:
>
> > the change to KoResourcePathsImpl::mapTypeToQStandardPaths() only has a real \
> > interest if it's used throughout the code to replace the explicit use of \
> > QStandardPaths locations. For now I have set this up through build-type specific \
> > preprocessor macros in KoResourcePaths.h [...]
>
> I don't want to start changing a huge amount of unknown code to use those macros \
> because there are lots of places that would have to be changed, and I don't think \
> it can be done by a few simple search-and-replace runs in an editor. Above all, I \
> want to be sure that you agree about this approach.
> And let me repeat: this isn't necessary for the kind of build that interests me on \
> Mac, so there's no hurry as far as I'm concerned ;)
> Dag Andersen wrote:
> Afaik KoResourcePaths is used in all calligra.
> There are issues and I have a patch waiting (https://phabricator.kde.org/D2577) \
> that didn't get into 3.0. It's fairly big so I'm waiting for 3.1 to look at it \
> again.
> Different thing: in KoApplication line 242 you added a comment:
> // add the paths that Krita5 sets in XDG_DATA_DIRS
> Krita is not using any calligra libs anymore, so this could possibly be removed but \
> I don't know if any other app depends on this, so...
> Afaik KoResourcePaths is used in all calligra.
True, but there are also many places where QStandardPaths locations are used \
directly. I don't know the code so cannot say if that's because they were never \
converted or because the current KoResourcePaths mapping function is too expensive. \
my rewrite should reduce the calling cost significantly but using preprocessor macros \
will of course be even cheaper but again, I don't know whether that cost is relevant. \
Standardising (making sure the right locations are used) is a different topic \
altogether of course.
> // add the paths that Krita5 sets in XDG_DATA_DIRS
Krita is not using any calligra libs anymore, so this could possibly be removed but I \
don't know if any other app depends on this, so...
The point is not that Krita uses calligra libs (or not), but how it handles the QSP \
locations problem on MS Windows. It's been released for a while now so I presume it's \
seen more testing too, so adding the locations it defines should only help if \
anything. OTOH I haven't tried to figure out if and how it actually uses the value of \
XDG_DATA_DIRS. From what I can tell that variable is used only by the Unix/Linux \
QStandardPaths, and it isn't read out anywhere in Calligra's code. But without a \
Windows dev system to verify this I cannot propose to drop setting XDG_DATA_DIRS.
- René J.V.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129800/#review101926
-----------------------------------------------------------
On Jan. 10, 2017, 5:34 p.m., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129800/
> -----------------------------------------------------------
>
> (Updated Jan. 10, 2017, 5:34 p.m.)
>
>
> Review request for Calligra and Camilla Boemann.
>
>
> Repository: calligra
>
>
> Description
> -------
>
> This proposal is an initial implementation of things discussed on the ML a short \
> while back.
> Building KF5 software on Mac as if on any Unix variant (with "Cocoa" instead of \
> X11) is possible and is what you get without specific changes to the build system. \
> With a few tweaks to Qt's QStandardPaths (provided by MacPorts) this kind of build \
> works flawlessly and with an identical feature set as on Linux c.s.
> NB: this build also puts applications in an app bundle "wrapper", but one that \
> contains just the minimal resources (executable, app icon and Info.plist).
> One can also tweak the build so that a relocatable and standalone app bundle \
> results which contains the application and all its 3rd-party dependencies (Qt5, KF5 \
> frameworks, etc.). This works with a stock Qt5 build but still requires patches \
> throughout KF5 code and build systems. Several projects already provide "official" \
> builds of this type for Mac: Kate, KDevelop, Marble and Krita to name a few.
> The current patch prepares for allowing a choice for either a standalone app bundle \
> build or a more traditional build via a CMake option `APPLE_STANDALONE_BUNDLE` and \
> preprocessor token of the same name. This makes it easy to dissociate the Apple \
> build types from general Apple build requirements. Testing for build flavour is \
> done by checking `APPLE_STANDALONE_BUNDLE`, testing for build platform by checking \
> `APPLE` (CMake) or `Q_OS_MACOS` (Qt/C++) (or `__APPLE__` in code not using Qt).
> In addition to the introduction of the CMake option, the patch
>
> - updates `KoApplication::start()`. Judging from Kate's approach it shouldnt' be \
> necessary on Mac to set `XDG_DATA_DIRS`, which isn't used anywhere in code (except \
> in MacPorts tweaked QStandardPaths!). The `PATH` env. variable also shouldn't be \
> *re*set and only needs changing (potentially!) in a standalone app bundle build. I \
> don't have a MS Windows dev. system so I've merged Krita's way of setting \
> `XDG_DATA_DIRS` with Calligra's current code. Note that Qt/Win also doesn't seem to \
> use that variable in `QStandardPaths`.
> - rewrites `KoResourcePathsImpl::mapTypeToQStandardPaths()` to use a much more \
> efficient static `QHash` table. *A priori* it should be possible to use \
> `QStandardPaths::AppDataLocation` to obtain the location of the app bundle \
> resources directory; it could be populated with symlinks into \
> `/path/to/foo.app/Contents/share` so resources can be found with minimum changes to \
> the build system (install locations). This will need to be established going \
> forward (knowing that my own main interest is with "linuxy \
> builds").
> - the change to `KoResourcePathsImpl::mapTypeToQStandardPaths()` only has a real \
> interest if it's used throughout the code instead of explicit use of QStandardPaths \
> locations. For now I have set this up through build-type specific preprocessor \
> macros in KoResourcePaths.h (because an enum would probably have to be cast to work \
> with the QSP methods). I haven't changed any code to use those macros.
> Not yet incorporated: tweaks to the `ecm_add_app_icon` calls to use its new \
> capability to generate an app icon from an SVG file (currently tested with Karbon). \
>
> Diffs
> -----
>
> CMakeLists.txt 13ac88f
> libs/main/KoApplication.cpp 7b23f8d
> libs/widgets/KoResourcePaths.h 8830a5a
> libs/widgets/KoResourcePaths.cpp 7df9dc6
>
> Diff: https://git.reviewboard.kde.org/r/129800/diff/
>
>
> Testing
> -------
>
> Karbon works as expected with this patch on Mac OS X 10.9.5 (and Linux) with Qt \
> 5.7.1 and KF5 5.29.0 installed under /opt/local .
> Without the patch Karbon crashes or aborts immediately on Mac because it doesn't \
> find a single resource in the locations indicated by the inappropriate \
> `XDG_DATA_DIRS` value.
>
> Thanks,
>
> René J.V. Bertin
>
>
--===============4209357761992438695==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/129800/">https://git.reviewboard.kde.org/r/129800/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On January 10th, 2017, 9:28 p.m. CET, <b>Anthony \
Fieroni</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">So when Karbon works, i'm +1, please try words / \
sheets are they worked as well as Karbon?</p></pre> </blockquote>
<p>On January 10th, 2017, 10 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
cannot tell yet, but I'm not expecting any issues because of this patch. As you can \
see it only touches common/shared code at the moment, and I am NOT building with \
<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: \
normal;margin: 0;line-height: inherit;">-DAPPLE_STANDALONE_BUILD</code>.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">I <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">think</em> I should have most external dependencies \
installed for Words and Sheets, so I'll be looking at building them in the near \
future, and update this ticket accordingly.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm my point of view \
that's icing on the cake that can come after we've reached feature parity with Linux. \
I can imagine others think different (that's what Macs are for ;)). Hence this patch, \
it'll make it easier to follow both approaches simultaneously.</p></pre> \
</blockquote>
<p>On January 10th, 2017, 10:03 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">BTW: \
I do hope for some collaborative brainstorming w.r.t. using info from \
KoResourcePaths.h more widely, instead of using QStandardPaths types directly. That's \
a rather delicate intervention which I'd prefer not to take the sole responsibility \
for.</p></pre> </blockquote>
<p>On January 11th, 2017, 4:54 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I can \
confirm Words works too with this patch.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">(I did have to fix a \
small issue in the words-odf filter, I've taken the liberty to push the fix: \
https://cgit.kde.org/calligra.git/commit/?id=a8fd10d8b0a24e581eeb4754b458ba98ddbf0167)</p></pre>
</blockquote>
<p>On January 14th, 2017, 3:22 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Sheets works nicely too!</p></pre> </blockquote>
<p>On January 14th, 2017, 3:45 p.m. CET, <b>Anthony Fieroni</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
response for only Karbon, wait for Camilla and Dag Andersen.</p></pre> </blockquote>
<p>On January 14th, 2017, 8:43 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Evidently.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Plan also works as far as I can tell (I \
never used it before).</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">There is an issue with <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">calligraplanwork</code> however, but given the error I think \
it has nothing to do with my changes. If anything, I may have missed a location where \
a change is required, for instance to install required DBus service files.</p></pre> \
</blockquote>
<p>On January 16th, 2017, 1:38 p.m. CET, <b>Dag Andersen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Afaics this should be ok. The defines in KoResourcePaths.h doesn't seem to \
be used? Can't comment on apple stuff, never seen one close up ;)</p></pre>
</blockquote>
<p>On January 16th, 2017, 6:06 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote \
style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">The defines in KoResourcePaths.h doesn't seem to be used?</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">No indeed. As I tried to explain in the description, \
they're more a proposal:</p> <blockquote style="text-rendering: inherit;padding: 0 0 \
0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 \
0.5em;line-height: inherit;"> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">the change to \
KoResourcePathsImpl::mapTypeToQStandardPaths() only has a real interest if it's used \
throughout the code to replace the explicit use of QStandardPaths locations. For now \
I have set this up through build-type specific preprocessor macros in \
KoResourcePaths.h [...]</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I don't want to start changing a huge amount of \
unknown code to use those macros because there are lots of places that would have to \
be changed, and I don't think it can be done by a few simple search-and-replace runs \
in an editor. Above all, I want to be sure that you agree about this approach.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">And let me repeat: this isn't necessary for the kind of build that \
interests me on Mac, so there's no hurry as far as I'm concerned ;)</p></pre> \
</blockquote>
<p>On January 17th, 2017, 11:36 a.m. CET, <b>Dag Andersen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Afaik \
KoResourcePaths is used in all calligra. There are issues and I have a patch waiting \
(https://phabricator.kde.org/D2577) that didn't get into 3.0. It's fairly big so I'm \
waiting for 3.1 to look at it again.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Different thing: in \
KoApplication line 242 you added a comment: // add the paths that Krita5 sets in \
XDG_DATA_DIRS Krita is not using any calligra libs anymore, so this could possibly be \
removed but I don't know if any other app depends on this, so...</p></pre> \
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote \
style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Afaik KoResourcePaths is used in all calligra.</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">True, but there are also many places where \
QStandardPaths locations are used directly. I don't know the code so cannot say if \
that's because they were never converted or because the current KoResourcePaths \
mapping function is too expensive. my rewrite should reduce the calling cost \
significantly but using preprocessor macros will of course be even cheaper but again, \
I don't know whether that cost is relevant. Standardising (making sure the right \
locations are used) is a different topic altogether of course.</p> <blockquote \
style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">// add the paths that Krita5 sets in XDG_DATA_DIRS Krita is not using any \
calligra libs anymore, so this could possibly be removed but I don't know if any \
other app depends on this, so...</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The point is not that Krita uses calligra libs (or \
not), but how it handles the QSP locations problem on MS Windows. It's been released \
for a while now so I presume it's seen more testing too, so adding the locations it \
defines should only help if anything. OTOH I haven't tried to figure out if and how \
it actually uses the value of XDG_DATA_DIRS. From what I can tell that variable is \
used only by the Unix/Linux QStandardPaths, and it isn't read out anywhere in \
Calligra's code. But without a Windows dev system to verify this I cannot propose to \
drop setting XDG_DATA_DIRS.</p></pre> <br />
<p>- René J.V.</p>
<br />
<p>On January 10th, 2017, 5:34 p.m. CET, René J.V. Bertin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Calligra and Camilla Boemann.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Jan. 10, 2017, 5:34 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This proposal is an initial implementation of things \
discussed on the ML a short while back.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Building KF5 software \
on Mac as if on any Unix variant (with "Cocoa" instead of X11) is possible and is \
what you get without specific changes to the build system. With a few tweaks to Qt's \
QStandardPaths (provided by MacPorts) this kind of build works flawlessly and with an \
identical feature set as on Linux c.s.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">NB: this build also \
puts applications in an app bundle "wrapper", but one that contains just the minimal \
resources (executable, app icon and Info.plist).</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">One \
can also tweak the build so that a relocatable and standalone app bundle results \
which contains the application and all its 3rd-party dependencies (Qt5, KF5 \
frameworks, etc.). This works with a stock Qt5 build but still requires patches \
throughout KF5 code and build systems. Several projects already provide "official" \
builds of this type for Mac: Kate, KDevelop, Marble and Krita to name a few.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">The current patch prepares for allowing a choice for either a standalone \
app bundle build or a more traditional build via a CMake option <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">APPLE_STANDALONE_BUNDLE</code> and preprocessor token of the \
same name. This makes it easy to dissociate the Apple build types from general Apple \
build requirements. Testing for build flavour is done by checking <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">APPLE_STANDALONE_BUNDLE</code>, testing for build platform \
by checking <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">APPLE</code> (CMake) or <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: i nherit;">Q_OS_MACOS</code> (Qt/C++) (or <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">__APPLE__</code> in code not using Qt).</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">In addition to the introduction of the CMake option, the patch</p> <ul \
style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: \
inherit;white-space: normal;"> <li style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">updates <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">KoApplication::start()</code>. Judging from Kate's approach it shouldnt' be \
necessary on Mac to set <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">XDG_DATA_DIRS</code>, which \
isn't used anywhere in code (except in MacPorts tweaked QStandardPaths!). The <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">PATH</code> env. variable also shouldn't be <em \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">re</em>set and only needs changing (potentially!) in a standalone app bundle \
build. I don't have a MS Windows dev. system so I've merged Krita's way of setting \
<code style="text-rendering: inher it;color: #4444cc;padding: 0;white-space: \
normal;margin: 0;line-height: inherit;">XDG_DATA_DIRS</code> with Calligra's current \
code. Note that Qt/Win also doesn't seem to use that variable in <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">QStandardPaths</code>.</li> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">rewrites <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: \
inherit;">KoResourcePathsImpl::mapTypeToQStandardPaths()</code> to use a much more \
efficient static <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">QHash</code> table. <em \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">A priori</em> it should be possible to use <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">QStandardPaths::AppDataLocation</code> to obtain the location of the app \
bundle resources directory; it could be populated with symlinks into <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">/path/to/foo.app/Contents/share< /code> so resources can be \
found with minimum changes to the build system (install locations). This will need to \
be established going forward (knowing that my own main interest is with "linuxy \
builds").</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">the change to <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">KoResourcePathsImpl::mapTypeToQStandardPaths()</code> only has a real \
interest if it's used throughout the code instead of explicit use of QStandardPaths \
locations. For now I have set this up through build-type specific preprocessor macros \
in KoResourcePaths.h (because an enum would probably have to be cast to work with the \
QSP methods). I haven't changed any code to use those macros.</li> </ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Not yet incorporated: tweaks to the <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">ecm_add_app_icon</code> calls to use its new capability to \
generate an app icon from an SVG file (currently tested with Karbon).</p></pre> \
</td> </tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Karbon works as expected with this patch on Mac OS X \
10.9.5 (and Linux) with Qt 5.7.1 and KF5 5.29.0 installed under /opt/local . </p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Without the patch Karbon crashes or aborts immediately on Mac because it \
doesn't find a single resource in the locations indicated by the inappropriate <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">XDG_DATA_DIRS</code> value.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(13ac88f)</span></li>
<li>libs/main/KoApplication.cpp <span style="color: grey">(7b23f8d)</span></li>
<li>libs/widgets/KoResourcePaths.h <span style="color: grey">(8830a5a)</span></li>
<li>libs/widgets/KoResourcePaths.cpp <span style="color: grey">(7df9dc6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129800/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============4209357761992438695==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic