From 41ad0021d13e60128cdbd0cd7f81ecf7dfaca9cc Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Thu, 19 Jan 2023 23:59:02 +0100 Subject: [PATCH] try to fix more mirror things --- include/powerloader/context.hpp | 4 ++ include/powerloader/download_target.hpp | 14 +---- src/download_target.cpp | 30 +++++----- src/downloader.cpp | 75 ++++++++++--------------- src/mirror.cpp | 2 +- src/python/main.cpp | 1 - src/target.cpp | 22 +++++--- 7 files changed, 66 insertions(+), 82 deletions(-) diff --git a/include/powerloader/context.hpp b/include/powerloader/context.hpp index ca9375f6..f0be5b53 100644 --- a/include/powerloader/context.hpp +++ b/include/powerloader/context.hpp @@ -35,8 +35,12 @@ namespace powerloader class mirror_map_type : private mirror_map_base { public: + using mirror_map_base::begin; using mirror_map_base::clear; + using mirror_map_base::empty; + using mirror_map_base::end; using mirror_map_base::mirror_map_base; + using mirror_map_base::size; // Get a list of unique mirorrs if existing for the provided host name, or an empty list // otherwise. diff --git a/include/powerloader/download_target.hpp b/include/powerloader/download_target.hpp index cff123d5..31153470 100644 --- a/include/powerloader/download_target.hpp +++ b/include/powerloader/download_target.hpp @@ -72,7 +72,6 @@ namespace powerloader void set_cache_options(const CacheControl& cache_control); void add_handle_options(CURLHandle& handle); - bool has_complete_url() const; bool validate_checksum(const fs::path& path); bool already_downloaded(); @@ -113,12 +112,6 @@ namespace powerloader m_base_url.clear(); } - - const std::string& complete_url() const noexcept - { - return m_complete_url; - } - const std::string& path() const noexcept { return m_path; @@ -167,14 +160,14 @@ namespace powerloader return m_range; } - bool is_zchunck() const noexcept + bool is_zchunk() const noexcept { return m_is_zchunk; } const zck_target& zck() const { - if (!is_zchunck()) // TODO: REVIEW: should this be an assert? + if (!is_zchunk()) // TODO: REVIEW: should this be an assert? throw std::invalid_argument("attempted to access zchunk data but there is none"); return *m_p_zck; } @@ -182,7 +175,7 @@ namespace powerloader // TODO: ownership/access issue: mostly modified outside zck_target& zck() { - if (!is_zchunck()) // TODO: REVIEW: should this be an assert? + if (!is_zchunk()) // TODO: REVIEW: should this be an assert? throw std::invalid_argument("attempted to access zchunk data but there is none"); return *m_p_zck; } @@ -288,7 +281,6 @@ namespace powerloader bool m_no_cache = false; bool m_head_only = false; - std::string m_complete_url; std::string m_path; std::string m_base_url; std::unique_ptr m_outfile; diff --git a/src/download_target.cpp b/src/download_target.cpp index df3891a0..cc7fa679 100644 --- a/src/download_target.cpp +++ b/src/download_target.cpp @@ -22,14 +22,8 @@ namespace powerloader , m_base_url(base_url) , m_destination_path(destination) { - if (path.find("://") != std::string::npos) - { - m_complete_url = path; - } - else if (base_url.find("://") != std::string::npos) - { - m_complete_url = join_url(base_url, path); - } + spdlog::warn( + "DownloadTarget::DownloadTarget: {}, {}, {}", path, base_url, destination.string()); #if WITH_ZCHUNK if (m_is_zchunk) @@ -59,10 +53,12 @@ namespace powerloader const std::string mirror_url = uh.url_without_path(); const fs::path dst = destination_path.empty() ? fs::path{ rsplit(path, "/", 1).back() } : destination_path; + spdlog::warn("SETTING MIRRORR >>>> {}, {}", host, mirror_url); - ctx.mirror_map.create_unique_mirror(host, ctx, mirror_url); + ctx.mirror_map.create_unique_mirror(mirror_url, ctx, mirror_url); - return std::make_shared(path.substr(1, std::string::npos), host, dst); + return std::make_shared( + path.substr(1, std::string::npos), mirror_url, dst); } else { @@ -73,7 +69,14 @@ namespace powerloader } const auto mirror = hostname_override ? hostname_override.value() : parts[0]; const auto path = parts[1]; - + if (!ctx.mirror_map.has_mirrors(mirror)) + { + throw std::runtime_error("Mirror " + mirror + " not found"); + } + else + { + spdlog::warn("Mirror {} already exists", mirror); + } fs::path dst = destination_path.empty() ? fs::path{ rsplit(path, "/", 1).back() } : destination_path; @@ -86,11 +89,6 @@ namespace powerloader DownloadTarget::~DownloadTarget() = default; - bool DownloadTarget::has_complete_url() const - { - return !m_complete_url.empty(); - } - bool DownloadTarget::validate_checksum(const fs::path& path) { if (m_checksums.empty()) diff --git a/src/downloader.cpp b/src/downloader.cpp index 4ffb49c2..b6bbf875 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -104,7 +104,7 @@ namespace powerloader target->headercb_interrupt_reason()) }); } #ifdef WITH_ZCHUNK - else if (target->range_fail()) + else if (target->range_fail() && target->target().is_zchunk()) { zckRange* range = zck_dl_get_range(target->target().zck().zck_dl); int range_count = zck_get_range_count(range); @@ -115,7 +115,7 @@ namespace powerloader target->mirror()->stats().max_ranges); } } - else if (target->target().zck().zck_dl != nullptr + else if (target->target().is_zchunk() && target->target().zck().zck_dl != nullptr && zck_is_error(zck_dl_get_zck(target->target().zck().zck_dl)) > 0) { zckCtx* zck = zck_dl_get_zck(target->target().zck().zck_dl); @@ -263,7 +263,7 @@ namespace powerloader } if (mirrors_iterated == 0 && mirror->protocol() == Protocol::kFTP - && target->target().is_zchunck()) + && target->target().is_zchunk()) { continue; } @@ -302,7 +302,7 @@ namespace powerloader return tl::unexpected(DownloaderError( { ErrorLevel::FATAL, ErrorCode::PD_NOURL, - fmt::format("No suitable mirror found for {}", target->target().complete_url()) })); + fmt::format("No suitable mirror found for {}", target->target().base_url()) })); } // Select next target @@ -317,12 +317,9 @@ namespace powerloader if (target->state() != DownloadState::kWAITING) continue; - // Determine if path is a complete URL - bool complete_url_in_path = target->target().has_complete_url(); - bool have_mirrors = !target->mirrors().empty(); // Sanity check - if (target->target().base_url().empty() && !have_mirrors && !complete_url_in_path) + if (target->target().base_url().empty() && !have_mirrors) { // Used relative path with empty internal mirrorlist and no basepath specified! return tl::unexpected(DownloaderError{ @@ -332,43 +329,36 @@ namespace powerloader } // Prepare full target URL - if (complete_url_in_path) + auto res = select_suitable_mirror(target); + if (!res) { - full_url = target->target().complete_url(); + // TODO: review this: why is the callback called without changing the state + // of the target? (see Target::set_failed() for example). + target->call_end_callback(TransferStatus::kERROR); + return tl::unexpected(res.error()); } - else - { - // Find a suitable mirror - auto res = select_suitable_mirror(target); - if (!res) - { - // TODO: review this: why is the callback called without changing the state - // of the target? (see Target::set_failed() for example). - target->call_end_callback(TransferStatus::kERROR); - return tl::unexpected(res.error()); - } - mirror = res.value(); + mirror = res.value(); - assert(mirror); + assert(mirror); - // TODO: create a `name()` or similar function - spdlog::info("Selected mirror: {}", mirror->url()); - if (mirror && !mirror->needs_preparation(target)) - { - full_url = mirror->format_url(target); - target->change_mirror(mirror); - } - else + // TODO: create a `name()` or similar function + spdlog::info("Selected mirror: {}", mirror->url()); + if (mirror && !mirror->needs_preparation(target)) + { + full_url = mirror->format_url(target); + target->change_mirror(mirror); + } + else + { + // No free mirror + if (!mirror->needs_preparation(target)) { - // No free mirror - if (!mirror->needs_preparation(target)) - { - spdlog::info("Currently there is no free mirror for {}", - target->target().path()); - } + spdlog::info("Currently there is no free mirror for {}", + target->target().path()); } } + // } // If LRO_OFFLINE is specified, check if the obtained full_url is local or not // This condition should never be true for a full_url built from a mirror, because @@ -579,7 +569,6 @@ namespace powerloader if (!result) { // int complete_url_in_path = strstr(target->target().path(), "://") ? 1 : 0; - int complete_url_in_path = false; bool retry = false; @@ -629,12 +618,11 @@ namespace powerloader current_target->lower_mirror_parallel_connections(); } + // TODO FIX remove complete url stuff // complete_url_in_path and target->base_url() doesn't have an // alternatives like using mirrors, therefore they are handled // differently - std::string complete_url_or_base_url - = complete_url_in_path ? current_target->target().path() - : current_target->target().base_url(); + std::string complete_url_or_base_url = current_target->target().path(); if (can_retry_download(static_cast(current_target->retries()), complete_url_or_base_url)) { @@ -664,8 +652,7 @@ namespace powerloader if (!retry) { // No more mirrors to try or base_url used or fatal error - spdlog::error("Retries exceeded for {}", - current_target->target().complete_url()); + spdlog::error("Retries exceeded for {}", current_target->target().base_url()); assert(!result); const CbReturnCode rc = current_target->set_failed(result.error()); @@ -829,7 +816,7 @@ namespace powerloader for (auto* dl_target : dl_targets) { - if (dl_target->is_zchunck()) + if (dl_target->is_zchunk()) { fs::path p = dl_target->destination_path(); try diff --git a/src/mirror.cpp b/src/mirror.cpp index 9377eeac..108a3a6a 100644 --- a/src/mirror.cpp +++ b/src/mirror.cpp @@ -125,7 +125,7 @@ namespace powerloader std::string Mirror::format_url(Target* target) const { - return fmt::format("{}/{}", m_url, target->target().path()); + return join_url(m_url, target->target().path()); } /** Sort mirrors. Penalize the error ones. diff --git a/src/python/main.cpp b/src/python/main.cpp index aee61157..429b1b70 100644 --- a/src/python/main.cpp +++ b/src/python/main.cpp @@ -24,7 +24,6 @@ PYBIND11_MODULE(pypowerloader, m) py::class_>(m, "DownloadTarget") .def(py::init()) - .def_property_readonly("complete_url", &DownloadTarget::complete_url) .def_property("progress_callback", &DownloadTarget::progress_callback, &DownloadTarget::set_progress_callback); diff --git a/src/target.cpp b/src/target.cpp index 95e8ea83..ff093798 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -20,6 +20,10 @@ namespace powerloader , m_mirrors(std::move(mirrors)) , m_ctx(ctx) { + for (auto& m : m_mirrors) + { + spdlog::warn("Mirror: {} for target {}", m->url(), m_target->path()); + } } Target::~Target() @@ -30,7 +34,7 @@ namespace powerloader bool Target::zck_running() const { #ifdef WITH_ZCHUNK - return m_target->is_zchunck() && m_zck_state != ZckState::kFINISHED; + return m_target->is_zchunk() && m_zck_state != ZckState::kFINISHED; #else return false; #endif @@ -186,7 +190,7 @@ namespace powerloader } #ifdef WITH_ZCHUNK - if (target->target().is_zchunck() && !target->m_range_fail && target->m_mirror + if (target->target().is_zchunk() && !target->m_range_fail && target->m_mirror && target->m_mirror->protocol() == Protocol::kHTTP) return zckheadercb(buffer, size, nitems, self); #endif /* WITH_ZCHUNK */ @@ -326,7 +330,7 @@ namespace powerloader std::size_t cur_written_expected = nitems, cur_written; #ifdef WITH_ZCHUNK - if (self->m_target->is_zchunck() && !self->m_range_fail && self->m_mirror + if (self->m_target->is_zchunk() && !self->m_range_fail && self->m_mirror && self->m_mirror->protocol() == Protocol::kHTTP) { return zckwritecb(buffer, size, nitems, self); @@ -439,7 +443,7 @@ namespace powerloader } #ifdef WITH_ZCHUNK - if (target->m_target->is_zchunck()) + if (target->m_target->is_zchunk()) { total_to_download = target->m_target->zck().total_to_download; now_downloaded = now_downloaded + target->m_target->zck().downloaded; @@ -553,14 +557,14 @@ namespace powerloader } // Prepare FILE #ifdef WITH_ZCHUNK - if (!m_target->is_zchunck()) + if (!m_target->is_zchunk()) { #endif this->open_target_file(); #ifdef WITH_ZCHUNK } // If file is zchunk, prep it - if (m_target->is_zchunck()) + if (m_target->is_zchunk()) { if (!m_target->outfile()) { @@ -712,7 +716,7 @@ namespace powerloader tl::expected Target::finish_transfer(const std::string& effective_url) { #ifdef WITH_ZCHUNK - if (m_target->is_zchunck()) + if (m_target->is_zchunk()) { if (m_zck_state == ZckState::kHEADER_LEAD) { @@ -865,7 +869,7 @@ namespace powerloader m_retries++; #ifdef WITH_ZCHUNK - if (!m_target->is_zchunck() || m_zck_state == ZckState::kHEADER) + if (!m_target->is_zchunk() || m_zck_state == ZckState::kHEADER) { #endif // Truncate file - remove downloaded garbage (error html page etc.) @@ -882,7 +886,7 @@ namespace powerloader void Target::finalize_transfer(const std::string& effective_url) { #ifdef WITH_ZCHUNK - if (m_target->is_zchunck() && m_zck_state != ZckState::kFINISHED) + if (m_target->is_zchunk() && m_zck_state != ZckState::kFINISHED) { m_state = DownloadState::kWAITING; if (m_mirror)