Skip to content

Commit

Permalink
Use correct url in metadata and mirrors (#3816)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hind-M authored Feb 21, 2025
1 parent 148a25c commit 8824be1
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 40 deletions.
4 changes: 2 additions & 2 deletions docs/source/developer_zone/changes-2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -198,5 +198,5 @@ Listing packages in the created ``pandoc_from_oci`` environment:
$ micromamba list -n pandoc_from_oci
Name Version Build Channel
─────────────────────────────────────────────────────────────────────────────────────
pandoc 3.2 ha770c72_0 https://pkg-containers.githubusercontent.com/ghcr1/blobs
──────────────────────────────────────────
pandoc 3.2 ha770c72_0 conda-forge
3 changes: 1 addition & 2 deletions libmamba/include/mamba/core/package_fetcher.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ namespace mamba

struct CheckSumParams;

bool is_local_package() const;
bool use_explicit_https_url() const;
bool use_oci() const;
const std::string& filename() const;
std::string channel() const;
std::string url_path() const;
Expand Down
3 changes: 3 additions & 0 deletions libmamba/include/mamba/core/subdirdata.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ namespace mamba
);

std::string repodata_url_path() const;
const std::string& repodata_full_url() const;

void
load(MultiPackageCache& caches, ChannelContext& channel_context, const specs::Channel& channel);
Expand Down Expand Up @@ -185,6 +186,8 @@ namespace mamba
std::string m_solv_fn;
bool m_is_noarch;

std::string m_full_url;

SubdirMetadata m_metadata;
std::unique_ptr<TemporaryFile> m_temp_file;
const Context* p_context;
Expand Down
48 changes: 25 additions & 23 deletions libmamba/src/core/package_fetcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ namespace mamba
cb.value()(success.transfer.downloaded_size);
}
m_needs_download = false;
m_downloaded_url = success.transfer.effective_url;
m_downloaded_url = m_package_info.package_url;
return expected_t<void>();
};

Expand Down Expand Up @@ -317,45 +317,47 @@ namespace mamba
/*******************
* Private methods *
*******************/
bool PackageFetcher::is_local_package() const
{
return util::starts_with(m_package_info.package_url, "file://");
}

bool PackageFetcher::use_explicit_https_url() const
const std::string& PackageFetcher::filename() const
{
// This excludes OCI case, which uses explicitly a "oci://" scheme,
// but is resolved later to something starting with `oci_base_url`
constexpr std::string_view oci_base_url = "https://pkg-containers.githubusercontent.com/";
return util::starts_with(m_package_info.package_url, "https://")
&& !util::starts_with(m_package_info.package_url, oci_base_url);
return m_package_info.filename;
}

const std::string& PackageFetcher::filename() const
bool PackageFetcher::use_oci() const
{
return m_package_info.filename;
constexpr std::string_view oci_scheme = "oci://";
return util::starts_with(m_package_info.package_url, oci_scheme);
}

// NOTE
// In the general case (not fetching from an oci registry),
// `channel()` and `url_path()` are concatenated when passed to `HTTPMirror`
// and the channel is resolved if needed (using the channel alias).
// Therefore, `util::url_concat("", m_package_info.package_url)`
// and `util::url_concat(m_package_info.channel, m_package_info.platform,
// m_package_info.filename)` should be equivalent, except when an explicit url is used as a spec
// with `--override-channels` option.
// Hence, we are favoring the first option (returning "" and `m_package_info.package_url`
// to be concatenated), valid for all the mentioned cases used with `HTTPMirror`.
// In the case of fetching from oci registries (using `OCIMirror`),the actual url
// used is built differently, and returning `m_package_info.package_url` is not relevant
// (i.e oci://ghcr.io/<mirror>/<channel>/<platform>/<filename>).
std::string PackageFetcher::channel() const
{
if (is_local_package() || use_explicit_https_url())
if (use_oci())
{
// Use explicit url or local package path
// to fetch package, leaving the channel empty.
return "";
return m_package_info.channel;
}
return m_package_info.channel;
return "";
}

std::string PackageFetcher::url_path() const
{
if (is_local_package() || use_explicit_https_url())
if (use_oci())
{
// Use explicit url or local package path
// to fetch package.
return m_package_info.package_url;
return util::concat(m_package_info.platform, '/', m_package_info.filename);
}
return util::concat(m_package_info.platform, '/', m_package_info.filename);
return m_package_info.package_url;
}

const std::string& PackageFetcher::url() const
Expand Down
8 changes: 7 additions & 1 deletion libmamba/src/core/subdirdata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ namespace mamba
, m_is_noarch(platform == "noarch")
, p_context(&(ctx))
{
m_full_url = util::url_concat(channel.url().str(), "/", repodata_url_path());
assert(!channel.is_package());
m_forbid_cache = (channel.mirror_urls().size() == 1u)
&& util::starts_with(channel.url().str(), "file://");
Expand All @@ -591,6 +592,11 @@ namespace mamba
return util::concat(m_platform, "/", m_repodata_fn);
}

const std::string& SubdirData::repodata_full_url() const
{
return m_full_url;
}

void
SubdirData::load(MultiPackageCache& caches, ChannelContext& channel_context, const specs::Channel& channel)
{
Expand Down Expand Up @@ -773,7 +779,7 @@ namespace mamba
}
else
{
return finalize_transfer(SubdirMetadata::HttpMetadata{ success.transfer.effective_url,
return finalize_transfer(SubdirMetadata::HttpMetadata{ repodata_full_url(),
success.etag,
success.last_modified,
success.cache_control });
Expand Down
1 change: 1 addition & 0 deletions libmamba/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ set(
src/core/test_invoke.cpp
src/core/test_lockfile.cpp
src/core/test_output.cpp
src/core/test_package_fetcher.cpp
src/core/test_pinning.cpp
src/core/test_progress_bar.cpp
src/core/test_shell_init.cpp
Expand Down
89 changes: 89 additions & 0 deletions libmamba/tests/src/core/test_package_fetcher.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright (c) 2025, QuantStack and Mamba Contributors
//
// Distributed under the terms of the BSD 3-Clause License.
//
// The full license is in the file LICENSE, distributed with this software.

#include <catch2/catch_all.hpp>

#include "mamba/core/context.hpp"
#include "mamba/core/package_fetcher.hpp"

namespace
{
using namespace mamba;

TEST_CASE("build_download_request")
{
auto ctx = Context();
MultiPackageCache package_caches{ ctx.pkgs_dirs, ctx.validation_params };

SECTION("From conda-forge")
{
static constexpr std::string_view url = "https://conda.anaconda.org/conda-forge/linux-64/pkg-6.4-bld.conda";
auto pkg_info = specs::PackageInfo::from_url(url).value();

PackageFetcher pkg_fetcher{ pkg_info, package_caches };
REQUIRE(pkg_fetcher.name() == pkg_info.name);

auto req = pkg_fetcher.build_download_request();
// Should correspond to package name
REQUIRE(req.name == pkg_info.name);
// Should correspond to PackageFetcher::channel()
REQUIRE(req.mirror_name == "");
// Should correspond to PackageFetcher::url_path()
REQUIRE(req.url_path == url);
}

SECTION("From some mirror")
{
static constexpr std::string_view url = "https://repo.prefix.dev/emscripten-forge-dev/emscripten-wasm32/cpp-tabulate-1.5.0-h7223423_2.tar.bz2";
auto pkg_info = specs::PackageInfo::from_url(url).value();

PackageFetcher pkg_fetcher{ pkg_info, package_caches };
REQUIRE(pkg_fetcher.name() == pkg_info.name);

auto req = pkg_fetcher.build_download_request();
// Should correspond to package name
REQUIRE(req.name == pkg_info.name);
// Should correspond to PackageFetcher::channel()
REQUIRE(req.mirror_name == "");
// Should correspond to PackageFetcher::url_path()
REQUIRE(req.url_path == url);
}

SECTION("From local file")
{
static constexpr std::string_view url = "file:///home/wolfv/Downloads/xtensor-0.21.4-hc9558a2_0.tar.bz2";
auto pkg_info = specs::PackageInfo::from_url(url).value();

PackageFetcher pkg_fetcher{ pkg_info, package_caches };
REQUIRE(pkg_fetcher.name() == pkg_info.name);

auto req = pkg_fetcher.build_download_request();
// Should correspond to package name
REQUIRE(req.name == pkg_info.name);
// Should correspond to PackageFetcher::channel()
REQUIRE(req.mirror_name == "");
// Should correspond to PackageFetcher::url_path()
REQUIRE(req.url_path == url);
}

SECTION("From oci")
{
static constexpr std::string_view url = "oci://ghcr.io/channel-mirrors/conda-forge/linux-64/xtensor-0.25.0-h00ab1b0_0.conda";
auto pkg_info = specs::PackageInfo::from_url(url).value();

PackageFetcher pkg_fetcher{ pkg_info, package_caches };
REQUIRE(pkg_fetcher.name() == pkg_info.name);

auto req = pkg_fetcher.build_download_request();
// Should correspond to package name
REQUIRE(req.name == pkg_info.name);
// Should correspond to PackageFetcher::channel()
REQUIRE(req.mirror_name == "oci://ghcr.io/channel-mirrors/conda-forge");
// Should correspond to PackageFetcher::url_path()
REQUIRE(req.url_path == "linux-64/xtensor-0.25.0-h00ab1b0_0.conda");
}
}
}
48 changes: 36 additions & 12 deletions micromamba/tests/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -1304,13 +1304,10 @@ def test_create_with_non_existing_subdir(tmp_home, tmp_root_prefix, tmp_path):
"https://conda.anaconda.org/conda-forge/linux-64/abacus-3.2.4-hb6c440e_0.conda",
],
)
def test_create_with_explicit_url(tmp_home, tmp_root_prefix, tmp_path, spec):
def test_create_with_explicit_url(tmp_home, tmp_root_prefix, spec):
"""Attempts to install a package using an explicit url."""
empty_root_prefix = tmp_path / "empty-root-create-from-explicit-url"
env_name = "env-create-from-explicit-url"

os.environ["MAMBA_ROOT_PREFIX"] = str(empty_root_prefix)

res = helpers.create(
spec, "--no-env", "-n", env_name, "--override-channels", "--json", default_channel=False
)
Expand All @@ -1337,6 +1334,33 @@ def test_create_with_explicit_url(tmp_home, tmp_root_prefix, tmp_path, spec):
assert pkgs[0]["channel"] == "https://conda.anaconda.org/conda-forge"


def test_create_from_mirror(tmp_home, tmp_root_prefix):
"""
Attempts to install a package using an explicit channel/mirror.
Non-regression test for https://github.com/mamba-org/mamba/issues/3804
"""
env_name = "env-create-from-mirror"

res = helpers.create(
"cpp-tabulate",
"-n",
env_name,
"-c",
"https://repo.prefix.dev/emscripten-forge-dev",
"--platform=emscripten-wasm32",
"--json",
default_channel=False,
)
assert res["success"]

assert any(
package["name"] == "cpp-tabulate"
and package["channel"] == "https://repo.prefix.dev/emscripten-forge-dev"
and package["subdir"] == "emscripten-wasm32"
for package in res["actions"]["LINK"]
)


@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
def test_create_with_multiple_files(tmp_home, tmp_root_prefix, tmpdir):
env_name = "myenv"
Expand Down Expand Up @@ -1458,8 +1482,8 @@ def test_create_from_oci_mirrored_channels(tmp_home, tmp_root_prefix, tmp_path,
assert pkg["name"] == "pandoc"
if spec == "pandoc=3.1.13":
assert pkg["version"] == "3.1.13"
assert pkg["base_url"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
assert pkg["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
assert pkg["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
assert pkg["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"


@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
Expand Down Expand Up @@ -1487,14 +1511,14 @@ def test_create_from_oci_mirrored_channels_with_deps(tmp_home, tmp_root_prefix,
assert len(packages) > 2
assert any(
package["name"] == "xtensor"
and package["base_url"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
and package["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
and package["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
and package["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"
for package in packages
)
assert any(
package["name"] == "xtl"
and package["base_url"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
and package["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
and package["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
and package["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"
for package in packages
)

Expand Down Expand Up @@ -1528,8 +1552,8 @@ def test_create_from_oci_mirrored_channels_pkg_name_mapping(
assert len(packages) == 1
pkg = packages[0]
assert pkg["name"] == "_go_select"
assert pkg["base_url"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
assert pkg["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
assert pkg["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
assert pkg["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"


@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
Expand Down

0 comments on commit 8824be1

Please sign in to comment.