From fc21ece7841ed87ddb1a58215bdb8c02a78b2143 Mon Sep 17 00:00:00 2001 From: Megamouse Date: Thu, 2 Mar 2023 17:02:18 +0100 Subject: [PATCH] pkg install: return correct error if possible. Add more early outs and skip workers on error, --- rpcs3/Crypto/unpkg.cpp | 66 +++++++++++++++++++++-------------- rpcs3/Crypto/unpkg.h | 4 +-- rpcs3/rpcs3qt/main_window.cpp | 12 +++---- 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/rpcs3/Crypto/unpkg.cpp b/rpcs3/Crypto/unpkg.cpp index 4d7105266f..98a3cc66fd 100644 --- a/rpcs3/Crypto/unpkg.cpp +++ b/rpcs3/Crypto/unpkg.cpp @@ -1032,8 +1032,8 @@ package_error package_reader::extract_data(std::deque& readers, if (!reader.set_install_path()) { error = package_error::other; - reader.m_result = result::error; - break; + reader.m_result = result::error; // We don't know if it's dirty yet. + return error; } } @@ -1042,41 +1042,52 @@ package_error package_reader::extract_data(std::deque& readers, // Use a seperate map for each reader. We need to check if the target app version exists for each package in sequence. std::map all_install_entries; - if (error == package_error::no_error) + if (error != package_error::no_error || num_failures > 0) { - // Check if this package is allowed to be installed on top of the existing data - error = reader.check_target_app_version(); + ensure(reader.m_result == result::error || reader.m_result == result::error_dirty); + return error; } - if (error == package_error::no_error) - { - reader.m_result = result::started; + // Check if this package is allowed to be installed on top of the existing data + error = reader.check_target_app_version(); - // Parse the files to be installed and create all paths. - if (!reader.fill_data(all_install_entries)) - { - error = package_error::other; - } + if (error != package_error::no_error) + { + reader.m_result = result::error; // We don't know if it's dirty yet. + return error; + } + + reader.m_result = result::started; + + // Parse the files to be installed and create all paths. + if (!reader.fill_data(all_install_entries)) + { + error = package_error::other; + // Do not return yet. We may need to clean up down below. } - reader.m_bufs.resize(std::min(utils::get_thread_count(), reader.m_install_entries.size())); reader.m_num_failures = error == package_error::no_error ? 0 : 1; - atomic_t thread_indexer = 0; - - named_thread_group workers("PKG Installer "sv, std::max(::narrow(reader.m_bufs.size()), 1) - 1, [&]() + if (reader.m_num_failures == 0) { - reader.extract_worker(thread_key{thread_indexer++}); - }); + reader.m_bufs.resize(std::min(utils::get_thread_count(), reader.m_install_entries.size())); - reader.extract_worker(thread_key{thread_indexer++}); - workers.join(); + atomic_t thread_indexer = 0; + + named_thread_group workers("PKG Installer "sv, std::max(::narrow(reader.m_bufs.size()), 1) - 1, [&]() + { + reader.extract_worker(thread_key{thread_indexer++}); + }); + + reader.extract_worker(thread_key{thread_indexer++}); + workers.join(); + + reader.m_bufs.clear(); + reader.m_bufs.shrink_to_fit(); + } num_failures += reader.m_num_failures; - reader.m_bufs.clear(); - reader.m_bufs.shrink_to_fit(); - // We don't count this package as aborted if all entries were processed. if (reader.m_num_failures || (reader.m_aborted && reader.m_entry_indexer < reader.m_install_entries.size())) { @@ -1099,13 +1110,14 @@ package_error package_reader::extract_data(std::deque& readers, if (reader.m_num_failures) { pkg_log.error("Package failed to install ('%s')", reader.m_install_path); - reader.m_result = cleaned ? result::error_cleaned : result::error; + reader.m_result = cleaned ? result::error : result::error_dirty; } else { pkg_log.warning("Package installation aborted ('%s')", reader.m_install_path); - reader.m_result = cleaned ? result::aborted_cleaned : result::aborted; + reader.m_result = cleaned ? result::aborted : result::aborted_dirty; } + break; } @@ -1121,7 +1133,7 @@ package_error package_reader::extract_data(std::deque& readers, bootable_paths.emplace_back(std::move(reader.m_bootable_file_path)); } - if (num_failures > 0) + if (error == package_error::no_error && num_failures > 0) { error = package_error::other; } diff --git a/rpcs3/Crypto/unpkg.h b/rpcs3/Crypto/unpkg.h index 03a1b13008..2db1345d37 100644 --- a/rpcs3/Crypto/unpkg.h +++ b/rpcs3/Crypto/unpkg.h @@ -338,9 +338,9 @@ public: started, success, aborted, - aborted_cleaned, + aborted_dirty, error, - error_cleaned + error_dirty }; bool is_valid() const { return m_is_valid; } diff --git a/rpcs3/rpcs3qt/main_window.cpp b/rpcs3/rpcs3qt/main_window.cpp index 7282a92d08..4819214317 100644 --- a/rpcs3/rpcs3qt/main_window.cpp +++ b/rpcs3/rpcs3qt/main_window.cpp @@ -1015,18 +1015,18 @@ void main_window::HandlePackageInstallation(QStringList file_paths) } case package_reader::result::not_started: case package_reader::result::started: - case package_reader::result::aborted_cleaned: + case package_reader::result::aborted: { gui_log.notice("Aborted installation of %s (title_id=%s, title=%s, version=%s).", sstr(package.path), sstr(package.title_id), sstr(package.title), sstr(package.version)); break; } - case package_reader::result::error_cleaned: + case package_reader::result::error: { gui_log.error("Failed to install %s (title_id=%s, title=%s, version=%s).", sstr(package.path), sstr(package.title_id), sstr(package.title), sstr(package.version)); break; } - case package_reader::result::aborted: - case package_reader::result::error: + case package_reader::result::aborted_dirty: + case package_reader::result::error_dirty: { gui_log.error("Partially installed %s (title_id=%s, title=%s, version=%s).", sstr(package.path), sstr(package.title_id), sstr(package.title), sstr(package.version)); break; @@ -1142,10 +1142,10 @@ void main_window::HandlePackageInstallation(QStringList file_paths) case package_reader::result::not_started: case package_reader::result::started: case package_reader::result::aborted: - case package_reader::result::aborted_cleaned: + case package_reader::result::aborted_dirty: break; case package_reader::result::error: - case package_reader::result::error_cleaned: + case package_reader::result::error_dirty: package = &packages[i]; break; }