Use correct `url` in metadata and mirrors (#3816)

docs/source/developer_zone/changes-2.0.rst

@@ -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

libmamba/include/mamba/core/package_fetcher.hpp

@@ -101,8 +101,7 @@ namespace mamba
 
         struct CheckSumParams;
 
-        bool is_local_package() const;
+        bool use_oci() const;
-        bool use_explicit_https_url() const;
         const std::string& filename() const;
         std::string channel() const;
         std::string url_path() const;

libmamba/include/mamba/core/subdirdata.hpp

@@ -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);
@@ -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;

libmamba/src/core/package_fetcher.cpp

@@ -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>();
         };
 
@@ -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
-    {
-        // 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);
-    }
 
     const std::string& PackageFetcher::filename() const
     {
         return m_package_info.filename;
     }
 
+    bool PackageFetcher::use_oci() const
+    {
+        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
+            return m_package_info.channel;
-            // to fetch package, leaving the channel empty.
-            return "";
         }
-        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
+            return util::concat(m_package_info.platform, '/', m_package_info.filename);
-            // to fetch package.
-            return m_package_info.package_url;
         }
-        return util::concat(m_package_info.platform, '/', m_package_info.filename);
+        return m_package_info.package_url;
     }
 
     const std::string& PackageFetcher::url() const

libmamba/src/core/subdirdata.cpp

@@ -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://");
@@ -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)
     {
@@ -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 });

libmamba/tests/CMakeLists.txt

@@ -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

libmamba/tests/src/core/test_package_fetcher.cpp

@@ -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");
+        }
+    }
+}

micromamba/tests/test_create.py

@@ -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
     )
@@ -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"
@@ -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["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
-    assert pkg["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
+    assert pkg["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"
 
 
 @pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
@@ -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["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
-        and package["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
+        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["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
-        and package["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
+        and package["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"
         for package in packages
     )
 
@@ -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["base_url"] == "oci://ghcr.io/channel-mirrors/conda-forge"
-    assert pkg["channel"] == "https://pkg-containers.githubusercontent.com/ghcr1/blobs"
+    assert pkg["channel"] == "oci://ghcr.io/channel-mirrors/conda-forge"
 
 
 @pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)