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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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 77026

View Options

src/runtime/kserviceactioncomponent.cpp

Loading...