Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use correct url in metadata and mirrors #3816

Merged
merged 12 commits into from
Feb 21, 2025
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
Loading