diff --git a/libmamba/include/mamba/core/fetch.hpp b/libmamba/include/mamba/core/fetch.hpp index e618db7c82..0d4904fc52 100644 --- a/libmamba/include/mamba/core/fetch.hpp +++ b/libmamba/include/mamba/core/fetch.hpp @@ -56,18 +56,21 @@ namespace mamba void set_expected_size(std::size_t size); void set_head_only(bool yes); - const std::string& name() const; - const std::string& url() const; - std::size_t expected_size() const; + const std::string& get_name() const; + const std::string& get_url() const; - void init_curl_ssl(); - void init_curl_target(const std::string& url); + const std::string& get_etag() const; + const std::string& get_mod() const; + const std::string& get_cache_control() const; - bool resource_exists(); - bool perform(); - CURL* handle(); + std::size_t get_expected_size() const; + int get_http_status() const; + std::size_t get_downloaded_size() const; - curl_off_t get_speed(); + std::size_t get_speed(); + + void init_curl_ssl(); + void init_curl_target(const std::string& url); template inline void set_finalize_callback(bool (C::*cb)(const DownloadTarget&), C* data) @@ -80,12 +83,18 @@ namespace mamba m_ignore_failure = yes; } - bool ignore_failure() const + bool get_ignore_failure() const { return m_ignore_failure; } void set_result(CURLcode r); + std::size_t get_result() const; + + bool resource_exists(); + bool perform(); + CURL* handle(); + bool finalize(); std::string get_transfer_msg(); @@ -95,16 +104,6 @@ namespace mamba std::chrono::steady_clock::time_point progress_throttle_time() const; void set_progress_throttle_time(const std::chrono::steady_clock::time_point& time); - CURLcode result = CURLE_OK; - bool failed = false; - int http_status = 10000; - char* effective_url = nullptr; - curl_off_t downloaded_size = 0; - curl_off_t avg_speed = 0; - std::string final_url; - - std::string etag, mod, cache_control; - private: std::unique_ptr m_zstd_stream; @@ -114,16 +113,24 @@ namespace mamba std::string m_name, m_filename, m_url; + int m_http_status; + std::size_t m_downloaded_size; + char* m_effective_url; + + CURLcode m_result; // Enum range from 0 to 99 + + std::string m_etag, m_mod, m_cache_control; + // validation - std::size_t m_expected_size = 0; + std::size_t m_expected_size; // retry & backoff std::chrono::steady_clock::time_point m_next_retry; - std::size_t m_retry_wait_seconds = get_default_retry_timeout(); - std::size_t m_retries = 0; + std::size_t m_retry_wait_seconds; + std::size_t m_retries; - bool m_has_progress_bar = false; - bool m_ignore_failure = false; + bool m_has_progress_bar; + bool m_ignore_failure; ProgressProxy m_progress_bar; diff --git a/libmamba/src/api/install.cpp b/libmamba/src/api/install.cpp index 63399a7fd2..f6e54be336 100644 --- a/libmamba/src/api/install.cpp +++ b/libmamba/src/api/install.cpp @@ -662,7 +662,7 @@ namespace mamba tmp_lock_file = std::make_unique(); DownloadTarget dt("Environment Lockfile", lockfile, tmp_lock_file->path()); bool success = dt.perform(); - if (!success || dt.http_status != 200) + if (!success || dt.get_http_status() != 200) { throw std::runtime_error( fmt::format("Could not download environment lockfile from {}", lockfile) diff --git a/libmamba/src/core/fetch.cpp b/libmamba/src/core/fetch.cpp index e528a38acc..e1432ee843 100644 --- a/libmamba/src/core/fetch.cpp +++ b/libmamba/src/core/fetch.cpp @@ -30,6 +30,15 @@ namespace mamba : m_name(name) , m_filename(filename) , m_url(unc_url(url)) + , m_http_status(10000) + , m_downloaded_size(0) + , m_effective_url(nullptr) + , m_result(CURLE_OK) + , m_expected_size(0) + , m_retry_wait_seconds(get_default_retry_timeout()) + , m_retries(0) + , m_has_progress_bar(false) + , m_ignore_failure(false) { m_curl_handle = std::make_unique(); init_curl_ssl(); @@ -220,7 +229,8 @@ namespace mamba bool DownloadTarget::can_retry() { - switch (result) + // TODO add a function here to wrap the switch and returning a bool + switch (m_result) { case CURLE_ABORTED_BY_CALLBACK: case CURLE_BAD_FUNCTION_ARGUMENT: @@ -245,7 +255,7 @@ namespace mamba } return m_retries < size_t(Context::instance().remote_fetch_params.max_retries) - && (http_status == 413 || http_status == 429 || http_status >= 500) + && (m_http_status == 413 || m_http_status == 429 || m_http_status >= 500) && !starts_with(m_url, "file://"); } @@ -334,15 +344,15 @@ namespace mamba std::string lkey = to_lower(key); if (lkey == "etag") { - s->etag = value; + s->m_etag = value; } else if (lkey == "cache-control") { - s->cache_control = value; + s->m_cache_control = value; } else if (lkey == "last-modified") { - s->mod = value; + s->m_mod = value; } } return nitems * size; @@ -409,7 +419,7 @@ namespace mamba target->set_progress_throttle_time(now); } - if (!total_to_download && !target->expected_size()) + if (!total_to_download && !target->get_expected_size()) { target->m_progress_bar.activate_spinner(); } @@ -418,7 +428,7 @@ namespace mamba target->m_progress_bar.deactivate_spinner(); } - if (!total_to_download && target->expected_size()) + if (!total_to_download && target->get_expected_size()) { target->m_progress_bar.update_current(static_cast(now_downloaded)); } @@ -430,7 +440,7 @@ namespace mamba ); } - target->m_progress_bar.set_speed(static_cast(target->get_speed())); + target->m_progress_bar.set_speed(target->get_speed()); return 0; } @@ -471,21 +481,65 @@ namespace mamba m_curl_handle->set_opt(CURLOPT_NOBODY, yes); } - const std::string& DownloadTarget::name() const + const std::string& DownloadTarget::get_name() const { return m_name; } - const std::string& DownloadTarget::url() const + const std::string& DownloadTarget::get_url() const { return m_url; } - std::size_t DownloadTarget::expected_size() const + const std::string& DownloadTarget::get_etag() const + { + return m_etag; + } + + const std::string& DownloadTarget::get_mod() const + { + return m_mod; + } + + const std::string& DownloadTarget::get_cache_control() const + { + return m_cache_control; + } + + std::size_t DownloadTarget::get_expected_size() const { return m_expected_size; } + int DownloadTarget::get_http_status() const + { + return m_http_status; + } + + std::size_t DownloadTarget::get_downloaded_size() const + { + return m_downloaded_size; + } + + std::size_t DownloadTarget::get_speed() + { + auto speed = m_curl_handle->get_info(CURLINFO_SPEED_DOWNLOAD_T); + // TODO Should we just drop all code below with progress_bar and use value_or(0) in get_info + // above instead? + if (!speed.has_value()) + { + if (m_has_progress_bar) + { + return m_progress_bar.avg_speed(); + } + else + { + return 0; + } + } + return speed.value(); + } + static size_t discard(char*, size_t size, size_t nmemb, void*) { return size * nmemb; @@ -527,9 +581,9 @@ namespace mamba { LOG_INFO << "Downloading to filename: " << m_filename; - result = curl_easy_perform(m_curl_handle->handle()); - set_result(result); - return finalize(); + m_result = curl_easy_perform(m_curl_handle->handle()); + set_result(m_result); + return ((m_result == CURLE_OK) && finalize()); } CURL* DownloadTarget::handle() @@ -537,34 +591,15 @@ namespace mamba return m_curl_handle->handle(); } - curl_off_t DownloadTarget::get_speed() - { - auto speed = m_curl_handle->get_info(CURLINFO_SPEED_DOWNLOAD_T); - // TODO Should we just drop all code below with progress_bar and use value_or(0) in get_info - // above instead? - if (!speed.has_value()) - { - if (m_has_progress_bar) - { - return static_cast(m_progress_bar.avg_speed()); - } - else - { - return 0; - } - } - return speed.value(); - } - void DownloadTarget::set_result(CURLcode r) { - result = r; + m_result = r; if (r != CURLE_OK) { auto leffective_url = m_curl_handle->get_info(CURLINFO_EFFECTIVE_URL).value(); std::stringstream err; - err << "Download error (" << result << ") " << curl_easy_strerror(result) << " [" + err << "Download error (" << m_result << ") " << curl_easy_strerror(m_result) << " [" << leffective_url << "]\n"; if (m_curl_handle->get_error_buffer()[0] != '\0') { @@ -579,7 +614,7 @@ namespace mamba { m_progress_bar.update_progress(0, 1); // m_progress_bar.set_elapsed_time(); - m_progress_bar.set_postfix(curl_easy_strerror(result)); + m_progress_bar.set_postfix(curl_easy_strerror(m_result)); } if (!m_ignore_failure && !can_retry()) { @@ -588,12 +623,17 @@ namespace mamba } } + std::size_t DownloadTarget::get_result() const + { + return static_cast(m_result); + } + bool DownloadTarget::finalize() { - avg_speed = get_speed(); - http_status = m_curl_handle->get_info(CURLINFO_RESPONSE_CODE).value_or(10000); - effective_url = m_curl_handle->get_info(CURLINFO_EFFECTIVE_URL).value(); - downloaded_size = m_curl_handle->get_info(CURLINFO_SIZE_DOWNLOAD_T).value_or(0); + auto avg_speed = get_speed(); + m_http_status = m_curl_handle->get_info(CURLINFO_RESPONSE_CODE).value_or(10000); + m_effective_url = m_curl_handle->get_info(CURLINFO_EFFECTIVE_URL).value(); + m_downloaded_size = m_curl_handle->get_info(CURLINFO_SIZE_DOWNLOAD_T).value_or(0); LOG_INFO << get_transfer_msg(); @@ -612,22 +652,21 @@ namespace mamba m_next_retry = std::chrono::steady_clock::now() + std::chrono::seconds(m_retry_wait_seconds); std::stringstream msg; - msg << "Failed (" << http_status << "), retry in " << m_retry_wait_seconds << "s"; + msg << "Failed (" << m_http_status << "), retry in " << m_retry_wait_seconds << "s"; if (m_has_progress_bar) { - m_progress_bar.update_progress(0, static_cast(downloaded_size)); + m_progress_bar.update_progress(0, m_downloaded_size); m_progress_bar.set_postfix(msg.str()); } return false; } m_file.close(); - final_url = effective_url; if (m_has_progress_bar) { - m_progress_bar.set_speed(static_cast(avg_speed)); - m_progress_bar.set_total(static_cast(downloaded_size)); + m_progress_bar.set_speed(avg_speed); + m_progress_bar.set_total(m_downloaded_size); m_progress_bar.set_full(); m_progress_bar.set_postfix("Downloaded"); } @@ -645,7 +684,7 @@ namespace mamba } else { - Console::instance().print(name() + " completed"); + Console::instance().print(m_name + " completed"); } } @@ -671,8 +710,8 @@ namespace mamba std::string DownloadTarget::get_transfer_msg() { std::stringstream ss; - ss << "Transfer finalized, status: " << http_status << " [" << effective_url << "] " - << downloaded_size << " bytes"; + ss << "Transfer finalized, status: " << m_http_status << " [" << m_effective_url << "] " + << m_downloaded_size << " bytes"; return ss.str(); } @@ -741,7 +780,7 @@ namespace mamba if (msg->msg == CURLMSG_DONE) { - LOG_INFO << "Transfer done for '" << current_target->name() << "'"; + LOG_INFO << "Transfer done for '" << current_target->get_name() << "'"; // We are only interested in messages about finished transfers curl_multi_remove_handle(m_handle, current_target->handle()); @@ -751,12 +790,12 @@ namespace mamba // transfer did not work! can we retry? if (current_target->can_retry()) { - LOG_INFO << "Setting retry for '" << current_target->name() << "'"; + LOG_INFO << "Setting retry for '" << current_target->get_name() << "'"; m_retry_targets.push_back(current_target); } else { - if (failfast && current_target->ignore_failure() == false) + if (failfast && current_target->get_ignore_failure() == false) { throw std::runtime_error( "Multi-download failed. Reason: " + current_target->get_transfer_msg() @@ -789,7 +828,7 @@ namespace mamba m_targets.begin(), m_targets.end(), [](DownloadTarget* a, DownloadTarget* b) -> bool - { return a->expected_size() > b->expected_size(); } + { return a->get_expected_size() > b->get_expected_size(); } ); } diff --git a/libmamba/src/core/subdirdata.cpp b/libmamba/src/core/subdirdata.cpp index 0645c149db..776381379b 100644 --- a/libmamba/src/core/subdirdata.cpp +++ b/libmamba/src/core/subdirdata.cpp @@ -451,7 +451,7 @@ namespace mamba bool MSubdirData::finalize_check(const DownloadTarget& target) { - LOG_INFO << "Checked: " << target.url() << " [" << target.http_status << "]"; + LOG_INFO << "Checked: " << target.get_url() << " [" << target.get_http_status() << "]"; if (m_progress_bar_check) { m_progress_bar_check.repr().postfix.set_value("Checked"); @@ -460,9 +460,9 @@ namespace mamba m_progress_bar_check.mark_as_completed(); } - if (ends_with(target.url(), ".zst")) + if (ends_with(target.get_url(), ".zst")) { - this->m_metadata.has_zst = { target.http_status == 200, utc_time_now() }; + this->m_metadata.has_zst = { target.get_http_status() == 200, utc_time_now() }; } return true; } @@ -689,14 +689,14 @@ namespace mamba bool MSubdirData::finalize_transfer(const DownloadTarget&) { - if (m_target->result != 0 || m_target->http_status >= 400) + if (m_target->get_result() != 0 || m_target->get_http_status() >= 400) { - LOG_INFO << "Unable to retrieve repodata (response: " << m_target->http_status - << ") for '" << m_target->url() << "'"; + LOG_INFO << "Unable to retrieve repodata (response: " << m_target->get_http_status() + << ") for '" << m_target->get_url() << "'"; if (m_progress_bar) { - m_progress_bar.set_postfix(std::to_string(m_target->http_status) + " failed"); + m_progress_bar.set_postfix(std::to_string(m_target->get_http_status()) + " failed"); m_progress_bar.set_full(); m_progress_bar.mark_as_completed(); } @@ -704,9 +704,10 @@ namespace mamba return false; } - LOG_DEBUG << "HTTP response code: " << m_target->http_status; + LOG_DEBUG << "HTTP response code: " << m_target->get_http_status(); // Note HTTP status == 0 for files - if (m_target->http_status == 0 || m_target->http_status == 200 || m_target->http_status == 304) + if (m_target->get_http_status() == 0 || m_target->get_http_status() == 200 + || m_target->get_http_status() == 304) { m_download_complete = true; } @@ -714,14 +715,14 @@ namespace mamba { LOG_WARNING << "HTTP response code indicates error, retrying."; throw mamba_error( - "Unhandled HTTP code: " + std::to_string(m_target->http_status), + "Unhandled HTTP code: " + std::to_string(m_target->get_http_status()), mamba_error_code::subdirdata_not_loaded ); } fs::u8path json_file, solv_file; - if (m_target->http_status == 304) + if (m_target->get_http_status() == 304) { // cache still valid LOG_INFO << "Cache is still valid"; @@ -802,7 +803,7 @@ namespace mamba } } - LOG_DEBUG << "Finalized transfer of '" << m_target->url() << "'"; + LOG_DEBUG << "Finalized transfer of '" << m_target->get_url() << "'"; fs::u8path writable_cache_dir = create_cache_dir(m_writable_pkgs_dir); json_file = writable_cache_dir / m_json_fn; @@ -810,10 +811,10 @@ namespace mamba auto file_size = fs::file_size(m_temp_file->path()); - m_metadata.url = m_target->url(); - m_metadata.etag = m_target->etag; - m_metadata.mod = m_target->mod; - m_metadata.cache_control = m_target->cache_control; + m_metadata.url = m_target->get_url(); + m_metadata.etag = m_target->get_etag(); + m_metadata.mod = m_target->get_mod(); + m_metadata.cache_control = m_target->get_cache_control(); m_metadata.stored_file_size = file_size; if (!Context::instance().repodata_use_zst) diff --git a/libmamba/src/core/transaction.cpp b/libmamba/src/core/transaction.cpp index 5742b1bb17..0b22bd518e 100644 --- a/libmamba/src/core/transaction.cpp +++ b/libmamba/src/core/transaction.cpp @@ -133,11 +133,11 @@ namespace mamba void PackageDownloadExtractTarget::validate() { m_validation_result = VALIDATION_RESULT::VALID; - if (m_expected_size && size_t(m_target->downloaded_size) != m_expected_size) + if (m_expected_size && (m_target->get_downloaded_size() != m_expected_size)) { LOG_ERROR << "File not valid: file size doesn't match expectation " << m_tarball_path << "\nExpected: " << m_expected_size - << "\nActual: " << size_t(m_target->downloaded_size) << "\n"; + << "\nActual: " << m_target->get_downloaded_size() << "\n"; if (m_has_progress_bars) { m_download_bar.set_postfix("validation failed"); @@ -344,10 +344,10 @@ namespace mamba m_download_bar.mark_as_completed(); } - if (m_target->http_status >= 400) + if (m_target->get_http_status() >= 400) { LOG_ERROR << "Failed to download package from " << m_url << " (status " - << m_target->http_status << ")"; + << m_target->get_http_status() << ")"; m_validation_result = VALIDATION_RESULT::UNDEFINED; return false; } diff --git a/libmamba/src/core/validate.cpp b/libmamba/src/core/validate.cpp index e16eaacddc..16e94157fd 100644 --- a/libmamba/src/core/validate.cpp +++ b/libmamba/src/core/validate.cpp @@ -1451,10 +1451,7 @@ namespace mamba::validation if (dl_target->resource_exists()) { - auto result = curl_easy_perform(dl_target->handle()); - dl_target->set_result(result); - - if ((result == CURLE_OK) && dl_target->finalize()) + if (dl_target->perform()) { KeyMgrRole key_mgr = create_key_mgr(tmp_metadata_path); @@ -1617,10 +1614,7 @@ namespace mamba::validation if (dl_target->resource_exists()) { - auto result = curl_easy_perform(dl_target->handle()); - dl_target->set_result(result); - - if ((result == CURLE_OK) && dl_target->finalize()) + if (dl_target->perform()) { PkgMgrRole pkg_mgr = create_pkg_mgr(tmp_metadata_path); @@ -2163,10 +2157,7 @@ namespace mamba::validation if (dl_target->resource_exists()) { - auto result = curl_easy_perform(dl_target->handle()); - dl_target->set_result(result); - - if ((result == CURLE_OK) && dl_target->finalize()) + if (dl_target->perform()) { break; } diff --git a/libmamba/tests/src/core/test_transfer.cpp b/libmamba/tests/src/core/test_transfer.cpp index ee65c4e57d..e718e38b46 100644 --- a/libmamba/tests/src/core/test_transfer.cpp +++ b/libmamba/tests/src/core/test_transfer.cpp @@ -37,7 +37,7 @@ namespace mamba multi_dl.download(MAMBA_DOWNLOAD_FAILFAST); // File does not exist - CHECK_EQ(cf.target()->result, 37); + CHECK_EQ(cf.target()->get_result(), 37); } { const mamba::Channel& c = mamba::make_channel("conda-forge");