[education/rkward] /: Fix assorted problems around cancelling package installation.
Thomas Friedrichsmeier
null at kde.org
Sun May 1 21:37:49 BST 2022
Git commit 2046c45059d828c7b7ef7807c4edd6effba1a994 by Thomas Friedrichsmeier.
Committed on 01/05/2022 at 12:13.
Pushed by tfry into branch 'master'.
Fix assorted problems around cancelling package installation.
Importantly, packages are now installed without a helper script, when installing as regular user.
(When installing as root, the helper script is created from with R itself.)
M +6 -1 ChangeLog
M +34 -47 rkward/dialogs/rkloadlibsdialog.cpp
M +1 -3 rkward/dialogs/rkloadlibsdialog.h
M +33 -7 rkward/misc/rkprogresscontrol.cpp
M +3 -1 rkward/misc/rkprogresscontrol.h
M +1 -0 rkward/rbackend/rkrbackend.cpp
M +2 -2 rkward/rbackend/rkrinterface.cpp
https://invent.kde.org/education/rkward/commit/2046c45059d828c7b7ef7807c4edd6effba1a994
diff --git a/ChangeLog b/ChangeLog
index c8950a2f..26af6786 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,13 @@
TODOs:
- More tolerant handshake on Windows? Simply a matter of allowing more time?
+- Fixed some problems with cancelling running commands
+- Package installation uses inline widget to provide progress feedback, instead of separate dialogs
+ - TODO: Finding package from require() fails, because list is not yet loaded
+ - TODO: Remove "Ok" button from dialog
+ - TODO: Provide separate message texts (and icons?) for running, finished successfully, finished with error
+ - TODO: Remove current repositories from .rk.get.package.installation.status() (no longer needed)
- Package installation no longer uses an external process, unless required for root permissions
- - TODO: Trim down required commands, show what's left, inline
- TODO: swallow installation output in require()
- Fixed: Package installation as root would fail to find kdesu/kdesudo on recent systems
- Fixed: R help pages would refuse to open in new tab
diff --git a/rkward/dialogs/rkloadlibsdialog.cpp b/rkward/dialogs/rkloadlibsdialog.cpp
index 60bfcf85..6d6c3642 100644
--- a/rkward/dialogs/rkloadlibsdialog.cpp
+++ b/rkward/dialogs/rkloadlibsdialog.cpp
@@ -236,15 +236,15 @@ bool RKLoadLibsDialog::removePackages (QStringList packages, QStringList from_li
#ifdef Q_OS_WIN
extern Q_CORE_EXPORT int qt_ntfs_permission_lookup;
#endif
-bool RKLoadLibsDialog::installPackages (const QStringList &packages, QString to_libloc, bool install_suggested_packages, const QStringList& repos) {
+bool RKLoadLibsDialog::installPackages (const QStringList &packages, QString to_libloc, bool install_suggested_packages) {
RK_TRACE (DIALOGS);
if (packages.isEmpty ()) return false;
bool as_root = false;
// It is ok, if the selected location does not yet exist. In order to know, whether we can write to it, we have to create it first.
- QDir().mkpath (to_libloc);
- QString altlibloc = RKSettingsModuleRPackages::addUserLibLocTo (library_locations).value (0);
+ QDir().mkpath(to_libloc);
+ QString altlibloc = RKSettingsModuleRPackages::addUserLibLocTo(library_locations).value(0);
#ifdef Q_OS_WIN
extern Q_CORE_EXPORT int qt_ntfs_permission_lookup;
qt_ntfs_permission_lookup++;
@@ -272,33 +272,18 @@ bool RKLoadLibsDialog::installPackages (const QStringList &packages, QString to_
if (res == KMessageBox::Cancel) return false;
}
- addLibraryLocation (to_libloc);
+ addLibraryLocation(to_libloc);
- QString command_string = RKSettingsModuleRPackages::libLocsCommand();
- command_string += "\ninstall.packages (c (\"" + packages.join ("\", \"") + "\")" + ", lib=" + RObject::rQuote (to_libloc);
+ QString command_string = "install.packages (c (\"" + packages.join("\", \"") + "\")" + ", lib=" + RObject::rQuote(to_libloc);
QString downloaddir = QDir (RKSettingsModuleGeneral::filesPath ()).filePath ("package_archive");
if (RKSettingsModuleRPackages::archivePackages ()) {
QDir (RKSettingsModuleGeneral::filesPath ()).mkdir ("package_archive");
command_string += ", destdir=" + RObject::rQuote (downloaddir);
}
if (install_suggested_packages) command_string += ", dependencies=TRUE";
- command_string += ")\n";
+ command_string += ")";
- QString repos_string = "options (repos= c(";
- for (int i = 0; i < repos.count (); ++i) {
- if (i) repos_string.append (", ");
- repos_string.append (RObject::rQuote (repos[i]));
- }
- repos_string.append ("))\n");
-
- repos_string.append (RKSettingsModuleRPackages::pkgTypeOption ());
-
- if (as_root) {
- KUser user;
- command_string.append ("system (\"chown " + user.loginName() + ' ' + downloaddir + "/*\")\n");
- }
-
- runInstallationCommand (repos_string + command_string, as_root, i18n ("Please stand by while installing selected packages"), i18n ("Installing packages"));
+ runInstallationCommand (command_string, as_root, i18n ("Please stand by while installing selected packages"), i18n ("Installing packages"));
return true;
}
@@ -306,16 +291,6 @@ bool RKLoadLibsDialog::installPackages (const QStringList &packages, QString to_
void RKLoadLibsDialog::runInstallationCommand (const QString& command, bool as_root, const QString& message, const QString& title) {
RK_TRACE (DIALOGS);
- QFile file(QDir(RKSettingsModuleGeneral::filesPath()).filePath("install_script.R"));
- if (file.open (QIODevice::WriteOnly)) {
- QTextStream stream (&file);
- stream << command;
- if (as_root) stream << "q()\n";
- file.close();
- } else {
- RK_ASSERT (false);
- }
-
auto control = new RKInlineProgressControl(install_packages_widget, true);
control->messageWidget()->setText(QString("<b>%1</b><p>%2</p>").arg(title, message));
control->show();
@@ -330,25 +305,39 @@ void RKLoadLibsDialog::runInstallationCommand (const QString& command, bool as_r
if (call.isEmpty ()) call = QStandardPaths::findExecutable("kdesudo", libexecpath);
if (call.isEmpty()) {
KMessageBox::sorry(this, i18n("Neither kdesu nor kdesudo could be found. To install as root, please install one of these packages."), i18n("kdesu not found"));
- file.remove();
return;
}
+
QStringList call_with_params(call);
- call_with_params << "-t" << "--" << RKSessionVars::RBinary() << "--no-save" << "--no-restore" << "--file=" + file.fileName();
- rcommand = new RCommand(QString("system(%1)").arg(RObject::rQuote(call_with_params.join(" "))), RCommand::App);
+ call_with_params << "-t" << "--" << RKSessionVars::RBinary() << "--no-save" << "--no-restore" << "--file=";
+ KUser user;
+ QString aux_command = QString("local({ "
+ "install_script <- tempfile(\".R\"); f <- file(install_script, \"w\")\n"
+ "repos <- options()$repos\n"
+ "pkgType <- options()$pkgType\n"
+ "libPaths <- .libPaths()\n"
+ "dump(c(\"repos\", \"pkgType\", \"libPaths\"), f)\n"
+ "cat(\"\\n\", file=f, append=TRUE)\n"
+ "cat(") + RObject::rQuote("options(\"repos\"=repos, \"pkgType\"=pkgType)\n") + QString(", file=f, append=TRUE)\n"
+ "cat(\".libPaths(libPaths)\\n\"") + QString(", file=f, append=TRUE)\n"
+ "cat(") + RObject::rQuote(command + "\n") + QString(", file=f, append=TRUE)\n"
+ "cat(") + RObject::rQuote("system(\"chown " + user.loginName() + ' ' + QDir(RKSettingsModuleGeneral::filesPath()).filePath("package_archive") + "/*\")") + QString(", file=f, append=TRUE)\n"
+ "cat(\"\\n\", file=f, append=TRUE)\n"
+ "close(f)\n"
+ "system(paste0(" + RObject::rQuote(call_with_params.join(' ')) + ", install_script))\n"
+ "unlink(install_script)\n"
+ "})");
+
+ rcommand = new RCommand(aux_command, RCommand::App);
} else {
- rcommand = new RCommand(QString("source(%1, local=new.env())").arg(RObject::rQuote(file.fileName())), RCommand::App);
+ rcommand = new RCommand(command, RCommand::App);
}
+
control->addRCommand(rcommand);
RInterface::issueCommand(rcommand, chain);
- bool done = false;
- connect(rcommand->notifier(), &RCommandNotifier::commandFinished, this, [&done]() { done = true; });
- while (!done) {
- QApplication::processEvents();
- }
-
- file.remove();
- emit installedPackagesChanged();
+ connect(rcommand->notifier(), &RCommandNotifier::commandFinished, this, [this]() {
+ emit installedPackagesChanged();
+ });
}
////////////////////// LoadUnloadWidget ////////////////////////////
@@ -763,7 +752,7 @@ void InstallPackagesWidget::doInstall (bool refresh) {
if (!install.isEmpty ()) {
QString dest = install_params->installLocation ();
if (!dest.isEmpty ()) {
- changed |= parent->installPackages (install, dest, install_params->installSuggestedPackages (), packages_status->currentRepositories ());
+ changed |= parent->installPackages (install, dest, install_params->installSuggestedPackages ());
}
}
@@ -941,8 +930,6 @@ void RKRPackageInstallationStatus::statusCommandFinished (RCommand *command) {
installed_has_update[updateable_packages_in_installed[i]] = true;
}
- current_repos = top[4]->stringVector ();
-
clearStatus ();
}
diff --git a/rkward/dialogs/rkloadlibsdialog.h b/rkward/dialogs/rkloadlibsdialog.h
index 93a2456a..9577cd9b 100644
--- a/rkward/dialogs/rkloadlibsdialog.h
+++ b/rkward/dialogs/rkloadlibsdialog.h
@@ -46,7 +46,7 @@ public:
~RKLoadLibsDialog ();
- bool installPackages (const QStringList &packages, QString to_libloc, bool install_suggested_packages, const QStringList& repos);
+ bool installPackages (const QStringList &packages, QString to_libloc, bool install_suggested_packages);
bool removePackages (QStringList packages, QStringList from_liblocs);
/** opens a modal RKLoadLibsDialog with the "Install new Packages" tab on front (To be used when a require () fails in the R backend
@@ -179,7 +179,6 @@ public:
QModelIndex markAllUpdatesForInstallation ();
/** reset all installation states to NoAction */
void clearStatus ();
- QStringList currentRepositories () const { return current_repos; };
bool initialized () const { return _initialized; };
private slots:
void statusCommandFinished (RCommand *command);
@@ -196,7 +195,6 @@ private:
QVector<bool> installed_has_update;
bool _initialized;
- QStringList current_repos;
QWidget *display_area;
};
diff --git a/rkward/misc/rkprogresscontrol.cpp b/rkward/misc/rkprogresscontrol.cpp
index dfed16ad..ce6dd3ff 100644
--- a/rkward/misc/rkprogresscontrol.cpp
+++ b/rkward/misc/rkprogresscontrol.cpp
@@ -376,11 +376,13 @@ void RKProgressControlDialog::closeEvent (QCloseEvent *e) {
}
#include <KMessageWidget>
+#include <KMessageBox>
#include <KStandardAction>
RKInlineProgressControl::RKInlineProgressControl(QWidget *display_area, bool allow_cancel) : QObject(display_area),
autoclose(false),
allow_cancel(allow_cancel),
+ is_done(false),
display_area(display_area),
prevent_close_message(nullptr),
close_action(nullptr) {
@@ -407,23 +409,29 @@ RKInlineProgressControl::RKInlineProgressControl(QWidget *display_area, bool all
RKInlineProgressControl::~RKInlineProgressControl() {
RK_TRACE(MISC);
- display_area->window()->removeEventFilter(this);
delete wrapper;
- for (int i = 0; i < unfinished_commands.size(); ++i) {
- RInterface::instance()->cancelCommand(unfinished_commands[i]);
- }
}
void RKInlineProgressControl::setCloseAction(const QString &label) {
RK_TRACE(MISC);
if (!close_action) {
- close_action = KStandardAction::create(KStandardAction::Close, this, &QObject::deleteLater, this);
+ close_action = KStandardAction::create(KStandardAction::Close, this, &RKInlineProgressControl::cancelAndClose, this);
message_widget->addAction(close_action);
}
close_action->setText(label);
}
+void RKInlineProgressControl::cancelAndClose() {
+ RK_TRACE(MISC);
+
+ display_area->window()->removeEventFilter(this);
+ for (int i = 0; i < unfinished_commands.size(); ++i) {
+ RInterface::instance()->cancelCommand(unfinished_commands[i]);
+ }
+ deleteLater();
+}
+
void RKInlineProgressControl::addRCommand(RCommand *command) {
RK_TRACE(MISC);
unfinished_commands.append(command);
@@ -459,6 +467,7 @@ void RKInlineProgressControl::done() {
} else {
setCloseAction(i18n("Close"));
}
+ is_done = true;
}
void RKInlineProgressControl::show(int delay_ms) {
@@ -478,9 +487,26 @@ bool RKInlineProgressControl::eventFilter(QObject *, QEvent *e) {
});
return false;
}
- if (!allow_cancel && e->type() == QEvent::Close) {
+ if ((e->type() == QEvent::Close) && !is_done) {
+ if (allow_cancel) {
+ bool ignore = (KMessageBox::warningContinueCancel(display_area, i18n("Closing this window will cancel the current operation. Are you sure?"), i18n("Cancel operation"), KGuiItem(i18n("Keep waiting")), KGuiItem(i18n("Cancel && Close"))) == KMessageBox::Continue);
+ if (ignore) {
+ e->ignore();
+ return true;
+ }
+ cancelAndClose();
+ return false;
+ }
// TODO
- prevent_close_message = new KMessageWidget(i18n("An operation is still running, please wait."));
+ if (!prevent_close_message) {
+ prevent_close_message = new KMessageWidget(i18n("An operation is still running, please wait."), display_area->window());
+ }
+ QSize s = prevent_close_message->sizeHint();
+ prevent_close_message->resize(display_area->window()->width(), s.height());
+ prevent_close_message->setMessageType(KMessageWidget::Error);
+ prevent_close_message->animatedShow();
+ QTimer::singleShot(5000, prevent_close_message, &KMessageWidget::animatedHide);
+ e->ignore();
return true;
}
return false;
diff --git a/rkward/misc/rkprogresscontrol.h b/rkward/misc/rkprogresscontrol.h
index 52c028d2..4dfab752 100644
--- a/rkward/misc/rkprogresscontrol.h
+++ b/rkward/misc/rkprogresscontrol.h
@@ -109,13 +109,15 @@ public:
void setAutoCloseWhenCommandsDone(bool _autoclose) { autoclose = _autoclose; };
void show(int delay_ms=0);
KMessageWidget *messageWidget() { return message_widget; };
+private:
void addOutput(const QString &output, bool is_error_warning);
void done();
-private:
void setCloseAction(const QString &label);
bool eventFilter(QObject *, QEvent *e) override;
+ void cancelAndClose();
bool autoclose;
bool allow_cancel;
+ bool is_done;
QWidget* wrapper;
QWidget* display_area;
KMessageWidget *message_widget;
diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index f4f1aa71..2c86cb90 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -129,6 +129,7 @@ void RKRBackend::scheduleInterrupt () {
void RKRBackend::interruptCommand (int command_id) {
RK_TRACE (RBACKEND);
+ RK_DEBUG(RBACKEND, DL_DEBUG, "Received interrupt request for command id %d", command_id);
QMutexLocker lock (&all_current_commands_mutex);
if (all_current_commands.isEmpty ()) return;
diff --git a/rkward/rbackend/rkrinterface.cpp b/rkward/rbackend/rkrinterface.cpp
index 575e24b3..95d6eb29 100644
--- a/rkward/rbackend/rkrinterface.cpp
+++ b/rkward/rbackend/rkrinterface.cpp
@@ -524,7 +524,7 @@ void RInterface::cancelAll () {
bool RInterface::softCancelCommand (RCommand* command) {
RK_TRACE (RBACKEND);
- if (!(command->type () & RCommand::Running)) {
+ if (!(command->status & RCommand::Running)) {
cancelCommand (command);
}
return command->status & RCommand::Canceled;
@@ -535,7 +535,7 @@ void RInterface::cancelCommand (RCommand *command) {
if (!(command->type () & RCommand::Sync)) {
command->status |= RCommand::Canceled;
- if (command->type () & RCommand::Running) {
+ if (command->status & RCommand::Running) {
if ((RKDebugHandler::instance ()->state () == RKDebugHandler::InDebugPrompt) && (command == RKDebugHandler::instance ()->command ())) {
RKDebugHandler::instance ()->sendCancel ();
} else {
More information about the rkward-tracker
mailing list