D29170: Detect executables without +x permission in $PATH to improve error message

[David Faure]

dfaure marked an inline comment as done.
dfaure added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in kprocessrunner.cpp:53
> KProcessRunner tries finding the executable in the current dir too, so to be precise in the reported error message maybe append currentDir.absolutePath() to searchPaths?
> 
> Also KProcessRunner only checks that the executable exists in the current dir, "!QFileInfo::exists(realExecutable)", it should also check that it's executable. So that KProcessRunner checks the "exists and is +x" and here we check "exists but not +x", if that makes sense.
> 
> I guess you'll have the add an ifdef kludge since QString::SkipEmptyParts is deprecated in Qt 5.15 according to https://lxr.kde.org/source/frameworks/frameworkintegration/src/kpackage-install-handlers/kns/main.cpp#0068 (I am still on 5.14).

In a GUI program started graphically, the notion of "current dir" means very little. You can't see it, you can't change it.
Granted, when starting it from the command line it does serve a purpose for command line arguments, but IMHO not after that.

What's more, here we're executing a KService i.e. usually a desktop file (unless it was constructed from an executable name, display name and icon).

Hmm OK I can make a testcase for it with a special case. A copy of dolphin's desktop file with Exec=dolphin2 and a copy of dolphin called dolphin2 in the same directory, and starting dolphin from that directory. Interesting, it leads to "execvp: Permission denied", that's a new one :)
(must be from QProcess).

My next path will fix that testcase, but it remains an unexpected corner case. The same dolphin started from $HOME and then navigating to that directory, cannot start that desktop file.
Maybe we want to look for executables relative to the desktop file, rather than from the hidden-to-the-user current directory... Actually I remember people asking for that to work, a VERY long time ago on the freedesktop xdg mailing-list. IIRC I even made it work back then.

Thanks for the SkipEmptyParts information and for noticing the weird if() -- fixed.

> ahmadsamir wrote in kprocessrunner.cpp:60
> Should be https://

I was counting on the redirection :-)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29170

To: dfaure, ahmadsamir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200425/e8112971/attachment-0001.html>