<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/124905/">https://git.reviewboard.kde.org/r/124905/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 26th, 2015, 7:24 a.m. UTC, <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 sounds to me like ecm_mark_non_gui_executable doesn't do the right thing then, and should be fixed, instead?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">These are non gui executables, so from an "API" point of view, using this function is correct.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Would we ever want a console window when running an app on windows? I guess not?
So maybe we should do whatever "WIN32" does from within that function, if it's doable outside the add_executable call?</p></pre>
</blockquote>
<p>On August 26th, 2015, 8:27 a.m. UTC, <b>Kevin Funk</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's rather that <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">ecm_mark_non_gui_executable</code> is has a misleading meaning: "Marks an executable target as not being a GUI application"</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well, in that case setting WIN32_EXECUTABLE to FALSE is <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">correct</em>, <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">but</em> in fact marking an executable as console application on Windows implies the executable showing a console window. I'm not sure if we should just switch the description/behavior of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">ecm_mark_non_gui_executable</code>, or just not use it (as I did). Honestly I think we'd be better off just not using it (it doesn't really increase convenience, in fact my patch just removes code and still "fixes" things).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Documentation of WIN32_EXECUTABLE:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">WIN32_EXECUTABLE
Build an executable with a WinMain entry point on windows.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When this property is set to true the executable when linked on Windows will be created with a WinMain() entry point instead of just main(). This makes it a GUI executable instead of a console application. See the CMAKE_MFC_FLAG variable documentation to configure use of MFC for WinMain executables. This property is initialized by the value of the variable CMAKE_WIN32_EXECUTABLE if it is set when a target is created.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">http://www.cmake.org/cmake/help/v3.0/prop_tgt/WIN32_EXECUTABLE.html#prop_tgt:WIN32_EXECUTABLE</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think what you're missing is that ecm_mark_non_gui_executable aims at "doing the right thing on all platforms", which includes having no .app bundle on OSX. Your patch goes back to unwanted app bundles on OSX, I presume.
This is why we should fix ecm_mark_non_gui_executable rather than not use it. Also for a linux developer it's easier to think in terms of "mark as non gui" than "do I need that weird WIN32 thing in there?".</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But I'm starting to understand something. Applications that are meant to be used on the command line like kioclient, should be console apps on windows i.e. use main() instead of WinMain(), otherwise we can't see any output, is that right?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So we don't have two cases (gui and non gui), we have three?
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> GUI application (user visible)
</em> helper binary (libexec style). Not user visible. Should be WIN32 to avoid a console window.
* command line tool (console app, "user visible" in the console). Should not be WIN32.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In that case we should have a different ecm macro for the second item in that list, that's what we're missing, isn't it?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On OSX, AFAIU, the last two would be "without app bundle".</p></pre>
<br />
<p>- David</p>
<br />
<p>On August 24th, 2015, 1:36 p.m. UTC, Kevin Funk 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 Frameworks, David Faure and Boudewijn Rempt.</div>
<div>By Kevin Funk.</div>
<p style="color: grey;"><i>Updated Aug. 24, 2015, 1:36 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;">Win: Hide console window for binaries in LIBEXEC</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>src/ioslaves/http/CMakeLists.txt <span style="color: grey">(76a8e2800b84c312431cc1996ac81d1ef6fb5cfc)</span></li>
<li>src/ioslaves/http/kcookiejar/CMakeLists.txt <span style="color: grey">(7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365)</span></li>
<li>src/kioexec/CMakeLists.txt <span style="color: grey">(91284a3a61b86770b4d1939da52d256840803608)</span></li>
<li>src/kioslave/CMakeLists.txt <span style="color: grey">(e02febd380b268c596e8ecc3b745b6f50993ab4e)</span></li>
<li>src/kpac/CMakeLists.txt <span style="color: grey">(fc5989714480ca49b5bd72e1c7b458b26bd0d9bc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124905/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>