Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

try to fix more mirror things #146

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/powerloader/context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ namespace powerloader
class mirror_map_type : private mirror_map_base
{
public:
using mirror_map_base::begin;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially voluntarilly didnt expose begin/end because that breaks the garantee that the values of the vector are unique (because then we can access and modify the vectors).
Suggestions: either

  1. use as_map() which makes a copy, not modifying this container;
  2. or expose the const versions of begin/end only.

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;

Expand Down
25 changes: 6 additions & 19 deletions include/powerloader/download_target.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -103,20 +102,9 @@ namespace powerloader
return m_no_cache;
}

const std::string& base_url() const noexcept
const std::string& mirror_name() const noexcept
{
return m_base_url;
}

void clear_base_url()
{
m_base_url.clear();
}


const std::string& complete_url() const noexcept
{
return m_complete_url;
return m_mirror_name;
}

const std::string& path() const noexcept
Expand Down Expand Up @@ -167,22 +155,22 @@ 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;
}

// 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;
}
Expand Down Expand Up @@ -288,9 +276,8 @@ 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::string m_mirror_name;
std::unique_ptr<FileIO> m_outfile;

fs::path m_destination_path;
Expand Down
2 changes: 1 addition & 1 deletion include/powerloader/downloader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace powerloader
bool is_max_mirrors_unlimited();

tl::expected<std::shared_ptr<Mirror>, DownloaderError> select_suitable_mirror(
Target* target);
const Target& target);

tl::expected<std::pair<Target*, std::string>, DownloaderError> select_next_target();

Expand Down
2 changes: 1 addition & 1 deletion src/cli/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ handle_download(Context& ctx,
target->set_progress_callback(std::bind(&progress_callback, target.get(), _1, _2));

spdlog::info("Downloading {} from {} to {}",
target->mirror_name(),
target->path(),
target->base_url(),
target->destination_path().string());
targets.push_back(std::move(target));
}
Expand Down
44 changes: 26 additions & 18 deletions src/download_target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,17 @@ namespace powerloader
#endif

DownloadTarget::DownloadTarget(const std::string& path,
const std::string& base_url,
const std::string& mirror_name,
const fs::path& destination)
: m_is_zchunk(ends_with(path, ".zck"))
, m_path(path)
, m_base_url(base_url)
, m_mirror_name(mirror_name)
, 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: {}::{} -> {}",
m_mirror_name,
m_path,
destination.string());

#if WITH_ZCHUNK
if (m_is_zchunk)
Expand All @@ -53,16 +49,26 @@ namespace powerloader
// we want to create a "mirror" for `http://test.com` to make sure we correctly
// retry and wait on mirror failures
URLHandler uh{ target_url };
if (uh.scheme() == "file")
{
spdlog::warn("PATH: {}", uh.path());
ctx.mirror_map.create_unique_mirror<Mirror>("[file]", ctx, "file://");
return std::make_shared<DownloadTarget>(uh.path(), "[file]", destination_path);
}

const std::string url = uh.url();
const std::string host = hostname_override ? hostname_override.value() : uh.host();
const std::string path = uh.path();
const std::string mirror_url = uh.url_without_path();
const fs::path dst = destination_path.empty() ? fs::path{ rsplit(path, "/", 1).back() }
: destination_path;

ctx.mirror_map.create_unique_mirror<Mirror>(host, ctx, mirror_url);
spdlog::warn("SETTING MIRRORR >>>> {}, {}", host, mirror_url);

ctx.mirror_map.create_unique_mirror<Mirror>(mirror_url, ctx, mirror_url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure: the original code in the cli's main.cpp was using the variable host as key, which is why it was like that before. It doesnt change much of the meaning here but I'm not sure if it's ok.


return std::make_shared<DownloadTarget>(path.substr(1, std::string::npos), host, dst);
return std::make_shared<DownloadTarget>(
path.substr(1, std::string::npos), mirror_url, dst);
}
else
{
Expand All @@ -73,7 +79,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;

Expand All @@ -86,11 +99,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())
Expand Down
Loading