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

dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29170 To: dfaure, ahmadsamir Cc: ngraham, meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

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

dfaure added a comment. In D29170#658605 , @ngraham wrote: > What does this mean for AppImages? They are not desktop files, they don't come into this code path (which takes a KService as input). Clicking on a +x AppImage asks for

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

ngraham added a comment. What does this mean for AppImages? REPOSITORY R241 KIO BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 To: dfaure, ahmadsamir Cc: ngraham, meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

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

dfaure updated this revision to Diff 81377. dfaure added a comment. Remove unused QFileInfo include REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81376=81377 BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170

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

dfaure updated this revision to Diff 81376. dfaure added a comment. Remove QDir::current REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81275=81376 BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 AFFECTED FILES

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

dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in kprocessrunner.cpp:65 > I guess there is no need to use QDir::current() any more. Good point, removing. REPOSITORY R241 KIO BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 To: dfaure,

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

dfaure added a comment. In D29170#658058 , @meven wrote: > Is it not sufficient to fix bug 415567 ? In which case replace in commit comment CCBUG by BUG No, that bug has two issues. "Program not found" and "Missing lib" -- which is

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

meven added a comment. Is it not sufficient to fix bug 415567 ? In which case replace in commit comment CCBUG by BUG INLINE COMMENTS > desktopexecparser.cpp:40 > #include > +#include > Not needed. REPOSITORY R241 KIO BRANCH 2020_04_findExecutable REVISION DETAIL

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

ahmadsamir accepted this revision. ahmadsamir added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kprocessrunner.cpp:65 > +const QStringList searchPaths = > QString::fromLocal8Bit(qgetenv("PATH")).split(QDir::listSeparator(), > skipEmptyParts); > +

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

dfaure updated this revision to Diff 81275. dfaure added a comment. Remove support for relative executables, as discussed REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81228=81275 BRANCH 2020_04_findExecutable REVISION DETAIL

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

ahmadsamir added a comment. In D29170#657689 , @dfaure wrote: > In D29170#657450 , @ahmadsamir wrote: > > > I don't think you need to go out of your way to support custom setups, after all it's

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

dfaure added a comment. In D29170#657450 , @ahmadsamir wrote: > I don't think you need to go out of your way to support custom setups, after all it's quite simple for the user to edit the .desktop file and specify the path to the executable.

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

dfaure updated this revision to Diff 81228. dfaure added a comment. add missing braces REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81213=81228 BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 AFFECTED FILES

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

dfaure added a comment. Meanwhile I sent an email to kde-frameworks-devel to get more input on the question. Supporting custom setups is a great feature for a powerful desktop environment. E.g. I deploy custom self-built apps for my users, so I use some of those custom KDE features. Not

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

ahmadsamir added a comment. In D29170#657334 , @dfaure wrote: > Resolve relative executables using the directory of the .desktop file referring to them > > Not useful for /usr/share/applications stuff, but useful for custom setups > where

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

dfaure updated this revision to Diff 81213. dfaure added a comment. Resolve relative executables using the directory of the .desktop file referring to them Not useful for /usr/share/applications stuff, but useful for custom setups where a custom desktop file refers to a local

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

dfaure updated this revision to Diff 81209. dfaure added a comment. Better support for relative vs absolute executable names, with unittest. But I'll look into relative-to-desktop-file instead of relative-to-QDir::current. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

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

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

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

ahmadsamir added inline comments. INLINE COMMENTS > kprocessrunner.cpp:53 > +// This is a *very* simplified version of > QStandardPaths::findExecutable > +const QStringList searchPaths = > QString::fromLocal8Bit(qgetenv("PATH")).split(QDir::listSeparator(), >

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

dfaure created this revision. dfaure added a reviewer: ahmadsamir. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY QStandardPaths::findExecutable will not return to us a non-executable binary. So