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.

Sign up for GitHub

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

feat: add canonical flag to list command #3777

Merged
merged 5 commits into from
Jan 31, 2025
Merged

feat: add canonical flag to list command #3777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af36985
add canonical subcommand
SandrineP Jan 21, 2025
b617d6e
add canonical test
SandrineP Jan 28, 2025
ab09b31
change options priority
SandrineP Jan 29, 2025
a517446
update test
SandrineP Jan 29, 2025
5339a38
edit warning
SandrineP Jan 31, 2025
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
31 changes: 24 additions & 7 deletions libmamba/src/api/list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,17 @@ namespace mamba
{
struct list_options
{
bool full_name;
bool no_pip;
bool reverse;
bool explicit_;
bool md5;
bool full_name = false;
bool no_pip = false;
bool reverse = false;
bool explicit_ = false;
bool md5 = false;
bool canonical = false;
};

struct formatted_pkg
{
std::string name, version, build, channel, url, md5;
std::string name, version, build, channel, url, md5, build_string, platform;
};

bool compare_alphabetically(const formatted_pkg& a, const formatted_pkg& b)
Expand Down Expand Up @@ -168,6 +169,7 @@ namespace mamba
obj["channel"] = get_formatted_channel(pkg_info, channels.front());
obj["base_url"] = get_base_url(pkg_info, channels.front());
obj["url"] = pkg_info.package_url;
obj["md5"] = pkg_info.md5;
obj["build_number"] = pkg_info.build_number;
obj["build_string"] = pkg_info.build_string;
obj["dist_name"] = pkg_info.str();
Expand Down Expand Up @@ -201,6 +203,8 @@ namespace mamba
formatted_pkgs.build = package.second.build_string;
formatted_pkgs.url = package.second.package_url;
formatted_pkgs.md5 = package.second.md5;
formatted_pkgs.build_string = package.second.build_string;
formatted_pkgs.platform = package.second.platform;
packages.push_back(formatted_pkgs);
}
}
Expand All @@ -209,9 +213,13 @@ namespace mamba
: compare_alphabetically;
std::sort(packages.begin(), packages.end(), comparator);

// format and print table
// format and print output
if (options.explicit_)
{
if (options.canonical)
{
LOG_WARNING << "Option --canonical ignored because of --explicit";
}
for (auto p : packages)
{
if (options.md5)
Expand All @@ -224,6 +232,14 @@ namespace mamba
}
}
}
else if (options.canonical)
{
for (auto p : packages)
{
std::cout << p.channel << "/" << p.platform << "::" << p.name << "-"
<< p.version << "-" << p.build_string << std::endl;
}
}
else
{
auto requested_specs = prefix_data.history().get_requested_specs_map();
Expand Down Expand Up @@ -268,6 +284,7 @@ namespace mamba
options.reverse = config.at("reverse").value<bool>();
options.explicit_ = config.at("explicit").value<bool>();
options.md5 = config.at("md5").value<bool>();
options.canonical = config.at("canonical").value<bool>();

auto channel_context = ChannelContext::make_conda_compatible(config.context());
detail::list_packages(config.context(), regex, channel_context, std::move(options));
Expand Down
12 changes: 6 additions & 6 deletions micromamba/src/list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ init_list_parser(CLI::App* subcom, Configuration& config)
);
subcom->add_flag("--md5", md5.get_cli_config<bool>(), md5.description());


// TODO: implement this in libmamba/list.cpp
/*auto& canonical = config.insert(Configurable("canonical", false)
.group("cli")
.description("Output canonical names of packages only."));
subcom->add_flag("-c,--canonical", canonical.get_cli_config<bool>(), canonical.description());*/
auto& canonical = config.insert(
Configurable("canonical", false)
.group("cli")
.description("Output canonical names of packages only. Ignored if --explicit.")
);
subcom->add_flag("-c,--canonical", canonical.get_cli_config<bool>(), canonical.description());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I just remembered that we can use excludes option in CLI11 to have the behavior we want:

Suggested change
subcom->add_flag("-c,--canonical", canonical.get_cli_config<bool>(), canonical.description());
subcom->add_flag("-c,--canonical", canonical.get_cli_config<bool>(), canonical.description())->excludes(explicit_flag);

explicit_flag being something like:
auto* explicit_flag = subcom->add_flag("--explicit", explicit_.get_cli_config<bool>(), explicit_.description());
This way, no need to add Ignored if --explicit. in the description.

Copy link
Collaborator Author

@SandrineP SandrineP Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to let the user know the behavior of the commands, so not sure about removing the message.
Should I use excludes but leave Ignored if --explicit. in the description ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think excludes automatically adds a note after the flag description in the --help text (saying that it's excluding some flag/option). But yes you can try it out and do whatever you think is user friendly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it and I don´t think that it is the behavior we want as the command doesn´t run when using excludes. I will keep the initial version.

}

void
Expand Down
24 changes: 19 additions & 5 deletions micromamba/tests/test_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,25 @@ def test_list_no_json(

@pytest.mark.parametrize("explicit_flag", ["", "--explicit"])
@pytest.mark.parametrize("md5_flag", ["", "--md5"])
@pytest.mark.parametrize("canonical_flag", ["", "-c", "--canonical"])
@pytest.mark.parametrize("env_selector", ["", "name", "prefix"])
@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
def test_list_explicit(
tmp_home, tmp_root_prefix, tmp_env_name, tmp_xtensor_env, env_selector, explicit_flag, md5_flag
def test_list_subcommands(
tmp_home,
tmp_root_prefix,
tmp_env_name,
tmp_xtensor_env,
env_selector,
explicit_flag,
md5_flag,
canonical_flag,
):
if env_selector == "prefix":
res = helpers.umamba_list("-p", tmp_xtensor_env, explicit_flag, md5_flag)
res = helpers.umamba_list("-p", tmp_xtensor_env, explicit_flag, md5_flag, canonical_flag)
elif env_selector == "name":
res = helpers.umamba_list("-n", tmp_env_name, explicit_flag, md5_flag)
res = helpers.umamba_list("-n", tmp_env_name, explicit_flag, md5_flag, canonical_flag)
else:
res = helpers.umamba_list(explicit_flag, md5_flag)
res = helpers.umamba_list(explicit_flag, md5_flag, canonical_flag)

outputs_list = res.strip().split("\n")[2:]
if explicit_flag == "--explicit":
Expand All @@ -101,6 +109,12 @@ def test_list_explicit(
assert "#" in output
else:
assert "#" not in output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a case where we have explicit and canonical?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is included in the if: if there is --explicit (with or without another flag), explicit should apply.
When running the test there is a case where there are both flags explicit and canonical and it passes.

else:
if canonical_flag == "--canonical":
items = ["conda-forge/", "::"]
for output in outputs_list:
assert all(i in output for i in items)
assert " " not in output


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can maybe combine with explicit tests (if it doesn't complicate things too much) and test the case where it should warn/abort (with excludes option)?

@pytest.mark.parametrize("quiet_flag", ["", "-q", "--quiet"])
Expand Down
Loading