Skip to content

Commit

Permalink
Parse subdirs in CLI match specs
Browse files Browse the repository at this point in the history 
Fixes #2792

For example, the channel in `install conda-forge/noarch::tzdata` would
previously be parsed as `conda-forge/noarch` while it should be parsed
as `conda-forge` with `noarch` subdir.
  • Loading branch information
@jonashaag
jonashaag committed Aug 29, 2023
1 parent e5abd42 commit 8f52bc5
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 99 deletions.
8 changes: 8 additions & 0 deletions libmamba/include/mamba/core/channel.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@

namespace mamba
{
// ATTENTION names with substrings need to go longer -> smalle
// otherwise linux-ppc64 matches for linux-ppc64le etc!
const std::vector<std::string> KNOWN_PLATFORMS = {
"noarch", "linux-32", "linux-64", "linux-aarch64", "linux-armv6l",
"linux-armv7l", "linux-ppc64le", "linux-ppc64", "osx-64", "osx-arm64",
"win-32", "win-64", "win-arm64", "zos-z"
};

// Note: Channels can only be created using ChannelContext.
class Channel
{
Expand Down
9 changes: 2 additions & 7 deletions libmamba/include/mamba/util/url_manip.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,8 @@ namespace mamba::util
bool with_credential
);

void split_platform(
const std::vector<std::string>& known_platforms,
const std::string& url,
const std::string& context_platform,
std::string& cleaned_url,
std::string& platform
);
std::pair<std::string, std::optional<std::string>>
split_platform(const std::string& url, const std::vector<std::string>& known_platforms);

/**
* If @p url starts with a scheme, return it, otherwise return empty string.
Expand Down
25 changes: 6 additions & 19 deletions libmamba/src/core/channel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ namespace mamba

const char LOCAL_CHANNELS_NAME[] = "local";
const char DEFAULT_CHANNELS_NAME[] = "defaults";

// ATTENTION names with substrings need to go longer -> smalle
// otherwise linux-ppc64 matches for linux-ppc64le etc!
const std::vector<std::string> KNOWN_PLATFORMS = {
"noarch", "linux-32", "linux-64", "linux-aarch64", "linux-armv6l",
"linux-armv7l", "linux-ppc64le", "linux-ppc64", "osx-64", "osx-arm64",
"win-32", "win-64", "win-arm64", "zos-z"
};
} // namespace

// Specific functions, used only in this file
Expand Down Expand Up @@ -231,19 +223,14 @@ namespace mamba
// that already contains the platform
else
{
std::string cleaned_url = "", platform = "";
util::split_platform(
KNOWN_PLATFORMS,
value,
Context::instance().platform,
cleaned_url,
platform
);
if (!platform.empty())
auto all_platforms = KNOWN_PLATFORMS;
all_platforms.insert(all_platforms.begin(), Context::instance().platform);
auto [cleaned_url, platform] = util::split_platform(value, all_platforms);
if (platform.has_value())
{
platforms.push_back(std::move(platform));
value = cleaned_url;
platforms.push_back(platform.value());
}
value = cleaned_url;
}
}

Expand Down
17 changes: 5 additions & 12 deletions libmamba/src/core/match_spec.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,17 +177,10 @@ namespace mamba
{
throw std::runtime_error("Parsing of channel / namespace / subdir failed.");
}
// TODO implement Channel, and parsing of the channel here!
// channel = subdir = channel_str;
// channel, subdir = _parse_channel(channel_str)
// if 'channel' in brackets:
// b_channel, b_subdir = _parse_channel(brackets.pop('channel'))
// if b_channel:
// channel = b_channel
// if b_subdir:
// subdir = b_subdir
// if 'subdir' in brackets:
// subdir = brackets.pop('subdir')

std::optional<std::string> splitted_subdir;
std::tie(channel, splitted_subdir) = util::split_platform(channel, KNOWN_PLATFORMS);
subdir = splitted_subdir.value_or(subdir);

// support faulty conda matchspecs such as `libblas=[build=*mkl]`, which is
// the repr of `libblas=*=*mkl`
Expand Down Expand Up @@ -279,7 +272,7 @@ namespace mamba
}
else if (k == "subdir")
{
subdir = v;
subdir = splitted_subdir.value_or(v);
}
else if (k == "url")
{
Expand Down
40 changes: 13 additions & 27 deletions libmamba/src/util/url_manip.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,13 @@ namespace mamba::util
}
}

void split_platform(
const std::vector<std::string>& known_platforms,
const std::string& url,
const std::string& context_platform,
std::string& cleaned_url,
std::string& platform
)
std::pair<std::string, std::optional<std::string>>
split_platform(const std::string& url, const std::vector<std::string>& known_platforms)
{
platform = "";
std::string cleaned_url = url;
std::optional<std::string> platform;

// Split platform
auto check_platform_position = [&url](std::size_t pos, const std::string& lplatform) -> bool
{
if (pos == std::string::npos)
Expand All @@ -167,31 +164,20 @@ namespace mamba::util

return true;
};

std::size_t pos = url.find(context_platform);
if (check_platform_position(pos, context_platform))
{
platform = context_platform;
}
else
std::size_t pos;
for (auto it = known_platforms.begin(); it != known_platforms.end(); ++it)
{
for (auto it = known_platforms.begin(); it != known_platforms.end(); ++it)
pos = url.find(*it);
if (check_platform_position(pos, *it))
{
pos = url.find(*it);
if (check_platform_position(pos, *it))
{
platform = *it;
break;
}
platform = *it;
cleaned_url.replace(pos - 1, platform.value().size() + 1, "");
break;
}
}

cleaned_url = url;
if (pos != std::string::npos)
{
cleaned_url.replace(pos - 1, platform.size() + 1, "");
}
cleaned_url = util::rstrip(cleaned_url, "/");
return { cleaned_url, platform };
}

auto url_get_scheme(std::string_view url) -> std::string_view
Expand Down
25 changes: 25 additions & 0 deletions libmamba/tests/src/core/test_cpp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,31 @@ namespace mamba
MatchSpec ms("numpy=1.20", channel_context);
CHECK_EQ(ms.str(), "numpy=1.20");
}

{
MatchSpec ms("conda-forge::tzdata", channel_context);
CHECK_EQ(ms.str(), "conda-forge::tzdata");
}
{
MatchSpec ms("conda-forge::noarch/tzdata", channel_context);
CHECK_EQ(ms.str(), "conda-forge::noarch/tzdata");
}
{
MatchSpec ms("pkgs/main::tzdata", channel_context);
CHECK_EQ(ms.str(), "pkgs/main::tzdata");
}
{
MatchSpec ms("pkgs/main/noarch::tzdata", channel_context);
CHECK_EQ(ms.str(), "pkgs/main/noarch::tzdata");
}
{
MatchSpec ms("conda-forge/noarch::tzdata[subdir=linux64]", channel_context);
CHECK_EQ(ms.str(), "conda-forge/noarch::tzdata");
}
{
MatchSpec ms("conda-forge::tzdata[subdir=linux64]", channel_context);
CHECK_EQ(ms.str(), "conda-forge/linux64::tzdata");
}
}

TEST_CASE("is_simple")
Expand Down
66 changes: 32 additions & 34 deletions libmamba/tests/src/util/test_url_manip.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,59 +76,57 @@ TEST_SUITE("util::url_manip")

TEST_CASE("split_platform")
{
std::string platform_found, cleaned_url;
split_platform(
{ "noarch", "linux-64" },
std::string cleaned_url;
std::optional<std::string> platform;

std::tie(cleaned_url, platform) = split_platform(
"https://mamba.com/linux-64/package.tar.bz2",
std::string(mamba::specs::build_platform_name()),
cleaned_url,
platform_found
{ std::string(mamba::specs::build_platform_name()), "noarch", "linux-64" }
);

CHECK_EQ(platform_found, "linux-64");
CHECK_EQ(platform, "linux-64");
CHECK_EQ(cleaned_url, "https://mamba.com/package.tar.bz2");

split_platform(
{ "noarch", "linux-64" },
std::tie(cleaned_url, platform) = split_platform(
"https://mamba.com/linux-64/noarch-package.tar.bz2",
std::string(mamba::specs::build_platform_name()),
cleaned_url,
platform_found
{ std::string(mamba::specs::build_platform_name()), "noarch", "linux-64" }
);
CHECK_EQ(platform_found, "linux-64");
CHECK_EQ(platform, "linux-64");
CHECK_EQ(cleaned_url, "https://mamba.com/noarch-package.tar.bz2");

split_platform(
{ "linux-64", "osx-arm64", "noarch" },
std::tie(cleaned_url, platform) = split_platform(
"https://mamba.com/noarch/kernel_linux-64-package.tar.bz2",
std::string(mamba::specs::build_platform_name()),
cleaned_url,
platform_found
{ std::string(mamba::specs::build_platform_name()), "linux-64", "osx-arm64", "noarch" }
);
CHECK_EQ(platform_found, "noarch");
CHECK_EQ(platform, "noarch");
CHECK_EQ(cleaned_url, "https://mamba.com/kernel_linux-64-package.tar.bz2");

split_platform(
{ "noarch", "linux-64" },
std::tie(cleaned_url, platform) = split_platform(
"https://mamba.com/linux-64",
std::string(mamba::specs::build_platform_name()),
cleaned_url,
platform_found
{ std::string(mamba::specs::build_platform_name()), "noarch", "linux-64" }
);

CHECK_EQ(platform_found, "linux-64");
CHECK_EQ(platform, "linux-64");
CHECK_EQ(cleaned_url, "https://mamba.com");

split_platform(
{ "noarch", "linux-64" },
std::tie(cleaned_url, platform) = split_platform(
"https://mamba.com/noarch",
std::string(mamba::specs::build_platform_name()),
cleaned_url,
platform_found
{ std::string(mamba::specs::build_platform_name()), "noarch", "linux-64" }
);

CHECK_EQ(platform_found, "noarch");
CHECK_EQ(platform, "noarch");
CHECK_EQ(cleaned_url, "https://mamba.com");

std::tie(cleaned_url, platform) = split_platform(
"https://conda.anaconda.org/conda-forge/noarch",
{ std::string(mamba::specs::build_platform_name()), "noarch", "linux-64" }
);
CHECK_EQ(platform, "noarch");
CHECK_EQ(cleaned_url, "https://conda.anaconda.org/conda-forge");

std::tie(cleaned_url, platform) = split_platform(
"https://conda.anaconda.org/pkgs/main/noarch",
{ std::string(mamba::specs::build_platform_name()), "noarch", "linux-64" }
);
CHECK_EQ(platform, "noarch");
CHECK_EQ(cleaned_url, "https://conda.anaconda.org/pkgs/main");
}

TEST_CASE("path_to_url")
Expand Down

0 comments on commit 8f52bc5

Please sign in to comment.