<table><tr><td style="">graesslin requested changes to this revision.<br />graesslin added inline comments.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D6004" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25576" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:78</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">    TYPE RUNTIME
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    PURPOSE "Needed for emergency unlock in case that the greeter is broken.<span class="bright"> In case your distribution does not provide loginctl please contact plasma-devel@kde.org to discuss alternatives.</span>"
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    PURPOSE "Needed for emergency unlock in case that the greeter is broken."
</div><div style="padding: 0 8px; margin: 0 4px; ">    )
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I would keep this information about the contact. We want to be friendly here and it's such a touchy topic that I prefer to be very explicit that we do want to support other solutions.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25578" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:81-87</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">find_package(ConsoleKit)
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">set_package_properties(ConsoleKit PROPERTIES
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    URL "http://www.freedesktop.org/wiki/Software/ConsoleKit"
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    DESCRIPTION "Framework for defining and tracking users"
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    TYPE RUNTIME
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    PURPOSE "Needed for emergency unlock in case that the greeter is broken."
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    )
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I would prefer if we only search for one of the two. What I want to not have is a situation where we get false positive warnings about missing runtime components. E.g. if your distro is using logind there is just no point in looking for ConsoleKit and it would now produce a false positive runtime warning. The same applies the other way around.</p>

<p style="padding: 0; margin: 8px;">Maybe we could work with add_feature_info?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25579" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:109-111</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">if (HAVE_CONSOLEKIT)
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    configure_file(ck-unlock-session.cmake ${CMAKE_CURRENT_BINARY_DIR}/ck-unlock-session)
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">endif ()
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">given that it's used for a configure check: this is not a runtime but a build time dependency.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25580" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">abstractlocker.cpp:52-57</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#if HAVE_LOGINCTL</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">        <span style="color: #aa4000">auto</span> <span class="n">text</span> <span style="color: #aa2211">=</span> <span class="n">ki18n</span><span class="p">(</span><span style="color: #766510">"The screen locker is broken and unlocking is not possible anymore.</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                          <span style="color: #766510">"In order to unlock switch to a virtual terminal (e.g. Ctrl+Alt+F2),</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                          <span style="color: #766510">"log in and execute the command:</span><span style="color: #bb6622">\n\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                          <span style="color: #766510">"loginctl unlock-sessions</span><span style="color: #bb6622">\n\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                          <span style="color: #766510">"Afterwards switch back to the running session (Ctrl+Alt+F%1)."</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">please update this part as the code got changed ;-)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25583" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">abstractlocker.cpp:62</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"log in as root and execute the command:</span><span style="color: #bb6622">\n\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"# ck-unlock-session <session-name></span><span style="color: #bb6622">\n\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"The <session-name> can be obtained by running the command:</span><span style="color: #bb6622">\n\n</span><span style="color: #766510">:"</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">cant we make the bash script just do what it is supposed to do?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25582" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">abstractlocker.cpp:67-70</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"Unfortunately your installation was compiled without support for</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"  1) loginctl</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"  2) consolekit</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                          <span style="color: #766510">"which could be used to unlock the session again."</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I would scrap that part. It's too technical and doesn't help the user.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25584" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ck-unlock-session.cmake:17</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="err">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="err">${dbussend_EXECUTABLE}</span> <span class="err">--system</span> <span class="err">--print-reply</span> <span class="err">--dest="org.freedesktop.ConsoleKit"</span> <span class="err">"/org/freedesktop/ConsoleKit/$1"</span> <span class="err">org.freedesktop.ConsoleKit.Session.Unlock</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="err">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Is this asking for the password in some way? Or do we now install a script which allows to bypass authentication? With logind one can configure the system to always require a password.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25585" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">FindConsoleKit.cmake:28</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">find_program</span><span class="p">(</span><span style="color: #766510">cklistsessions_EXECUTABLE</span> <span style="color: #766510">NAMES</span> <span style="color: #766510">ck-list-sessions</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">find_program</span><span class="p">(</span><span style="color: #766510">dbussend_EXECUTABLE</span> <span style="color: #766510">NAMES</span> <span style="color: #766510">dbus-send</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">find_package_handle_standard_args</span><span class="p">(</span><span style="color: #766510">ConsoleKit</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">just wondering: why dbus-send and not qdbus? qdbus would be a binary our session depends on anyway.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6004#inline-25586" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">config-kscreenlocker.h.cmake:17</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">#cmakedefine01 HAVE_LOGINCTL</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This also turns loginctl into a build time dependency.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R133 KScreenLocker</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D6004" rel="noreferrer">https://phabricator.kde.org/D6004</a></div></div><br /><div><strong>To: </strong>tcberner, FreeBSD, graesslin, Plasma<br /><strong>Cc: </strong>erichameleers, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas<br /></div>