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

[micromamba] [mamba] Fix wrong download url for custom channels #2596

Merged
merged 5 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions libmamba/include/mamba/core/util_string.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ namespace mamba
*/
const char* raw_str_or_empty(const char* ptr);

/**
* Return the common substring of two strings by blocks located between the '/' separator,
* and considering that the common substring would be located at the end of str1 (search from
* left to right).
* str1 is considered smaller than (or equal to) str2.
* cf. Channels use case.
*/
std::string get_common_substr(std::string_view str1, std::string_view str2);

/**
* Safe non utf-8 wrapping of <cctype> (see its doc).
*/
Expand Down
18 changes: 15 additions & 3 deletions libmamba/src/core/channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,10 +537,22 @@ namespace mamba
// of the channel (e.g. -c testchannel/mylabel/xyz)
// needs to result in `name = private/testchannel/mylabel/xyz`
std::string combined_name = it->second.name();
if (name != combined_name && name.find('/') != std::string::npos)
if (combined_name != name)
{
combined_name += "/";
combined_name += name.substr(name.find('/') + 1, std::string::npos);
// Find common string between `name` and `combined_name`
auto common_str = get_common_substr(combined_name, name);
// Combine names properly
if (common_str.empty())
{
combined_name += "/" + name;
}
else
{
// NOTE We assume that the `common_str`, if not empty, is necessarily at the
// beginning of `name` and at the end of `combined_name` (I don't know about
// other use cases for now)
combined_name += name.substr(common_str.size());
}
}

return Channel(
Expand Down
21 changes: 21 additions & 0 deletions libmamba/src/core/util_string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,27 @@

namespace mamba
{
/********************************************************
* Implementation of Channels use case util function *
*******************************************************/
Hind-M marked this conversation as resolved.
Show resolved Hide resolved
std::string get_common_substr(std::string_view str1, std::string_view str2)
Hind-M marked this conversation as resolved.
Show resolved Hide resolved
{
std::string common_str{ str1 };
while ((str2.find(common_str) == std::string::npos))
{
if (common_str.find('/') != std::string::npos)
{
common_str = common_str.substr(common_str.find('/') + 1);
}
else
{
common_str = "";
break;
}
}
return common_str;
}

/****************************************
* Implementation of cctype functions *
****************************************/
Expand Down
17 changes: 17 additions & 0 deletions libmamba/tests/src/core/test_channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ namespace mamba
auto& ctx = Context::instance();
ctx.custom_channels = {
{ "test_channel", "https://server.com/private/channels" },
{ "random/test_channel", "https://server.com/random/channels" },
};
ChannelContext channel_context;

Expand Down Expand Up @@ -381,6 +382,22 @@ namespace mamba
CHECK_EQ(c.urls(), exp_urls);
}

{
std::string value = "random/test_channel/pkg";
const Channel& c = channel_context.make_channel(value);
CHECK_EQ(c.scheme(), "https");
CHECK_EQ(c.location(), "server.com/random/channels");
CHECK_EQ(c.name(), "random/test_channel/pkg");
CHECK_EQ(c.canonical_name(), "random/test_channel/pkg");
CHECK_EQ(c.platforms(), std::vector<std::string>({ platform, "noarch" }));
std::vector<std::string> exp_urls(
{ std::string("https://server.com/random/channels/random/test_channel/pkg/")
+ platform,
std::string("https://server.com/random/channels/random/test_channel/pkg/noarch") }
);
CHECK_EQ(c.urls(), exp_urls);
}

ctx.channel_alias = "https://conda.anaconda.org";
ctx.custom_channels.clear();
}
Expand Down
16 changes: 16 additions & 0 deletions libmamba/tests/src/core/test_util_string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,22 @@ namespace mamba
{
CHECK_EQ(concat("aa", std::string("bb"), std::string_view("cc"), 'd'), "aabbccd");
}

TEST_CASE("get_common_substr")
{
CHECK_EQ(get_common_substr("", ""), "");
CHECK_EQ(get_common_substr("", "test"), "");
CHECK_EQ(get_common_substr("test", "test"), "test");
CHECK_EQ(get_common_substr("test/chan", "test/chan"), "test/chan");
Hind-M marked this conversation as resolved.
Show resolved Hide resolved
CHECK_EQ(get_common_substr("test/chan/label", "label/abcd/xyz"), "label");
CHECK_EQ(get_common_substr("test/chan/label", "chan/label/abcd"), "chan/label");
CHECK_EQ(get_common_substr("test/chan/label", "abcd/chan/label"), "chan/label");
CHECK_EQ(get_common_substr("test", "abcd"), "");
CHECK_EQ(get_common_substr("test", "abcd/xyz"), "");
CHECK_EQ(get_common_substr("test/xyz", "abcd/xyz"), "xyz");
CHECK_EQ(get_common_substr("test/xyz", "abcd/gef"), "");
CHECK_EQ(get_common_substr("abcd/test", "abcd/xyz"), "");
Hind-M marked this conversation as resolved.
Show resolved Hide resolved
}
}

} // namespace mamba
3 changes: 2 additions & 1 deletion mamba/mamba/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ def get_base_url(url, name=None):
tmp = url.rsplit("/", 1)[0]
if name:
if tmp.endswith(name):
return tmp.rsplit("/", 1)[0]
# Return base url stripped from name
return tmp[: -(len(name) + 1)]
return tmp

api_ctx.channel_alias = str(
Expand Down