diff --git a/include/mamba/thread_utils.hpp b/include/mamba/thread_utils.hpp index 3dc8d1b22..480aa8c2b 100644 --- a/include/mamba/thread_utils.hpp +++ b/include/mamba/thread_utils.hpp @@ -50,7 +50,7 @@ namespace mamba // Must be called by the cleaning thread to ensure // it won't free ressources that could be required // by threads still active. - void wait_before_cleaning(); + void wait_for_all_threads(); void notify_cleanup(); // Should be called by the main thread to ensure @@ -145,7 +145,7 @@ namespace mamba m_cleanup_function = std::bind(std::forward(func), std::forward(args)...); std::signal(SIGINT, [](int) { set_sig_interrupted(); - wait_before_cleaning(); + wait_for_all_threads(); m_cleanup_function(); }); } @@ -159,11 +159,15 @@ namespace mamba block_signals(); auto f = std::bind(std::forward(func), std::forward(args)...); std::thread victor_the_cleaner([f, this]() { + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, nullptr); + pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, nullptr); + wait_for_signal(); + if (m_interrupt.load()) { set_sig_interrupted(); - wait_before_cleaning(); + wait_for_all_threads(); f(); notify_cleanup(); } diff --git a/include/mamba/transaction.hpp b/include/mamba/transaction.hpp index 7fc05b091..3c12f1764 100644 --- a/include/mamba/transaction.hpp +++ b/include/mamba/transaction.hpp @@ -50,8 +50,23 @@ namespace mamba bool finalize_callback(); bool finished(); bool validate_extract(); + auto validation_result() const; + void clear_cache() const; + DownloadTarget* target(const fs::path& cache_path, MultiPackageCache& cache); + enum VALIDATION_RESULT + { + UNDEFINED = 0, + VALID = 1, + SHA256_ERROR, + MD5SUM_ERROR, + SIZE_ERROR, + EXTRACT_ERROR + }; + + std::exception m_decompress_exception; + private: bool m_finished; PackageInfo m_package_info; @@ -67,6 +82,7 @@ namespace mamba std::future m_extract_future; + VALIDATION_RESULT m_validation_result = VALIDATION_RESULT::UNDEFINED; static std::mutex extract_mutex; }; diff --git a/src/link.cpp b/src/link.cpp index 1fc5f1897..4159d58a2 100644 --- a/src/link.cpp +++ b/src/link.cpp @@ -463,6 +463,43 @@ namespace mamba return tf; } + auto prepare_wrapped_call(const fs::path& prefix, const std::vector& cmd) + { + std::vector command_args; + std::unique_ptr script_file; + + if (on_win) + { + ensure_comspec_set(); + std::string comspec = env::get("COMSPEC"); + if (comspec.size() == 0) + { + throw std::runtime_error( + concat("Failed to run script: COMSPEC not set in env vars.")); + } + + script_file = wrap_call( + Context::instance().root_prefix, prefix, Context::instance().dev, false, cmd); + + command_args = { comspec, "/d", "/c", script_file->path() }; + } + else + { + // shell_path = 'sh' if 'bsd' in sys.platform else 'bash' + fs::path shell_path = env::which("bash"); + if (shell_path.empty()) + { + shell_path = env::which("sh"); + } + + script_file = wrap_call( + Context::instance().root_prefix, prefix, Context::instance().dev, false, cmd); + command_args.push_back(shell_path); + command_args.push_back(script_file->path()); + } + return std::make_tuple(command_args, std::move(script_file)); + } + /* call the post-link or pre-unlink script and return true / false on success / failure @@ -473,13 +510,17 @@ namespace mamba const std::string& env_prefix = "", bool activate = false) { -#ifdef _WIN32 - auto path = prefix / get_bin_directory_short_path() - / concat(".", pkg_info.name, "-", action, ".bat"); -#else - auto path = prefix / get_bin_directory_short_path() - / concat(".", pkg_info.name, "-", action, ".sh"); -#endif + fs::path path; + if (on_win) + { + path = prefix / get_bin_directory_short_path() + / concat(".", pkg_info.name, "-", action, ".bat"); + } + else + { + path = prefix / get_bin_directory_short_path() + / concat(".", pkg_info.name, "-", action, ".sh"); + } if (!fs::exists(path)) { @@ -500,56 +541,60 @@ namespace mamba std::vector command_args; std::unique_ptr script_file; -#ifdef _WIN32 - ensure_comspec_set(); - std::string comspec = env::get("COMSPEC"); - if (comspec.size() == 0) + if (on_win) { - LOG_ERROR << "Failed to run " << action << " for " << pkg_info.name - << " due to COMSPEC not set in env vars."; - return false; + ensure_comspec_set(); + std::string comspec = env::get("COMSPEC"); + if (comspec.size() == 0) + { + LOG_ERROR << "Failed to run " << action << " for " << pkg_info.name + << " due to COMSPEC not set in env vars."; + return false; + } + + if (activate) + { + script_file = wrap_call(Context::instance().root_prefix, + prefix, + Context::instance().dev, + false, + { "@CALL", path }); + + command_args = { comspec, "/d", "/c", script_file->path() }; + } + else + { + command_args = { comspec, "/d", "/c", path }; + } } - if (activate) - { - script_file = wrap_call(Context::instance().root_prefix, - prefix, - Context::instance().dev, - false, - { "@CALL", path }); - - command_args = { comspec, "/d", "/c", script_file->path() }; - } else { - command_args = { comspec, "/d", "/c", path }; - } -#else - // shell_path = 'sh' if 'bsd' in sys.platform else 'bash' - fs::path shell_path = env::which("bash"); - if (shell_path.empty()) - { - shell_path = env::which("sh"); - } + // shell_path = 'sh' if 'bsd' in sys.platform else 'bash' + fs::path shell_path = env::which("bash"); + if (shell_path.empty()) + { + shell_path = env::which("sh"); + } - if (activate) - { - // std::string caller - script_file = wrap_call(Context::instance().root_prefix, - prefix, - Context::instance().dev, - false, - { ".", path }); - command_args.push_back(shell_path); - command_args.push_back(script_file->path()); + if (activate) + { + // std::string caller + script_file = wrap_call(Context::instance().root_prefix, + prefix, + Context::instance().dev, + false, + { ".", path }); + command_args.push_back(shell_path); + command_args.push_back(script_file->path()); + } + else + { + command_args.push_back(shell_path); + command_args.push_back("-x"); + command_args.push_back(path); + } } - else - { - command_args.push_back(shell_path); - command_args.push_back("-x"); - command_args.push_back(path); - } -#endif envmap["ROOT_PREFIX"] = Context::instance().root_prefix; envmap["PREFIX"] = env_prefix.size() ? env_prefix : std::string(prefix); @@ -875,9 +920,6 @@ namespace mamba } all_py_files_f.close(); - // TODO use the real python file here?! - LOG_DEBUG << "Running " << m_context->target_prefix / m_context->python_path - << " -Wi -m compileall -q -l -i " << all_py_files.path() << std::endl; std::vector command = { m_context->target_prefix / m_context->python_path, "-Wi", "-m", @@ -901,18 +943,10 @@ namespace mamba std::string cwd = m_context->target_prefix; options.working_directory = cwd.c_str(); - std::map envmap = env::copy(); -#ifdef _WIN32 - CmdExeActivator activator; -#else - PosixActivator activator; -#endif - std::string current_env_path = activator.add_prefix_to_path(m_context->target_prefix, 0); - envmap["PATH"] = current_env_path; - options.env.behavior = reproc::env::empty; - options.env.extra = envmap; - - auto [_, ec] = reproc::run(command, options); + auto [wrapped_command, script_file] + = prepare_wrapped_call(m_context->target_prefix, command); + LOG_INFO << "Running wrapped python compilation command " << join(" ", command); + auto [_, ec] = reproc::run(wrapped_command, options); if (ec) { @@ -934,11 +968,11 @@ namespace mamba { LOG_INFO << "Executing install for " << m_source; nlohmann::json index_json, out_json; - LOG_WARNING << "Opening: " << m_source / "info" / "paths.json"; + LOG_INFO << "Opening: " << m_source / "info" / "paths.json"; auto paths_data = read_paths(m_source); - LOG_WARNING << "Opening: " << m_source / "info" / "repodata_record.json"; + LOG_INFO << "Opening: " << m_source / "info" / "repodata_record.json"; std::ifstream repodata_f(m_source / "info" / "repodata_record.json"); repodata_f >> index_json; diff --git a/src/micromamba/main.cpp b/src/micromamba/main.cpp index 9a1d606b6..c03f70ba0 100644 --- a/src/micromamba/main.cpp +++ b/src/micromamba/main.cpp @@ -792,7 +792,6 @@ download_explicit(const std::vector& pkgs) std::vector> targets; MultiDownloadTarget multi_dl; MultiPackageCache pkg_cache({ Context::instance().root_prefix / "pkgs" }); - Console::instance().init_multi_progress(); for (auto& pkg : pkgs) { @@ -1240,9 +1239,11 @@ main(int argc, char** argv) { CLI11_PARSE(app, argc, argv); } - catch (std::exception& e) + catch (const std::exception& e) { LOG_ERROR << e.what(); + std::raise(SIGTERM); + std::cout << "Exiting." << std::endl; exit(1); } diff --git a/src/package_handling.cpp b/src/package_handling.cpp index 9e4e19d64..9090bebfd 100644 --- a/src/package_handling.cpp +++ b/src/package_handling.cpp @@ -371,22 +371,24 @@ namespace mamba fs::path extract(const fs::path& file) { std::string dest_dir = file; + TemporaryDirectory temp_extract_dir; + if (ends_with(dest_dir, ".tar.bz2")) { dest_dir = dest_dir.substr(0, dest_dir.size() - 8); - extract_archive(file, dest_dir); - return dest_dir; + extract_archive(file, temp_extract_dir.path()); } else if (ends_with(dest_dir, ".conda")) { dest_dir = dest_dir.substr(0, dest_dir.size() - 6); - extract_conda(file, dest_dir); - return dest_dir; + extract_conda(file, temp_extract_dir.path()); } else { throw std::runtime_error("Unknown package format (" + file.string() + ")"); } + fs::rename(temp_extract_dir.path(), dest_dir); + return dest_dir; } bool transmute(const fs::path& pkg_file, const fs::path& target, int compression_level) diff --git a/src/thread_utils.cpp b/src/thread_utils.cpp index 7fbefa013..b5cdc4328 100644 --- a/src/thread_utils.cpp +++ b/src/thread_utils.cpp @@ -78,7 +78,7 @@ namespace mamba return thread_count; } - void wait_before_cleaning() + void wait_for_all_threads() { std::unique_lock lk(clean_mutex); clean_var.wait(lk, []() { return thread_count == 0; }); @@ -98,11 +98,17 @@ namespace mamba namespace { - std::thread::native_handle_type cleanup_id; + std::thread::native_handle_type cleanup_id = 0; } void register_cleaning_thread_id(std::thread::native_handle_type id) { +#ifndef _WIN32 + if (cleanup_id) + { + pthread_cancel(cleanup_id); + } +#endif cleanup_id = id; } @@ -160,10 +166,13 @@ namespace mamba if (is_sig_interrupted()) { wait_for_cleanup(); + std::cout << "Cleanup done." << std::endl; } m_interrupt.store(false); pthread_sigmask(SIG_UNBLOCK, &sigset, nullptr); set_default_signal_handler(); + pthread_cancel(get_cleaning_thread_id()); + register_cleaning_thread_id(0); } void interruption_guard::block_signals() const diff --git a/src/transaction.cpp b/src/transaction.cpp index 3c49c3afb..64a3d6a9c 100644 --- a/src/transaction.cpp +++ b/src/transaction.cpp @@ -79,29 +79,39 @@ namespace mamba bool PackageDownloadExtractTarget::validate_extract() { // Validation + m_validation_result = VALIDATION_RESULT::VALID; if (m_expected_size && size_t(m_target->downloaded_size) != m_expected_size) { - LOG_ERROR << "File not valid: file size doesn't match expectation " << m_tarball_path; - throw std::runtime_error("File not valid: file size doesn't match expectation (" - + std::string(m_tarball_path) + ")"); + LOG_ERROR << "File not valid: file size doesn't match expectation " << m_tarball_path + << "\nExpected: " << m_expected_size << "\n"; + m_progress_proxy.mark_as_completed("File size validation error."); + m_validation_result = SIZE_ERROR; } interruption_point(); if (!m_sha256.empty() && !validate::sha256(m_tarball_path, m_sha256)) { - LOG_ERROR << "File not valid: SHA256 sum doesn't match expectation " << m_tarball_path; - throw std::runtime_error("File not valid: SHA256 sum doesn't match expectation (" - + std::string(m_tarball_path) + ")"); + m_validation_result = SHA256_ERROR; + m_progress_proxy.mark_as_completed("SHA256 sum validation error."); + LOG_ERROR << "File not valid: SHA256 sum doesn't match expectation " << m_tarball_path + << "\nExpected: " << m_sha256 << "\n"; } else { if (!m_md5.empty() && !validate::md5(m_tarball_path, m_md5)) { - LOG_ERROR << "File not valid: MD5 sum doesn't match expectation " << m_tarball_path; - throw std::runtime_error("File not valid: MD5 sum doesn't match expectation (" - + std::string(m_tarball_path) + ")"); + m_validation_result = MD5SUM_ERROR; + m_progress_proxy.mark_as_completed("MD5 sum validation error."); + LOG_ERROR << "File not valid: MD5 sum doesn't match expectation " << m_tarball_path + << "\nExpected: " << m_md5 << "\n"; } } + if (m_validation_result != VALIDATION_RESULT::VALID) + { + // abort here, but set finished to true + m_finished = true; + return true; + } interruption_point(); LOG_INFO << "Waiting for decompression " << m_tarball_path; @@ -112,10 +122,22 @@ namespace mamba interruption_point(); m_progress_proxy.set_postfix("Decompressing..."); LOG_INFO << "Decompressing " << m_tarball_path; - auto extract_path = extract(m_tarball_path); - LOG_INFO << "Extracted to " << extract_path; - write_repodata_record(extract_path); - add_url(); + try + { + auto extract_path = extract(m_tarball_path); + LOG_INFO << "Extracted to " << extract_path; + write_repodata_record(extract_path); + add_url(); + } + catch (std::exception& e) + { + LOG_ERROR << "Error when extracting package: " << e.what(); + m_decompress_exception = e; + m_validation_result = VALIDATION_RESULT::EXTRACT_ERROR; + m_finished = true; + m_progress_proxy.mark_as_completed("Extraction error"); + return m_finished; + } } interruption_point(); @@ -152,6 +174,21 @@ namespace mamba return m_target == nullptr ? true : m_finished; } + auto PackageDownloadExtractTarget::validation_result() const + { + return m_validation_result; + } + + void PackageDownloadExtractTarget::clear_cache() const + { + fs::remove_all(m_tarball_path); + fs::path dest_dir = strip_package_extension(m_tarball_path); + if (fs::exists(dest_dir)) + { + fs::remove_all(dest_dir); + } + } + // todo remove cache from this interface DownloadTarget* PackageDownloadExtractTarget::target(const fs::path& cache_path, MultiPackageCache& cache) @@ -166,6 +203,7 @@ namespace mamba if (valid && !dest_dir_exists) { // TODO add extract job here + // finalize_callback(); } // tarball can be removed, it's fine if only the correct dest dir exists @@ -663,6 +701,7 @@ namespace mamba interruption_guard g([]() { Console::instance().init_multi_progress(); }); bool downloaded = multi_dl.download(true); + bool all_valid = true; if (!downloaded) { @@ -688,7 +727,19 @@ namespace mamba std::this_thread::sleep_for(std::chrono::milliseconds(100)); } - return !is_sig_interrupted() && downloaded; + for (const auto& t : targets) + { + if (t->validation_result() != PackageDownloadExtractTarget::VALIDATION_RESULT::VALID + && t->validation_result() + != PackageDownloadExtractTarget::VALIDATION_RESULT::UNDEFINED) + { + t->clear_cache(); + all_valid = false; + throw std::runtime_error("Found incorrect download. Aborting"); + } + } + + return !is_sig_interrupted() && downloaded && all_valid; } bool MTransaction::empty()