From 739fc57bbbe834e92fd2ca6edd36d468068162d7 Mon Sep 17 00:00:00 2001 From: Pawel Sikorski Date: Tue, 11 Aug 2015 14:25:10 +0200 Subject: [PATCH] Step::undo can also return ERROR. Since AppInstaller is calling all "undo()" even if one of them returns ERROR, Step::undo can return ERROR. Commit adds returning ERROR from some of the Step::undo methods. Change-Id: I989521c43ed6a603e24c06adb58c35fb1e0ed6c9 --- src/common/app_installer.cc | 2 +- src/common/step/step_backup_icons.cc | 2 +- src/common/step/step_copy_storage_directories.cc | 11 ++++++----- src/common/step/step_register_app.cc | 8 ++++++-- src/common/step/step_remove_files.cc | 2 +- src/common/step/step_remove_icons.cc | 4 +++- src/common/step/step_unregister_app.cc | 3 +-- src/tpk/step/step_copy_manifest_xml.cc | 5 ++++- src/tpk/step/step_create_symbolic_link.cc | 14 +++++++++++--- 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/common/app_installer.cc b/src/common/app_installer.cc index 10a55ac..7eba068 100644 --- a/src/common/app_installer.cc +++ b/src/common/app_installer.cc @@ -70,7 +70,7 @@ int AppInstaller::Run() { LOG(ERROR) << "Failure occurs"; do { if ((*it)->undo() != Step::Status::OK) { - LOG(ERROR) << "Error during undo operation"; + LOG(ERROR) << "Error during undo operation, but continuing...!"; ret = -2; } } while (it-- != itStart); diff --git a/src/common/step/step_backup_icons.cc b/src/common/step/step_backup_icons.cc index f77f6ff..e02b1ea 100755 --- a/src/common/step/step_backup_icons.cc +++ b/src/common/step/step_backup_icons.cc @@ -53,7 +53,7 @@ Step::Status StepBackupIcons::undo() { for (auto& pair : icons_) { if (!MoveFile(pair.second, pair.first)) { LOG(ERROR) << "Cannot revert icon from backup: " << pair.first; - // undo cannot fail, so no break + return Status::ERROR; } } LOG(DEBUG) << "Icons reverted from backup"; diff --git a/src/common/step/step_copy_storage_directories.cc b/src/common/step/step_copy_storage_directories.cc index 43d8992..e230f3e 100644 --- a/src/common/step/step_copy_storage_directories.cc +++ b/src/common/step/step_copy_storage_directories.cc @@ -66,21 +66,22 @@ common_installer::Step::Status StepCopyStorageDirectories::process() { } common_installer::Step::Status StepCopyStorageDirectories::undo() { + common_installer::Step::Status ret = Status::OK; if (!MoveAppStorage(context_->pkg_path.get(), backup_path_, kDataLocation)) { - LOG(ERROR) << "Failed to restore private directory for widget in update"; -// return Status::ERROR; // undo cannot fail... + LOG(ERROR) << "Failed to restore private directory for package in update"; + ret = Status::ERROR; } if (!MoveAppStorage(context_->pkg_path.get(), backup_path_, kSharedLocation)) { - LOG(ERROR) << "Failed to restore shared directory for widget in update"; -// return Status::ERROR; // undo cannot fail... + LOG(ERROR) << "Failed to restore shared directory for package in update"; + ret = Status::ERROR; } - return Status::OK; + return ret; } } // namespace filesystem diff --git a/src/common/step/step_register_app.cc b/src/common/step/step_register_app.cc index 5391e1b..0b050b6 100644 --- a/src/common/step/step_register_app.cc +++ b/src/common/step/step_register_app.cc @@ -46,8 +46,12 @@ Step::Status StepRegisterApplication::process() { } Step::Status StepRegisterApplication::undo() { - UnregisterAppInPkgmgr(context_->xml_path.get(), context_->pkgid.get(), - context_->uid.get()); + if (!UnregisterAppInPkgmgr(context_->xml_path.get(), context_->pkgid.get(), + context_->uid.get())) { + LOG(ERROR) << "Application couldn't be unregistered"; + return Status::ERROR; + } + LOG(INFO) << "Successfuly clean database"; return Status::OK; } diff --git a/src/common/step/step_remove_files.cc b/src/common/step/step_remove_files.cc index a10bdb5..b52fbed 100755 --- a/src/common/step/step_remove_files.cc +++ b/src/common/step/step_remove_files.cc @@ -57,7 +57,7 @@ Step::Status StepRemoveFiles::undo() { LOG(DEBUG) << "Restoring directory: " << context_->pkg_path.get(); if (!MoveDir(backup_path, context_->pkg_path.get())) { LOG(ERROR) << "Cannot restore widget files"; - return Status::OK; + return Status::ERROR; } } diff --git a/src/common/step/step_remove_icons.cc b/src/common/step/step_remove_icons.cc index 6cb3d9e..327d6ee 100644 --- a/src/common/step/step_remove_icons.cc +++ b/src/common/step/step_remove_icons.cc @@ -63,16 +63,18 @@ Step::Status StepRemoveIcons::clean() { } Step::Status StepRemoveIcons::undo() { + Step::Status ret = Status::OK; if (!backups_.empty()) { LOG(DEBUG) << "Restoring icons files..."; for (auto& pair : backups_) { if (!MoveFile(pair.first, pair.second)) { LOG(ERROR) << "Failed to restore: " << pair.second; // We need to try to restore all icons anyway... + ret = Status::ERROR; } } } - return Status::OK; + return ret; } } // namespace filesystem } // namespace common_installer diff --git a/src/common/step/step_unregister_app.cc b/src/common/step/step_unregister_app.cc index 1e9a031..9743aaa 100755 --- a/src/common/step/step_unregister_app.cc +++ b/src/common/step/step_unregister_app.cc @@ -97,8 +97,7 @@ Step::Status StepUnregisterApplication::undo() { context_->certificate_info.get(), context_->uid.get())) { LOG(ERROR) << "Failed to restore the app registration in pkgmgr"; - // Continue to revert... - return Step::Status::OK; + return Step::Status::ERROR; } LOG(INFO) << "Successfully restored the app registration in pkgmgr"; diff --git a/src/tpk/step/step_copy_manifest_xml.cc b/src/tpk/step/step_copy_manifest_xml.cc index c11e0fc..bb71e26 100644 --- a/src/tpk/step/step_copy_manifest_xml.cc +++ b/src/tpk/step/step_copy_manifest_xml.cc @@ -178,7 +178,10 @@ Status StepCopyManifestXml::undo() { // Revert old xml file if exists bf::path old_dest_xml_path = _getOldDestXmlPath(*dest_xml_path_); if (bf::exists(old_dest_xml_path)) { - common_installer::MoveFile(old_dest_xml_path, *dest_xml_path_); + if (!common_installer::MoveFile(old_dest_xml_path, *dest_xml_path_)) { + LOG(ERROR) << "Cannot revert old xml file: " << old_dest_xml_path; + return Status::ERROR; + } } return Status::OK; diff --git a/src/tpk/step/step_create_symbolic_link.cc b/src/tpk/step/step_create_symbolic_link.cc index 72b7855..921ece1 100644 --- a/src/tpk/step/step_create_symbolic_link.cc +++ b/src/tpk/step/step_create_symbolic_link.cc @@ -109,10 +109,18 @@ Status StepCreateSymbolicLink::undo() { manifest_x* m = context_->manifest_data.get(); uiapplication_x *uiapp = m->uiapplication; serviceapplication_x *svcapp = m->serviceapplication; - if (!RemoveSymLink(uiapp, context_)) return Status::ERROR; - if (!RemoveSymLink(svcapp, context_)) return Status::ERROR; - return Status::OK; + Step::Status ret = Status::OK; + if (!RemoveSymLink(uiapp, context_)) { + LOG(ERROR) << "Cannot remove Symboliclink for uiapp"; + ret = Status::ERROR; + } + if (!RemoveSymLink(svcapp, context_)) { + LOG(ERROR) << "Cannot remove Symboliclink for svcapp"; + ret = Status::ERROR; + } + + return ret; } } // namespace filesystem -- 2.7.4