Review Request 119154: Install knetattach regularly instead of into libexec

[Eike Hein]

> On July 7, 2014, 12:01 p.m., Aleix Pol Gonzalez wrote:
> > knetattach/CMakeLists.txt, line 13
> > <https://git.reviewboard.kde.org/r/119154/diff/1/?file=288163#file288163line13>
> >
> >     We've used configure variables to do that. Doesn't it apply here?
> 
> Eike Hein wrote:
>     Can you elaborate?
> 
> Aleix Pol Gonzalez wrote:
>     See how kcheckpass is being found now:
>     ksmserver/config-ksmserver.h.cmake:#define KCHECKPASS_BIN "${CMAKE_INSTALL_PREFIX}/${LIBEXEC_INSTALL_DIR}/kcheckpass"
>     
>     So who should call knetattach? The attached bug report seems the wrong one...
> 
> Eike Hein wrote:
>     No, the attached bug report is the correct one from what I can tell, and contains the explanation of all that, as does the review request actually :).
>     
>     The kcheckpass example doesn't apply (nor do other examples like kfontinst) because these are being executed somewhere close to those definitions, e.g. directly using QProcess.
>     
>     In the case of knetattach, though, we're relying on its .desktop file for execution, which just specifies "knetattach" as Exec. It pops up in launchers and in the listing produced by the remote:/ kioslave, which generatea a KIO::UDSEntry pointing at the .desktop file's local path.
>     
>     Unless you're saying we should bake an absolut path into the .desktop file ...?
> 
> Aleix Pol Gonzalez wrote:
>     Or not using the desktop file...? Well, I guess if we tackle that, then the complexity of the patch implodes.
>     
>     Let's just move knetattach to the PATH, since it seems it will end up being called from different places and Qt doesn't support libexec.
> 
> Eike Hein wrote:
>     > Or not using the desktop file...? Well, I guess if we tackle that, then the complexity of the patch implodes.
>     
>     That'd require refactoring the remote:/ kioslave to handle execution itself, and either dropping knetattach from menus, or trying to get libexec back into .desktop execution somehow ...
>     
>     
>     > Let's just move knetattach to the PATH, since it seems it will end up being called from different places and Qt doesn't support libexec.
>     
>     Yeah that was my conclusion too ...
> 
> Aleix Pol Gonzalez wrote:
>     Is knetattach on the menus? If so there's no reason for it to be in libexec.

Yup, it always has been; as mentioned the difference is that KStandardDirs::findExecutable() used to be able to locate it in libexec in 4.


- Eike


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


On July 7, 2014, 11:33 a.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119154/
> -----------------------------------------------------------
> 
> (Updated July 7, 2014, 11:33 a.m.)
> 
> 
> Review request for Plasma and David Faure.
> 
> 
> Bugs: 337167
>     https://bugs.kde.org/show_bug.cgi?id=337167
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> We currently have multiple launch locations for KNetAttach that are broken because the .desktop execution machinery lost the ability to execute things from libexec (probably due to KStandardDirs -> QStandardPaths, KStandardDirs::findExecutable used to work differently). Namely the regular menu entry, and the "Add Network Folder" virtual item generated by the remote:/ kioslave (showing as "Network" in the Dolphin sidebar).
> 
> This patch changes the build system to install knetattach regularly, to match that we apparently want to be able to launch it regularly.
> 
> See https://bugs.kde.org/show_bug.cgi?id=337117 for ticket.
> 
> 
> Diffs
> -----
> 
>   knetattach/CMakeLists.txt e3853ec 
> 
> Diff: https://git.reviewboard.kde.org/r/119154/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140707/b3cf8797/attachment-0001.html>