Changeset View
Standalone View
src/core/desktopexecparser.cpp
| Show All 31 Lines | |||||
| 32 | #include <kmimetypetrader.h> | 32 | #include <kmimetypetrader.h> | ||
| 33 | 33 | | |||
| 34 | #include <QFile> | 34 | #include <QFile> | ||
| 35 | #include <QDir> | 35 | #include <QDir> | ||
| 36 | #include <QUrl> | 36 | #include <QUrl> | ||
| 37 | #include <QStandardPaths> | 37 | #include <QStandardPaths> | ||
| 38 | #include <QDBusConnection> | 38 | #include <QDBusConnection> | ||
| 39 | #include <QDBusReply> | 39 | #include <QDBusReply> | ||
| 40 | 40 | | |||
meven: Not needed. | |||||
| 41 | #include <config-kiocore.h> // CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 | 41 | #include <config-kiocore.h> // CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 | ||
| 42 | 42 | | |||
| 43 | #include "kiocoredebug.h" | 43 | #include "kiocoredebug.h" | ||
| 44 | 44 | | |||
| 45 | class KRunMX1 : public KMacroExpanderBase | 45 | class KRunMX1 : public KMacroExpanderBase | ||
| 46 | { | 46 | { | ||
| 47 | public: | 47 | public: | ||
| 48 | explicit KRunMX1(const KService &_service) | 48 | explicit KRunMX1(const KService &_service) | ||
| ▲ Show 20 Lines • Show All 389 Lines • ▼ Show 20 Line(s) | 421 | if (d->service.terminal()) { | |||
| 438 | mx2.expandMacrosShellQuote(terminal); | 438 | mx2.expandMacrosShellQuote(terminal); | ||
| 439 | result = KShell::splitArgs(terminal); // assuming that the term spec never needs a shell! | 439 | result = KShell::splitArgs(terminal); // assuming that the term spec never needs a shell! | ||
| 440 | result << QStringLiteral("-e"); | 440 | result << QStringLiteral("-e"); | ||
| 441 | } | 441 | } | ||
| 442 | 442 | | |||
| 443 | KShell::Errors err; | 443 | KShell::Errors err; | ||
| 444 | QStringList execlist = KShell::splitArgs(exec, KShell::AbortOnMeta | KShell::TildeExpand, &err); | 444 | QStringList execlist = KShell::splitArgs(exec, KShell::AbortOnMeta | KShell::TildeExpand, &err); | ||
| 445 | if (err == KShell::NoError && !execlist.isEmpty()) { // mx1 checked for syntax errors already | 445 | if (err == KShell::NoError && !execlist.isEmpty()) { // mx1 checked for syntax errors already | ||
| 446 | const QString executable = execlist.at(0); | ||||
| 447 | if (QDir::isRelativePath(executable)) { | ||||
| 446 | // Resolve the executable to ensure that helpers in libexec are found. | 448 | // Resolve the executable to ensure that helpers in libexec are found. | ||
| 447 | // Too bad for commands that need a shell - they must reside in $PATH. | 449 | // Too bad for commands that need a shell - they must reside in $PATH. | ||
| 448 | QString exePath = QStandardPaths::findExecutable(execlist.first()); | 450 | QString exePath = QStandardPaths::findExecutable(executable); | ||
| 449 | if (exePath.isEmpty()) { | 451 | if (exePath.isEmpty()) { | ||
| 450 | exePath = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/") + execlist.first(); | 452 | exePath = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/") + executable; | ||
| 451 | } | 453 | } | ||
| 452 | if (QFile::exists(exePath)) { | 454 | if (QFile::exists(exePath)) { | ||
| 453 | execlist[0] = exePath; | 455 | execlist[0] = exePath; | ||
| 454 | } | 456 | } | ||
| 455 | } | 457 | } | ||
| 458 | } | ||||
| 456 | if (d->service.substituteUid()) { | 459 | if (d->service.substituteUid()) { | ||
| 457 | if (d->service.terminal()) { | 460 | if (d->service.terminal()) { | ||
| 458 | result << QStringLiteral("su"); | 461 | result << QStringLiteral("su"); | ||
| 459 | } else { | 462 | } else { | ||
| 460 | QString kdesu = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/kdesu"); | 463 | QString kdesu = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/kdesu"); | ||
| 461 | if (!QFile::exists(kdesu)) { | 464 | if (!QFile::exists(kdesu)) { | ||
I think we should check for isExecutable() here too (this matches the behaviour of QStandardPaths::findExecutable()). ahmadsamir: I think we should check for isExecutable() here too (this matches the behaviour of… | |||||
But if we do that, a non-executable file gets ignored here and we don't get the warning that this commit is all about. It would all be much simpler [for this custom purpose] if findExecutable didn't check the executable bit ;-) Then we'd have one full path to check ourselves. Instead we have to duplicate the PATH lookup. I'd like to avoid having to also duplicate the current-dir-of-desktop-file lookup and the libexec lookup. dfaure: But if we do that, a non-executable file gets ignored here and we don't get the warning that… | |||||
Righto. The isExecutable() check is done in KProcessRunner. And I echo that request, findExecutable() would be slightly more useful if it reports that it found a file with that name but it's not executable; the QStandardPaths API doesn't offer any other way of searching PATH, so the only method it has should report such a case. I am not sure if upstream would take a patch for that... ahmadsamir: > But if we do that, a non-executable file gets ignored here and we don't get the warning that… | |||||
| 462 | kdesu = QStandardPaths::findExecutable(QStringLiteral("kdesu")); | 465 | kdesu = QStandardPaths::findExecutable(QStringLiteral("kdesu")); | ||
| 463 | } | 466 | } | ||
| 464 | if (!QFile::exists(kdesu)) { | 467 | if (!QFile::exists(kdesu)) { | ||
| 465 | // Insert kdesu as string so we show a nice warning: 'Could not launch kdesu' | 468 | // Insert kdesu as string so we show a nice warning: 'Could not launch kdesu' | ||
| 466 | result << QStringLiteral("kdesu"); | 469 | result << QStringLiteral("kdesu"); | ||
| 467 | return result; | 470 | return result; | ||
| 468 | } else { | 471 | } else { | ||
| 469 | result << kdesu << QStringLiteral("-u"); | 472 | result << kdesu << QStringLiteral("-u"); | ||
| ▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines | |||||
Not needed.