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 | #include <QFileInfo> | ||||
meven: Not needed. | |||||
| 40 | 41 | | |||
| 41 | #include <config-kiocore.h> // CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 | 42 | #include <config-kiocore.h> // CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 | ||
| 42 | 43 | | |||
| 43 | #include "kiocoredebug.h" | 44 | #include "kiocoredebug.h" | ||
| 44 | 45 | | |||
| 45 | class KRunMX1 : public KMacroExpanderBase | 46 | class KRunMX1 : public KMacroExpanderBase | ||
| 46 | { | 47 | { | ||
| 47 | public: | 48 | public: | ||
| ▲ Show 20 Lines • Show All 390 Lines • ▼ Show 20 Line(s) | 422 | if (d->service.terminal()) { | |||
| 438 | mx2.expandMacrosShellQuote(terminal); | 439 | mx2.expandMacrosShellQuote(terminal); | ||
| 439 | result = KShell::splitArgs(terminal); // assuming that the term spec never needs a shell! | 440 | result = KShell::splitArgs(terminal); // assuming that the term spec never needs a shell! | ||
| 440 | result << QStringLiteral("-e"); | 441 | result << QStringLiteral("-e"); | ||
| 441 | } | 442 | } | ||
| 442 | 443 | | |||
| 443 | KShell::Errors err; | 444 | KShell::Errors err; | ||
| 444 | QStringList execlist = KShell::splitArgs(exec, KShell::AbortOnMeta | KShell::TildeExpand, &err); | 445 | QStringList execlist = KShell::splitArgs(exec, KShell::AbortOnMeta | KShell::TildeExpand, &err); | ||
| 445 | if (err == KShell::NoError && !execlist.isEmpty()) { // mx1 checked for syntax errors already | 446 | if (err == KShell::NoError && !execlist.isEmpty()) { // mx1 checked for syntax errors already | ||
| 447 | const QString executable = execlist.at(0); | ||||
| 448 | if (QDir::isRelativePath(executable)) { | ||||
| 449 | bool resolved = false; | ||||
| 450 | // Resolve executables relative to the .desktop file | ||||
| 451 | if (QDir::isAbsolutePath(d->service.entryPath())) { | ||||
| 452 | const QString localCandidate = QFileInfo(d->service.entryPath()).absolutePath() + QLatin1Char('/') + executable; | ||||
| 453 | if (QFileInfo::exists(localCandidate)) { | ||||
| 454 | execlist[0] = localCandidate; | ||||
| 455 | resolved = true; | ||||
| 456 | } | ||||
| 457 | } | ||||
| 458 | if (!resolved) { | ||||
| 446 | // Resolve the executable to ensure that helpers in libexec are found. | 459 | // 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. | 460 | // Too bad for commands that need a shell - they must reside in $PATH. | ||
| 448 | QString exePath = QStandardPaths::findExecutable(execlist.first()); | 461 | QString exePath = QStandardPaths::findExecutable(executable); | ||
| 449 | if (exePath.isEmpty()) { | 462 | if (exePath.isEmpty()) { | ||
| 450 | exePath = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/") + execlist.first(); | 463 | exePath = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/") + executable; | ||
| 451 | } | 464 | } | ||
| 452 | if (QFile::exists(exePath)) { | 465 | if (QFile::exists(exePath)) { | ||
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… | |||||
| 453 | execlist[0] = exePath; | 466 | execlist[0] = exePath; | ||
| 454 | } | 467 | } | ||
| 455 | } | 468 | } | ||
| 469 | } | ||||
| 470 | } | ||||
| 456 | if (d->service.substituteUid()) { | 471 | if (d->service.substituteUid()) { | ||
| 457 | if (d->service.terminal()) { | 472 | if (d->service.terminal()) { | ||
| 458 | result << QStringLiteral("su"); | 473 | result << QStringLiteral("su"); | ||
| 459 | } else { | 474 | } else { | ||
| 460 | QString kdesu = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/kdesu"); | 475 | QString kdesu = QFile::decodeName(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 "/kdesu"); | ||
| 461 | if (!QFile::exists(kdesu)) { | 476 | if (!QFile::exists(kdesu)) { | ||
| 462 | kdesu = QStandardPaths::findExecutable(QStringLiteral("kdesu")); | 477 | kdesu = QStandardPaths::findExecutable(QStringLiteral("kdesu")); | ||
| 463 | } | 478 | } | ||
| ▲ Show 20 Lines • Show All 47 Lines • Show Last 20 Lines | |||||
Not needed.