From: Junghyun Yeon Date: Thu, 1 Jul 2021 07:24:29 +0000 (+0900) Subject: Minor refactor pkg_upgrade tool X-Git-Tag: submit/tizen/20210811.093352~12 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F94%2F260694%2F2;p=platform%2Fcore%2Fappfw%2Fpkgmgr-tool.git Minor refactor pkg_upgrade tool - Refactor CompareVersion() to return verson compare result directly. - Change enum type name to use more common abbreviation of "compare". - Fix using UnzipPkgFromZip() to deliver std::string. - Remove unnecessary variable initialization. - Change return type of UnzipPkgFromZip to boolean. - Replace some command and its arguments into constexpr. - Extract BackendInvoker constructor with several functions for readability. - Adjust ordering of include files with alphabetical order. Change-Id: I4c6f7a8617da1e3fbbf6eca53fe308a8f0b61d8d Signed-off-by: Junghyun Yeon --- diff --git a/src/pkg_upgrade/include/backend_invoker.hh b/src/pkg_upgrade/include/backend_invoker.hh index 0f30fd2..4343dca 100644 --- a/src/pkg_upgrade/include/backend_invoker.hh +++ b/src/pkg_upgrade/include/backend_invoker.hh @@ -38,6 +38,11 @@ class BackendInvoker { private: std::list parameters_; + + void SetBackend(PkgType type); + void SetOperation(PkgOperation op, const std::string& pkgid); + void SetPreload(PkgLocation loc); + void SetRemovable(bool removable); }; } // common_fota diff --git a/src/pkg_upgrade/include/common_type.hh b/src/pkg_upgrade/include/common_type.hh index abdf6dc..a003d03 100644 --- a/src/pkg_upgrade/include/common_type.hh +++ b/src/pkg_upgrade/include/common_type.hh @@ -42,7 +42,7 @@ enum class PkgOperation { COMPLEX }; -enum class PkgVersionCompResult { +enum class PkgVersionCmpResult { LOWER, SAME, HIGHER, diff --git a/src/pkg_upgrade/include/pkg_upgrader.hh b/src/pkg_upgrade/include/pkg_upgrader.hh index fc7c096..fe96679 100644 --- a/src/pkg_upgrade/include/pkg_upgrader.hh +++ b/src/pkg_upgrade/include/pkg_upgrader.hh @@ -36,8 +36,7 @@ class PkgUpgrader { std::string GetId() const; std::string GetVersion() const; const BackendInvoker& GetBackendInvoker() const; - bool CompareVersion(const PkgUpgrader& pkg, - PkgVersionCompResult* result) const; + PkgVersionCmpResult CompareVersion(const PkgUpgrader& pkg) const; virtual bool Upgrade() = 0; diff --git a/src/pkg_upgrade/include/rw_upgrader.hh b/src/pkg_upgrade/include/rw_upgrader.hh index 8bb4dcf..a11a77a 100644 --- a/src/pkg_upgrade/include/rw_upgrader.hh +++ b/src/pkg_upgrade/include/rw_upgrader.hh @@ -32,7 +32,7 @@ class RwUpgrader : public PkgUpgrader { int UnzipFiles(const std::string& dest_path); int UnzipXml(const std::string& pkgid); int UnzipData(const std::string& pkgid, const std::string& dest); - int UnzipPkgFromZip(const std::string& pkgid); + bool UnzipPkgFromZip(const std::string& pkgid); private: std::unique_ptr old_pkg_; diff --git a/src/pkg_upgrade/src/backend_invoker.cc b/src/pkg_upgrade/src/backend_invoker.cc index 5326855..c2f5823 100644 --- a/src/pkg_upgrade/src/backend_invoker.cc +++ b/src/pkg_upgrade/src/backend_invoker.cc @@ -14,58 +14,61 @@ * limitations under the License. */ -#include -#include -#include +#include "backend_invoker.hh" + #include +#include + +#include #include #include +#include -#include "backend_invoker.hh" #include "logging.hh" namespace { -constexpr char kManifestDirectInstallCmdStr[] = "-y"; -constexpr char kUninstallCmdStr[] = "-d"; +constexpr char kTpkBackendCmdStr[] = "/usr/bin/tpk-backend"; +constexpr char kWgtBackendCmdStr[] = "/usr/bin/wgt-backend"; + +constexpr char kManifestDirectInstallOptStr[] = "-y"; +constexpr char kUninstallOptStr[] = "-d"; + +constexpr char kPreloadOptStr[] = "--preload"; +constexpr char kPreloadRWOptStr[] = "--preload-rw"; +constexpr char kPartialRWOptStr[] = "--partial-rw"; + +constexpr char kForceRemoveOptStr[] = "--force-remove"; +constexpr char kNoRemoveOptStr[] = "--no-remove"; + +constexpr char kKeepRWDataOptStr[] = "--keep-rwdata"; +constexpr char kSkipCheckReferenceOptStr[] = "--skip-check-reference"; } // namespace namespace common_fota { -BackendInvoker::BackendInvoker(std::string pkgid, PkgType type, PkgLocation loc, - PkgOperation op, bool removable) { - if (type == PkgType::WGT) - parameters_.push_back("/usr/bin/wgt-backend"); - else - parameters_.push_back("/usr/bin/tpk-backend"); +BackendInvoker::BackendInvoker(std::string pkgid, PkgType type, + PkgLocation loc, PkgOperation op, bool removable) { + SetBackend(type); + SetPreload(loc); + SetOperation(op, pkgid); if (op == PkgOperation::INSTALL || op == PkgOperation::UPDATE) { - parameters_.push_back(kManifestDirectInstallCmdStr); - parameters_.push_back(std::move(pkgid)); if (loc == PkgLocation::RO) { - parameters_.push_back("--preload"); - parameters_.push_back("--partial-rw"); + parameters_.push_back(kPartialRWOptStr); } else { - if (!removable) - parameters_.push_back("--no-remove"); - parameters_.push_back("--preload-rw"); + SetRemovable(removable); + + parameters_.push_back(kPreloadRWOptStr); } - parameters_.push_back("--skip-check-reference"); + parameters_.push_back(kSkipCheckReferenceOptStr); } else if (op == PkgOperation::UNINSTALL) { - parameters_.push_back(kUninstallCmdStr); - parameters_.push_back(std::move(pkgid)); - if (loc == PkgLocation::RO) - parameters_.push_back("--preload"); - parameters_.push_back("--force-remove"); - parameters_.push_back("--partial-rw"); + parameters_.push_back(kForceRemoveOptStr); + parameters_.push_back(kPartialRWOptStr); } else if (op == PkgOperation::UNINSTALL_KEEP_RW_DATA) { - parameters_.push_back(kUninstallCmdStr); - parameters_.push_back(std::move(pkgid)); - if (loc == PkgLocation::RO) - parameters_.push_back("--preload"); - parameters_.push_back("--force-remove"); - parameters_.push_back("--keep-rwdata"); + parameters_.push_back(kForceRemoveOptStr); + parameters_.push_back(kKeepRWDataOptStr); } } @@ -73,6 +76,35 @@ std::list BackendInvoker::GetParameter() { return parameters_; } +void BackendInvoker::SetBackend(PkgType type) { + if (type == PkgType::WGT) + parameters_.push_back(kWgtBackendCmdStr); + else + parameters_.push_back(kTpkBackendCmdStr); +} + +void BackendInvoker::SetOperation( + PkgOperation op, const std::string& pkgid) { + if (op == PkgOperation::INSTALL || op == PkgOperation::UPDATE) + parameters_.push_back(kManifestDirectInstallOptStr); + else if (op == PkgOperation::UNINSTALL || + op == PkgOperation::UNINSTALL_KEEP_RW_DATA) + parameters_.push_back(kUninstallOptStr); + + parameters_.push_back(std::move(pkgid)); +} + +void BackendInvoker::SetPreload(PkgLocation loc) { + if (loc == PkgLocation::RO) + parameters_.push_back(kPreloadOptStr); +} + +void BackendInvoker::SetRemovable(bool removable) { + if (!removable) + parameters_.push_back(kNoRemoveOptStr); +} + + int BackendInvoker::Run() const { const char* param[parameters_.size() + 1] = { nullptr, }; diff --git a/src/pkg_upgrade/src/file_logbackend.cc b/src/pkg_upgrade/src/file_logbackend.cc index 3303a67..6e36fe3 100644 --- a/src/pkg_upgrade/src/file_logbackend.cc +++ b/src/pkg_upgrade/src/file_logbackend.cc @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "file_logbackend.hh" + #include #include #include @@ -25,7 +27,6 @@ #include #include "logging.hh" -#include "file_logbackend.hh" namespace utils { diff --git a/src/pkg_upgrade/src/pkg_finder.cc b/src/pkg_upgrade/src/pkg_finder.cc index f539faa..8763431 100644 --- a/src/pkg_upgrade/src/pkg_finder.cc +++ b/src/pkg_upgrade/src/pkg_finder.cc @@ -16,14 +16,15 @@ #include "pkg_finder.hh" +#include #include #include -#include + #include -#include #include #include +#include #include "logging.hh" #include "backend_invoker.hh" diff --git a/src/pkg_upgrade/src/pkg_upgrader.cc b/src/pkg_upgrade/src/pkg_upgrader.cc index e683c8d..4043183 100644 --- a/src/pkg_upgrade/src/pkg_upgrader.cc +++ b/src/pkg_upgrade/src/pkg_upgrader.cc @@ -13,7 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + #include + #include #include "pkg_upgrader.hh" @@ -57,22 +59,19 @@ const BackendInvoker& PkgUpgrader::GetBackendInvoker() const { return backend_; } -bool PkgUpgrader::CompareVersion(const PkgUpgrader& pkg, - PkgVersionCompResult* result) const { +PkgVersionCmpResult PkgUpgrader::CompareVersion( + const PkgUpgrader& pkg) const { pkgmgrinfo_version_compare_type compare = PMINFO_VERSION_OLD; - int ret = pkgmgrinfo_compare_package_version(version_.c_str(), - pkg.GetVersion().c_str(), &compare); - if (ret != 0) - return false; + if (pkgmgrinfo_compare_package_version(version_.c_str(), + pkg.GetVersion().c_str(), &compare) != 0) + return PkgVersionCmpResult::UNKNOWN; if (compare == PMINFO_VERSION_NEW) - *result = PkgVersionCompResult::HIGHER; + return PkgVersionCmpResult::HIGHER; else if (compare == PMINFO_VERSION_OLD) - *result = PkgVersionCompResult::LOWER; + return PkgVersionCmpResult::LOWER; else - *result = PkgVersionCompResult::SAME; - - return true; + return PkgVersionCmpResult::SAME; } diff --git a/src/pkg_upgrade/src/pkg_upgrader_factory.cc b/src/pkg_upgrade/src/pkg_upgrader_factory.cc index b27c438..f0dffbb 100644 --- a/src/pkg_upgrade/src/pkg_upgrader_factory.cc +++ b/src/pkg_upgrade/src/pkg_upgrader_factory.cc @@ -14,13 +14,14 @@ * limitations under the License. */ +#include "pkg_upgrader_factory.hh" + #include #include "logging.hh" -#include "pkg_upgrader_factory.hh" #include "ro2rw_upgrader.hh" -#include "rw2ro_upgrader.hh" #include "ro_upgrader.hh" +#include "rw2ro_upgrader.hh" #include "rw_upgrader.hh" #include "simple_upgrader.hh" diff --git a/src/pkg_upgrade/src/ro_upgrader.cc b/src/pkg_upgrade/src/ro_upgrader.cc index daaae49..846724a 100644 --- a/src/pkg_upgrade/src/ro_upgrader.cc +++ b/src/pkg_upgrade/src/ro_upgrader.cc @@ -29,14 +29,14 @@ RoUpgrader::RoUpgrader(std::unique_ptr old_pkg, bool RoUpgrader::Upgrade() { if (old_pkg_) { - PkgVersionCompResult result = PkgVersionCompResult::UNKNOWN; - if (!new_pkg_->CompareVersion(*old_pkg_, &result)) { - LOG(ERROR) << "Failed to compare version of package(" << new_pkg_->GetId() - << ")"; + PkgVersionCmpResult result = new_pkg_->CompareVersion(*old_pkg_); + if (result == PkgVersionCmpResult::UNKNOWN) { + LOG(ERROR) << "Failed to compare version of package(" + << new_pkg_->GetId() << ")"; return false; } - if (result == PkgVersionCompResult::SAME) { + if (result == PkgVersionCmpResult::SAME) { LOG(INFO) << new_pkg_->GetId() << " has same version(" << new_pkg_->GetVersion() << "). Nothing to do."; return true; diff --git a/src/pkg_upgrade/src/rw2ro_upgrader.cc b/src/pkg_upgrade/src/rw2ro_upgrader.cc index 97fc084..3cd9911 100644 --- a/src/pkg_upgrade/src/rw2ro_upgrader.cc +++ b/src/pkg_upgrade/src/rw2ro_upgrader.cc @@ -30,14 +30,14 @@ Rw2RoUpgrader::Rw2RoUpgrader(std::unique_ptr old_pkg, bool Rw2RoUpgrader::Upgrade() { // Update only if New RO package has higher(or same) version. - PkgVersionCompResult result = PkgVersionCompResult::UNKNOWN; - if (!new_pkg_->CompareVersion(*old_pkg_, &result)) { + PkgVersionCmpResult result = new_pkg_->CompareVersion(*old_pkg_); + if (result == PkgVersionCmpResult::UNKNOWN) { LOG(ERROR) << "Failed to compare version of package(" << new_pkg_->GetId() << ")"; return false; } - if (result == PkgVersionCompResult::LOWER) { + if (result == PkgVersionCmpResult::LOWER) { LOG(INFO) << old_pkg_->GetId() << " has higher version(" << old_pkg_->GetVersion() << ") already. Nothing to do."; return true; diff --git a/src/pkg_upgrade/src/rw_upgrader.cc b/src/pkg_upgrade/src/rw_upgrader.cc index e5286bc..666cabe 100644 --- a/src/pkg_upgrade/src/rw_upgrader.cc +++ b/src/pkg_upgrade/src/rw_upgrader.cc @@ -35,24 +35,22 @@ RwUpgrader::RwUpgrader(std::unique_ptr old_pkg, bool RwUpgrader::Upgrade() { if (old_pkg_) { - PkgVersionCompResult result = PkgVersionCompResult::UNKNOWN; - if (!new_pkg_->CompareVersion(*old_pkg_, &result)) { - LOG(ERROR) << "Failed to compare version of package(" << new_pkg_->GetId() - << ")"; + PkgVersionCmpResult result = new_pkg_->CompareVersion(*old_pkg_); + if (result == PkgVersionCmpResult::UNKNOWN) { + LOG(ERROR) << "Failed to compare version of package(" + << new_pkg_->GetId() << ")"; return false; } - if (result == PkgVersionCompResult::SAME) { + if (result == PkgVersionCmpResult::SAME) { LOG(INFO) << new_pkg_->GetId() << " has same version(" << new_pkg_->GetVersion() << "). Nothing to do."; return true; } } - if (UnzipPkgFromZip(new_pkg_->GetId().c_str()) != 0) { - LOG(ERROR) << "UnzipPkgFromZip(" << new_pkg_->GetId() << ") failed"; + if (!UnzipPkgFromZip(new_pkg_->GetId())) return false; - } if (!new_pkg_->Upgrade()) { LOG(ERROR) << "Upgrade operation failed"; @@ -65,42 +63,40 @@ bool RwUpgrader::Upgrade() { int RwUpgrader::UnzipFiles(const std::string& dest_path) { const char* unzip_argv[] = { "/usr/bin/unzip", "-oXqq", kOptZipFile, dest_path.c_str(), "-d", "/", nullptr }; + return BackendInvoker::XSystem(unzip_argv); } int RwUpgrader::UnzipXml(const std::string& pkgid) { std::string path = "opt/share/packages/" + pkgid + ".xml"; + return UnzipFiles(path); } int RwUpgrader::UnzipData(const std::string& pkgid, const std::string& dest) { std::string path = dest + pkgid + "/*"; + return UnzipFiles(path); } -int RwUpgrader::UnzipPkgFromZip(const std::string& pkgid) { - int ret = -1; - - ret = UnzipXml(pkgid); - if (ret != 0) { +bool RwUpgrader::UnzipPkgFromZip(const std::string& pkgid) { + if (UnzipXml(pkgid) != 0) { LOG(ERROR) << "UnzipXml(" << pkgid << ") failed"; - return ret; + return false; } - ret = UnzipData(pkgid, "opt/usr/globalapps/"); - if (ret != 0) { - LOG(ERROR) << "UnzipData(" << pkgid << ") failed"; - return ret; + if (UnzipData(pkgid, "opt/usr/globalapps/") != 0) { + LOG(ERROR) << "Failed to unzip userdata for pkg(" << pkgid << ")"; + return false; } - ret = UnzipData(pkgid, "opt/etc/skel/apps_rw/"); - if (ret != 0) { - LOG(ERROR) << "UnzipData(" << pkgid << ") failed"; - return ret; + if (UnzipData(pkgid, "opt/etc/skel/apps_rw/") != 0) { + LOG(ERROR) << "Failed to unzip skel for pkg(" << pkgid << ")"; + return false; } - return 0; + return true; } } // namespace common_fota diff --git a/src/pkg_upgrade/src/upgrader.cc b/src/pkg_upgrade/src/upgrader.cc index c085bfe..63e9fca 100644 --- a/src/pkg_upgrade/src/upgrader.cc +++ b/src/pkg_upgrade/src/upgrader.cc @@ -14,26 +14,28 @@ * limitations under the License. */ +#include "upgrader.hh" + +#include +#include +#include #include +#include #include -#include #include #include -#include -#include +#include #include #include -#include -#include +#include #include #include -#include -#include +#include + #include #include "logging.hh" #include "pkg_upgrader_factory.hh" -#include "upgrader.hh" using std::list; using std::shared_ptr;