Remove MatchSpec::url

mamba-org · Feb 28, 2024 · 3d7061b · 3d7061b
1 parent 5d9e022
commit 3d7061b
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 47 deletions.
diff --git a/libmamba/include/mamba/specs/match_spec.hpp b/libmamba/include/mamba/specs/match_spec.hpp
@@ -86,9 +86,6 @@ namespace mamba::specs
         [[nodiscard]] auto optional() const -> bool;
         void set_optional(bool opt);
 
-        // TODO as string_view conditional on channel type
-        [[nodiscard]] auto url() const -> const std::string&;
-
         [[nodiscard]] auto conda_build_form() const -> std::string;
         [[nodiscard]] auto str() const -> std::string;
 
@@ -115,8 +112,6 @@ namespace mamba::specs
         std::string m_name_space;
         BuildNumberSpec m_build_number;
         util::heap_optional<ExtraMembers> m_extra = {};  // unlikely data
-        // TODO can put inside channel
-        std::string m_url;
 
         auto extra() -> ExtraMembers&;
         [[nodiscard]] auto channel_is_file() const -> bool;

diff --git a/libmamba/src/specs/match_spec.cpp b/libmamba/src/specs/match_spec.cpp
@@ -36,7 +36,6 @@ namespace mamba::specs
                             .or_else([](specs::ParseError&& error) { throw std::move(error); })
                             .value();
         auto [_, pkg] = util::rsplit_once(out.m_channel->location(), '/');
-        out.m_url = util::path_or_url_to_url(spec);
 
         // Build string
         auto [head, tail] = util::rsplit_once(strip_archive_extension(pkg), '-');
@@ -388,6 +387,12 @@ namespace mamba::specs
                                 .or_else([](ParseError&& error) { throw std::move(error); })
                                 .value());
         }
+        if (const auto& val = at_or(extra, "url", ""); !val.empty())
+        {
+            out.set_channel(UnresolvedChannel::parse(val)
+                                .or_else([](ParseError&& error) { throw std::move(error); })
+                                .value());
+        }
         if (const auto& val = at_or(extra, "subdir", ""); !val.empty())
         {
             if (!out.m_channel.has_value())
@@ -404,10 +409,6 @@ namespace mamba::specs
                 );
             }
         }
-        if (const auto& val = at_or(extra, "url", ""); !val.empty())
-        {
-            out.m_url = val;
-        }
         if (const auto& val = at_or(extra, "fn", ""); !val.empty())
         {
             out.set_filename(val);
@@ -533,7 +534,7 @@ namespace mamba::specs
 
     auto MatchSpec::is_file() const -> bool
     {
-        return (!filename().empty()) || (!m_url.empty());
+        return !filename().empty();
     }
 
     auto MatchSpec::name_space() const -> const std::string&
@@ -701,11 +702,6 @@ namespace mamba::specs
         }
     }
 
-    auto MatchSpec::url() const -> const std::string&
-    {
-        return m_url;
-    }
-
     auto MatchSpec::conda_build_form() const -> std::string
     {
         const bool has_version = !m_version.is_explicitly_free();
@@ -747,6 +743,7 @@ namespace mamba::specs
         //     else:
         //         brackets.append("subdir=%s" % subdir_matcher)
 
+        // TODO change as attribute if complex URL, and has "url" if PackageUrl
         if (m_channel.has_value())
         {
             res << fmt::format("{}::", *m_channel);
@@ -816,12 +813,7 @@ namespace mamba::specs
             const auto& q = maybe_quote(feats);
             formatted_brackets.push_back(util::concat("features=", q, feats, q));
         }
-        if (const auto& u = url(); !u.empty())
-        {
-            const auto& q = maybe_quote(u);
-            formatted_brackets.push_back(util::concat("url=", q, u, q));
-        }
-        else if (const auto& fn = filename(); !fn.empty())
+        else if (const auto& fn = filename(); !fn.empty() && !channel_is_file())
         {
             // No "fn" when we have a URL
             const auto& q = maybe_quote(fn);

diff --git a/libmamba/tests/src/specs/test_match_spec.cpp b/libmamba/tests/src/specs/test_match_spec.cpp
@@ -90,7 +90,7 @@ TEST_SUITE("specs::match_spec")
             CHECK_EQ(ms.version().str(), "==6.4");
             CHECK_EQ(ms.build_string().str(), "h59595ed_2");
             CHECK_EQ(
-                ms.url(),
+                ms.channel().value().str(),
                 "https://conda.anaconda.org/conda-forge/linux-64/ncurses-6.4-h59595ed_2.conda"
             );
             CHECK_EQ(ms.filename(), "ncurses-6.4-h59595ed_2.conda");
@@ -104,7 +104,7 @@ TEST_SUITE("specs::match_spec")
             CHECK_EQ(ms.version().str(), "==0.1");
             CHECK_EQ(ms.build_string().str(), "conda_forge");
             CHECK_EQ(
-                ms.url(),
+                ms.channel().value().str(),
                 "https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
             );
             CHECK_EQ(ms.filename(), "_libgcc_mutex-0.1-conda_forge.tar.bz2");
@@ -117,7 +117,7 @@ TEST_SUITE("specs::match_spec")
             CHECK_EQ(ms.version().str(), "==11.2.0");
             CHECK_EQ(ms.build_string().str(), "h1d223b6_13");
             CHECK_EQ(
-                ms.url(),
+                ms.channel().value().str(),
                 "https://conda.anaconda.org/conda-forge/linux-64/libgcc-ng-11.2.0-h1d223b6_13.tar.bz2"
             );
             CHECK_EQ(ms.filename(), "libgcc-ng-11.2.0-h1d223b6_13.tar.bz2");
@@ -129,34 +129,21 @@ TEST_SUITE("specs::match_spec")
             CHECK_EQ(ms.name().str(), "_libgcc_mutex");
             CHECK_EQ(ms.version().str(), "==0.1");
             CHECK_EQ(ms.build_string().str(), "conda_forge");
-            if (util::on_win)
-            {
-                std::string driveletter = fs::absolute(fs::u8path("/")).string().substr(0, 1);
-                CHECK_EQ(
-                    ms.url(),
-                    util::concat(
-                        "file://",
-                        driveletter,
-                        ":/home/randomguy/Downloads/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
-                    )
-                );
-            }
-            else
-            {
-                CHECK_EQ(
-                    ms.url(),
-                    "file:///home/randomguy/Downloads/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
-                );
-            }
-
+            CHECK_EQ(
+                ms.channel().value().str(),
+                "/home/randomguy/Downloads/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
+            );
             CHECK_EQ(ms.filename(), "_libgcc_mutex-0.1-conda_forge.tar.bz2");
         }
         {
             auto ms = MatchSpec::parse(
                 "xtensor[url=file:///home/wolfv/Downloads/xtensor-0.21.4-hc9558a2_0.tar.bz2]"
             );
             CHECK_EQ(ms.name().str(), "xtensor");
-            CHECK_EQ(ms.url(), "file:///home/wolfv/Downloads/xtensor-0.21.4-hc9558a2_0.tar.bz2");
+            CHECK_EQ(
+                ms.channel().value().str(),
+                "file:///home/wolfv/Downloads/xtensor-0.21.4-hc9558a2_0.tar.bz2"
+            );
         }
         {
             auto ms = MatchSpec::parse("foo=1.0=2");
@@ -173,8 +160,12 @@ TEST_SUITE("specs::match_spec")
             auto ms = MatchSpec::parse(
                 "foo=1.0=2[md5=123123123, license=BSD-3, fn='test 123.tar.bz2', url='abcdef']"
             );
+            CHECK_EQ(ms.channel().value().str(), "abcdef");
             CHECK_EQ(ms.conda_build_form(), "foo 1.0.* 2");
-            CHECK_EQ(ms.str(), R"ms(foo=1.0=2[url=abcdef,md5=123123123,license=BSD-3])ms");
+            CHECK_EQ(
+                ms.str(),
+                R"ms(abcdef::foo=1.0=2[fn="test 123.tar.bz2",md5=123123123,license=BSD-3])ms"
+            );
         }
         {
             auto ms = MatchSpec::parse("libblas=*=*mkl");