<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/128202/">https://git.reviewboard.kde.org/r/128202/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 18th, 2016, 11:17 a.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/128202/diff/1/?file=468904#file468904line388" style="color: black; font-weight: bold; text-decoration: underline;">kde-modules/KDEInstallDirs.cmake</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">388</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nb">_define_absolute</span><span class="p">(</span><span class="s">BUNDLEDIR</span> <span class="s2">"<span class="hl">/</span>Applications/KDE"</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">388</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nb">_define_absolute</span><span class="p">(</span><span class="s">BUNDLEDIR</span> <span class="s2">"Applications/KDE"</span></pre></td>
</tr>
</tbody>
</table>
<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;">Is this going to change anything when you define BUNDLE_INSTALL_DIR on the commandline?</p></pre>
</blockquote>
<p>On June 18th, 2016, 11:35 a.m. CEST, <b>Christoph Cullmann</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;">emerge doesn't set BUNDLE_INSTALL_DIR and yes, if you set it, that will overwrite the dir computed, that is clear.
The question is, why bundle_dir is the only dir that doesn't honor: CMAKE_INSTALL_PREFIX like all other dirs.
I see no problem in letting it honor that to be able to locally install stuff if prefix is set.</p></pre>
</blockquote>
<p>On June 18th, 2016, 12:06 p.m. CEST, <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 think you should begin by fixing your build script so that it uses the provided mechanism to initialise BUNDLE_DIR to exactly what you want it to be, and only patch ECM if that doesn't suffice.
Making the default path relative may work for you, but it will lead to surprise for others. I guess your use here is to let helper applications be installed automagically into the app bundle you're building. For that use case they do NOT have to be in a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">/Applications/KDE</code> subdirectory. There doesn't appear to be an official Apple guideline on where they should be installed, but there seems to be a pattern of putting helper applications directly into Contents/Resources.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As long as BUNDLE_INSTALL_DIR still overrides CMAKE_INSTALL_PREFIX there is no problem in initialising BUNDLE_DIR from CMAKE_INSTALL_PREFIX. The reason it is not is simple: the standard BUNDLE_DIR (/Applications) is not a subdirectory of the standard INSTALL_PREFIX for system software. Also, application bundles are NOT found via the standard Unix search path, whereas things installed into CMAKE_INSTALL_PREFIX are.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is a question you'd want to take up on the cmake ML, but I think that any answer you'll get will be of the type "works as intended".</p></pre>
</blockquote>
<p>On June 18th, 2016, 12:22 p.m. CEST, <b>David Faure</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;">It seems to me that we might need an ECM setting to choose between two modes when building on Mac: the "self-contained app bundles" mode, where we try to put as much as possible into the bundle, and the "full install" which is more like the Linux setup (more shared stuff). You can then each maintain your own solution ;)</p></pre>
</blockquote>
<p>On June 18th, 2016, 1:01 p.m. CEST, <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;">That would at least reassure me somewhat ...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But WTF gave the green light to commit this? What's the point in doing RRs if we're going to commit our proposals when there are open issues and no one gave a ship-it??</p></pre>
</blockquote>
<p>On June 18th, 2016, 1:15 p.m. CEST, <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;">Cf. my https://git.reviewboard.kde.org/r/127649/ . Not sure what would be the leanest and most readable: turning the option on by default or using the inverse logic (= via an intermediate variable that will avoid the need for plenty of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">NOT BUILD_APPLE_APPBUNDLE</code> statements). I guess time and practice will tell.</p></pre>
</blockquote>
<p>On June 18th, 2016, 1:40 p.m. CEST, <b>Christoph Cullmann</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;">See for my commit:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">cullmann committed rEMERGEd365877d9138: set BUNDLE_DIR in emerge, I will reopen the review request after thinking… (authored by cullmann).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">set BUNDLE_DIR in emerge, I will reopen the review request after thinking more about the issue</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">about the two ECM modes: I am not sure if that makes things not even more confusing as they are atm, but I would not oppose if that is wanted</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">REVIEW: 128202</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">BRANCHES
unix3</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">AFFECTED FILES
/bin/BuildSystem/CMakeBuildSystem.py</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">USERS
cullmann (Author)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">COMMIT
https://phabricator.kde.org/rEMERGEd365877d9138</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">Ah, right, using 2 different review mechanisms doesn't make things more confusing <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">at all</em> =)
Phabricator also has the kde-mac group ...
But I <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">am</em> confused: if you set BUNDLE_DIR in emerge, why commit this change even if it's to a side branch?</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On June 18th, 2016, 12:38 p.m. CEST, Christoph Cullmann 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 KDE Software on Mac OS X, KDE Frameworks, Alex Merry, and David Faure.</div>
<div>By Christoph Cullmann.</div>
<p style="color: grey;"><i>Updated June 18, 2016, 12:38 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
extra-cmake-modules
</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;">Current behavior: Even if you have some own installation prefix like emerge, ECM assumes all stuff in the global /Applications/KDE
This doesn't work as stuff like kded5 is not found after installation.
Making it relative resolves this issue.</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;">emerge okular works a bit more with this patch, e.g. kde4support is able to detect kded5.</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>kde-modules/KDEInstallDirs.cmake <span style="color: grey">(f518a4a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128202/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>