Differential D27863

Fix "Make it compile against last qt5.15 without deprecated method. QProcess::execute(QString) is deprecated"
ClosedPublic
Actions

Authored by fvogt on Mar 5 2020, 10:46 AM.

Details

Reviewers
mlaurent
Group Reviewers
Frameworks
Commits
R268:d2df38f1c4f4: GIT_SILENT Upgrade KF5 version to 5.68.0.
R268:8e14750977c6: Fix "Make it compile against last qt5.15 without deprecated method. QProcess…
Summary

This port is broken AFAICT - it tries to run the full Exec= line as binary, without
splitting arguments first.

This reverts commit 59cbea835502428f30c1495abe4a1b3d133103e3.

Test Plan

Builds fine.

Diff Detail

Repository
R268 KGlobalAccel
Branch
somefix
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23310
Build 23328: arc lint + arc unit
fvogt created this revision.Mar 5 2020, 10:46 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 5 2020, 10:46 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
fvogt requested review of this revision.Mar 5 2020, 10:46 AM
mlaurent added inline comments.Mar 5 2020, 12:00 PM
src/runtime/kserviceactioncomponent.cpp
72

Hum... interesting:

#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
QProcess::startDetached(m_desktopFile.desktopGroup().readEntry(QStringLiteral("Exec"), QString()));
#else
const QString commandLine = m_desktopFile.desktopGroup().readEntry(QStringLiteral("Exec"), QString());
if (!commandLine.isEmpty()) {

QStringList arguments = QProcess::splitCommand(commandLine);
const QString prog = arguments.takeFirst();
QProcess::startDetached(prog, arguments);

}
#endif

ahmadsamir added a subscriber: ahmadsamir.EditedMar 5 2020, 12:12 PM

Looking at upstream code, it looks like they introduced a static splitCommad() method since 5.15, so I think we'll have to use a
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)

kludge to handle this case. Their rationale is more precise argument handling, which sort of makes sense:
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=92eea633491ce8138c5caceb904ad26c1eb91044
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=2090b770daa87039f27c15aad6bdbc42e5fe702c

EDIT: too late, I took too long to type :D

fvogt added a comment.Mar 5 2020, 12:15 PM

The split arguments are already available as parts above, as used in the klauncher call AFAICT.

mlaurent added a comment.Mar 5 2020, 1:09 PM

if splitting is already done why this code re-call "m_desktopFile.desktopGroup().readEntry(QStringLiteral("Exec"), QString())" ?

> QProcess::startDetached(commands, parts) no ?

fvogt added a comment.Mar 5 2020, 1:39 PM
In D27863#622655, @mlaurent wrote:

if splitting is already done why this code re-call "m_desktopFile.desktopGroup().readEntry(QStringLiteral("Exec"), QString())" ?
=> QProcess::startDetached(commands, parts) no ?

I would assume that works, yes.

fvogt updated this revision to Diff 77022.Mar 5 2020, 1:42 PM

Do it differently, just like it's done below

fvogt retitled this revision from Revert "Make it compile against last qt5.15 without deprecated method. QProcess::execute(QString) is deprecated" to Fix "Make it compile against last qt5.15 without deprecated method. QProcess::execute(QString) is deprecated".Mar 5 2020, 1:42 PM
fvogt edited the test plan for this revision. (Show Details)Mar 5 2020, 1:45 PM
fvogt marked an inline comment as done.
mlaurent accepted this revision.Mar 5 2020, 1:58 PM
This revision is now accepted and ready to land.Mar 5 2020, 1:58 PM
Closed by commit R268:8e14750977c6: Fix "Make it compile against last qt5.15 without deprecated method. QProcess… (authored by fvogt). · Explain WhyMar 5 2020, 2:02 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 77022

View Options

src/runtime/kserviceactioncomponent.cpp

Loading...