MatchSpec small improvements (#3043)

* Small CQA * Add ChannelSpec::str * Add ChannelContext::make_channel for ChannelSpec * Split MatchSpec tests
mamba-org · Dec 7, 2023 · dfcb8c2 · dfcb8c2
1 parent df4aeac
commit dfcb8c2
Show file tree

Hide file tree

Showing 11 changed files with 362 additions and 279 deletions.
diff --git a/libmamba/include/mamba/core/channel_context.hpp b/libmamba/include/mamba/core/channel_context.hpp
@@ -51,7 +51,8 @@ namespace mamba
          */
         ChannelContext(ChannelResolveParams params, std::vector<Channel> has_zst);
 
-        auto make_channel(std::string_view name) -> const channel_list&;
+        [[nodiscard]] auto make_channel(specs::ChannelSpec spec) -> const channel_list&;
+        [[nodiscard]] auto make_channel(std::string_view name) -> const channel_list&;
 
         [[nodiscard]] auto params() const -> const specs::ChannelResolveParams&;
 

diff --git a/libmamba/include/mamba/core/match_spec.hpp b/libmamba/include/mamba/core/match_spec.hpp
@@ -31,7 +31,7 @@ namespace mamba
 
         bool is_simple() const;
 
-        static std::tuple<std::string, std::string> parse_version_and_build(const std::string& s);
+        static std::tuple<std::string, std::string> parse_version_and_build(std::string_view s);
         std::string spec;
 
         std::string name;

diff --git a/libmamba/include/mamba/specs/channel_spec.hpp b/libmamba/include/mamba/specs/channel_spec.hpp
@@ -11,6 +11,9 @@
 #include <string>
 #include <string_view>
 
+#include <fmt/core.h>
+#include <fmt/format.h>
+
 #include "mamba/util/flat_set.hpp"
 
 namespace mamba::specs
@@ -98,11 +101,32 @@ namespace mamba::specs
         [[nodiscard]] auto platform_filters() && -> dynamic_platform_set;
         auto clear_platform_filters() -> dynamic_platform_set;
 
+        [[nodiscard]] auto str() const -> std::string;
+
     private:
 
         std::string m_location = std::string(unknown_channel);
         dynamic_platform_set m_platform_filters = {};
         Type m_type = Type::Unknown;
     };
 }
+
+template <>
+struct fmt::formatter<mamba::specs::ChannelSpec>
+{
+    using ChannelSpec = ::mamba::specs::ChannelSpec;
+
+    constexpr auto parse(format_parse_context& ctx) -> format_parse_context::iterator
+    {
+        // make sure that range is empty
+        if (ctx.begin() != ctx.end() && *ctx.begin() != '}')
+        {
+            throw fmt::format_error("Invalid format");
+        }
+        return ctx.begin();
+    }
+
+    auto format(const ChannelSpec& spec, format_context& ctx) const -> format_context::iterator;
+};
+
 #endif
diff --git a/libmamba/src/core/channel_context.cpp b/libmamba/src/core/channel_context.cpp
@@ -214,6 +214,22 @@ namespace mamba
         return { std::move(params), has_zst };
     }
 
+    auto ChannelContext::make_channel(specs::ChannelSpec spec) -> const channel_list&
+    {
+        auto str = spec.str();
+        if (const auto it = m_channel_cache.find(str); it != m_channel_cache.end())
+        {
+            return it->second;
+        }
+
+        auto [it, inserted] = m_channel_cache.emplace(
+            std::move(str),
+            Channel::resolve(std::move(spec), params())
+        );
+        assert(inserted);
+        return it->second;
+    }
+
     auto ChannelContext::make_channel(std::string_view name) -> const channel_list&
     {
         if (const auto it = m_channel_cache.find(std::string(name)); it != m_channel_cache.end())

diff --git a/libmamba/src/core/match_spec.cpp b/libmamba/src/core/match_spec.cpp
@@ -41,14 +41,14 @@ namespace mamba
         parse(ctx, channel_context);
     }
 
-    std::tuple<std::string, std::string> MatchSpec::parse_version_and_build(const std::string& s)
+    std::tuple<std::string, std::string> MatchSpec::parse_version_and_build(std::string_view s)
     {
-        std::size_t pos = s.find_last_of(" =");
+        std::size_t const pos = s.find_last_of(" =");
         if (pos == s.npos || pos == 0)
         {
-            std::string tmp = s;
+            std::string tmp = std::string(s);
             util::replace_all(tmp, " ", "");
-            return { tmp, "" };
+            return { std::move(tmp), "" };
         }
         else
         {
@@ -59,17 +59,18 @@ namespace mamba
                 char d = s[pm1];
                 if (d == '=' || d == '!' || d == '|' || d == ',' || d == '<' || d == '>' || d == '~')
                 {
-                    std::string tmp = s;
+                    auto tmp = std::string(s);
                     util::replace_all(tmp, " ", "");
                     return { tmp, "" };
                 }
             }
             // c is either ' ' or pm1 is none of the forbidden chars
 
-            std::string v = s.substr(0, pos), b = s.substr(pos + 1);
+            auto v = std::string(s.substr(0, pos));
+            auto b = std::string(s.substr(pos + 1));
             util::replace_all(v, " ", "");
             util::replace_all(b, " ", "");
-            return { v, b };
+            return { std::move(v), std::move(b) };
         }
     }
 
@@ -124,7 +125,7 @@ namespace mamba
 
         auto extract_kv = [&spec_str](const std::string& kv_string, auto& map)
         {
-            static std::regex kv_re("([a-zA-Z0-9_-]+?)=([\"\']?)([^\'\"]*?)(\\2)(?:[\'\", ]|$)");
+            static const std::regex kv_re("([a-zA-Z0-9_-]+?)=([\"\']?)([^\'\"]*?)(\\2)(?:[\'\", ]|$)");
             std::cmatch kv_match;
             const char* text_iter = kv_string.c_str();
 

diff --git a/libmamba/src/specs/channel_spec.cpp b/libmamba/src/specs/channel_spec.cpp
@@ -163,7 +163,7 @@ namespace mamba::specs
         if (m_type == Type::Unknown)
         {
             m_location = unknown_channel;
-            m_platform_filters = {};
+            // Allowing in any platform filters for unkown type can be useful in MatchSpec
         }
         if (m_location.empty())
         {
@@ -208,4 +208,21 @@ namespace mamba::specs
     {
         return std::exchange(m_platform_filters, {});
     }
+
+    auto ChannelSpec::str() const -> std::string
+    {
+        return fmt::format("{}", *this);
+    }
+}
+
+auto
+fmt::formatter<mamba::specs::ChannelSpec>::format(const ChannelSpec& spec, format_context& ctx) const
+    -> format_context::iterator
+{
+    auto out = fmt::format_to(ctx.out(), "{}", spec.location());
+    if (!spec.platform_filters().empty())
+    {
+        out = fmt::format_to(ctx.out(), "[{}]", fmt::join(spec.platform_filters(), ","));
+    }
+    return out;
 }
diff --git a/libmamba/tests/CMakeLists.txt b/libmamba/tests/CMakeLists.txt
@@ -58,6 +58,7 @@ set(
     src/core/test_activation.cpp
     src/core/test_channel_context.cpp
     src/core/test_configuration.cpp
+    src/core/test_match_spec.cpp
     src/core/test_cpp.cpp
     src/core/test_downloader.cpp
     src/core/test_env_file_reading.cpp