D29451: KNS: Do not mark entry as installed if install script failed

[Dan Leinir Turthra Jensen]

leinir requested changes to this revision.
leinir added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> installation.cpp:355
>              QProcess* p = runPostInstallationCommand(installedFiles.size() == 1 ? installedFiles.first() : targetPath);
> -            connect(p, static_cast<void(QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, installationFinished);
> +            connect(p, static_cast<void(QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished),
> +                    [entry, installationFinished, this] (int exitCode, QProcess::ExitStatus) {

Remember to give connect an object context for the slot (i did not realise until recently what leaving that out means, but it turns out to be potentially quite bad and crashy in difficult to track ways - in short, if our this instance is destroyed (say, the user quits the application) while an installation is ongoing, this would crash when attempting to emit or call installationFinished - it /should be fine, as i said, in most cases, as installation's a long lifetime object, but also just good style to add that context - see https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_simple_function for a detailed explanation, but in short, just add `this,` before the lambda and you're good to go :) )

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29451

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200507/1b820c8b/attachment-0001.html>