diff --git a/.github/workflows/brew.yml b/.github/workflows/brew.yml index 2a451fdec..faeaeee75 100644 --- a/.github/workflows/brew.yml +++ b/.github/workflows/brew.yml @@ -29,7 +29,7 @@ jobs: build_homebrew: name: Build on homebrew - runs-on: macos-13 + runs-on: macos-15 steps: - name: Checkout mamba repository diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake index c9e5d327d..8deff3603 100644 --- a/cmake/CompilerWarnings.cmake +++ b/cmake/CompilerWarnings.cmake @@ -97,8 +97,8 @@ function(mamba_target_add_compile_warnings target) -Wconversion # Warn on sign conversions -Wsign-conversion - # Warn if a null dereference is detected - -Wnull-dereference + # Warn if a null dereference is detected Deactivated because it produced too many false + # positive with ranges while we are not doing many pointer operations. -Wnull-dereference # Warn if float is implicit promoted to double -Wdouble-promotion # Warn on security issues around functions that format output (ie printf) diff --git a/libmamba/include/mamba/solver/solution.hpp b/libmamba/include/mamba/solver/solution.hpp index 005c74262..9ae5533fe 100644 --- a/libmamba/include/mamba/solver/solution.hpp +++ b/libmamba/include/mamba/solver/solution.hpp @@ -7,12 +7,12 @@ #ifndef MAMBA_CORE_SOLUTION_HPP #define MAMBA_CORE_SOLUTION_HPP +#include #include #include #include #include "mamba/specs/package_info.hpp" -#include "mamba/util/loop_control.hpp" #include "mamba/util/type_traits.hpp" namespace mamba::solver @@ -67,23 +67,40 @@ namespace mamba::solver using action_list = std::vector; action_list actions = {}; + + /** + * Return a view of all unique packages involved in the solution. + * + * The view is invalidated if @ref actions is modified. + */ + [[nodiscard]] auto packages() const; + [[nodiscard]] auto packages(); + + /** + * Return a view of all packages that need to be removed. + * + * The view is invalidated if @ref actions is modified. + */ + [[nodiscard]] auto packages_to_remove() const; + [[nodiscard]] auto packages_to_remove(); + + /** + * Return a view of all packages that need to be installed. + * + * The view is invalidated if @ref actions is modified. + */ + [[nodiscard]] auto packages_to_install() const; + [[nodiscard]] auto packages_to_install(); + + /** + * Return a view of all packages that are omitted. + * + * The view is invalidated if @ref actions is modified. + */ + [[nodiscard]] auto packages_to_omit() const; + [[nodiscard]] auto packages_to_omit(); }; - template - void for_each_to_remove(Iter first, Iter last, UnaryFunc&& func); - template - void for_each_to_remove(Range&& actions, UnaryFunc&& func); - - template - void for_each_to_install(Iter first, Iter last, UnaryFunc&& func); - template - void for_each_to_install(Range&& actions, UnaryFunc&& func); - - template - void for_each_to_omit(Iter first, Iter last, UnaryFunc&& func); - template - void for_each_to_omit(Range&& actions, UnaryFunc&& func); - /******************************** * Implementation of Solution * ********************************/ @@ -112,35 +129,27 @@ namespace mamba::solver action ); } - } - // TODO(C++20): Poor man's replacement to range filter transform - template - void for_each_to_remove(Iter first, Iter last, UnaryFunc&& func) - { - for (; first != last; ++first) + template + auto packages_to_remove_impl(Range& actions) { - if (auto* const ptr = detail::to_remove_ptr(*first)) - { - if constexpr (std::is_same_v) - { - if (func(*ptr) == util::LoopControl::Break) - { - break; - } - } - else - { - func(*ptr); - } - } + namespace views = std::ranges::views; + return actions // + | views::transform([](auto& a) { return detail::to_remove_ptr(a); }) + | views::filter([](const auto* ptr) { return ptr != nullptr; }) + | views::transform([](auto* ptr) -> decltype(auto) { return *ptr; }); } + } - template - void for_each_to_remove(Range&& actions, UnaryFunc&& func) + inline auto Solution::packages_to_remove() const { - return for_each_to_remove(actions.begin(), actions.end(), std::forward(func)); + return detail::packages_to_remove_impl(actions); + } + + inline auto Solution::packages_to_remove() + { + return detail::packages_to_remove_impl(actions); } namespace detail @@ -167,35 +176,27 @@ namespace mamba::solver action ); } - } - template - void for_each_to_install(Iter first, Iter last, UnaryFunc&& func) - { - for (; first != last; ++first) + template + auto packages_to_install_impl(Range& actions) { - if (auto* const ptr = detail::to_install_ptr(*first)) - { - if constexpr (std::is_same_v) - { - if (func(*ptr) == util::LoopControl::Break) - { - break; - } - } - else - { - func(*ptr); - } - } + namespace views = std::ranges::views; + return actions // + | views::transform([](auto& a) { return detail::to_install_ptr(a); }) + | views::filter([](const auto* ptr) { return ptr != nullptr; }) + | views::transform([](auto* ptr) -> decltype(auto) { return *ptr; }); } + } - // TODO(C++20): Poor man's replacement to range filter transform - template - void for_each_to_install(Range&& actions, UnaryFunc&& func) + inline auto Solution::packages_to_install() const { - return for_each_to_install(actions.begin(), actions.end(), std::forward(func)); + return detail::packages_to_install_impl(actions); + } + + inline auto Solution::packages_to_install() + { + return detail::packages_to_install_impl(actions); } namespace detail @@ -218,35 +219,72 @@ namespace mamba::solver action ); } + + template + auto packages_to_omit_impl(Range& actions) + { + namespace views = std::ranges::views; + return actions // + | views::transform([](auto& a) { return detail::to_omit_ptr(a); }) + | views::filter([](const auto* ptr) { return ptr != nullptr; }) + | views::transform([](auto* ptr) -> decltype(auto) { return *ptr; }); + } + } - // TODO(C++20): Poor man's replacement to range filter transform - template - void for_each_to_omit(Iter first, Iter last, UnaryFunc&& func) + inline auto Solution::packages_to_omit() const { - for (; first != last; ++first) + return detail::packages_to_omit_impl(actions); + } + + inline auto Solution::packages_to_omit() + { + return detail::packages_to_omit_impl(actions); + } + + namespace detail + { + template + constexpr auto package_unique_ptrs(Action& action) { - if (auto* const ptr = detail::to_omit_ptr(*first)) + auto out = std::array{ + to_omit_ptr(action), + to_install_ptr(action), + to_remove_ptr(action), + }; + for (std::size_t i = 1; i < out.size(); ++i) { - if constexpr (std::is_same_v) + for (std::size_t j = i + 1; j < out.size(); ++j) { - if (func(*ptr) == util::LoopControl::Break) + if (out[j] == out[i]) { - break; + out[j] = nullptr; } } - else - { - func(*ptr); - } } + return out; + } + + template + auto packages_impl(Range& actions) + { + namespace views = std::ranges::views; + return actions // + | views::transform([](auto& a) { return package_unique_ptrs(a); }) // + | views::join // + | views::filter([](const auto* ptr) { return ptr != nullptr; }) // + | views::transform([](auto* ptr) -> decltype(auto) { return *ptr; }); } } - template - void for_each_to_omit(Range&& actions, UnaryFunc&& func) + inline auto Solution::packages() const { - return for_each_to_omit(actions.begin(), actions.end(), std::forward(func)); + return detail::packages_impl(actions); + } + + inline auto Solution::packages() + { + return detail::packages_impl(actions); } } #endif diff --git a/libmamba/src/core/transaction.cpp b/libmamba/src/core/transaction.cpp index 1d249b774..111314a8b 100644 --- a/libmamba/src/core/transaction.cpp +++ b/libmamba/src/core/transaction.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -242,16 +243,13 @@ namespace mamba else { // The specs to install become all the dependencies of the non intstalled specs - for_each_to_omit( - m_solution.actions, - [&](const specs::PackageInfo& pkg) + for (const specs::PackageInfo& pkg : m_solution.packages_to_omit()) + { + for (const auto& dep : pkg.dependencies) { - for (const auto& dep : pkg.dependencies) - { - m_history_entry.update.push_back(dep); - } + m_history_entry.update.push_back(dep); } - ); + } } using Request = solver::Request; @@ -384,7 +382,7 @@ namespace mamba // to be URL like (i.e. explicit). Below is a loop to fix the channel of the linked // packages (fix applied to the unlinked packages to avoid potential bugs). Ideally, this // should be normalised when reading the data. - const auto fix_channel = [&](specs::PackageInfo& pkg) + for (specs::PackageInfo& pkg : m_solution.packages()) { auto unresolved_pkg_channel = mamba::specs::UnresolvedChannel::parse(pkg.channel).value(); auto pkg_channel = mamba::specs::Channel::resolve( @@ -395,32 +393,15 @@ namespace mamba auto channel_url = pkg_channel[0].platform_url(pkg.platform).str(); pkg.channel = channel_url; }; - for_each_to_install(m_solution.actions, fix_channel); - for_each_to_remove(m_solution.actions, fix_channel); - for_each_to_omit(m_solution.actions, fix_channel); TransactionRollback rollback; TransactionContext transaction_context(ctx.transaction_params(), m_py_versions, m_requested_specs); - const auto link = [&](const specs::PackageInfo& pkg) + for (const specs::PackageInfo& pkg : m_solution.packages_to_remove()) { if (is_sig_interrupted()) { - return util::LoopControl::Break; - } - Console::stream() << "Linking " << pkg.str(); - const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg, false)); - LinkPackage lp(pkg, cache_path, &transaction_context); - lp.execute(); - rollback.record(lp); - m_history_entry.link_dists.push_back(pkg.long_str()); - return util::LoopControl::Continue; - }; - const auto unlink = [&](const specs::PackageInfo& pkg) - { - if (is_sig_interrupted()) - { - return util::LoopControl::Break; + break; } Console::stream() << "Unlinking " << pkg.str(); const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg)); @@ -428,11 +409,21 @@ namespace mamba up.execute(); rollback.record(up); m_history_entry.unlink_dists.push_back(pkg.long_str()); - return util::LoopControl::Continue; - }; + } - for_each_to_remove(m_solution.actions, unlink); - for_each_to_install(m_solution.actions, link); + for (const specs::PackageInfo& pkg : m_solution.packages_to_install()) + { + if (is_sig_interrupted()) + { + break; + } + Console::stream() << "Linking " << pkg.str(); + const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg, false)); + LinkPackage lp(pkg, cache_path, &transaction_context); + lp.execute(); + rollback.record(lp); + m_history_entry.link_dists.push_back(pkg.long_str()); + } if (is_sig_interrupted()) { @@ -451,25 +442,29 @@ namespace mamba auto MTransaction::to_conda() -> to_conda_type { - to_remove_type to_remove_structured = {}; - to_remove_structured.reserve(m_solution.actions.size()); // Upper bound - for_each_to_remove( - m_solution.actions, - [&](const auto& pkg) - { - to_remove_structured.emplace_back(pkg.channel, pkg.filename); // - } - ); + namespace views = std::ranges::views; - to_install_type to_install_structured = {}; - to_install_structured.reserve(m_solution.actions.size()); // Upper bound - for_each_to_install( - m_solution.actions, - [&](const auto& pkg) - { - to_install_structured.emplace_back(pkg.channel, pkg.filename, nl::json(pkg).dump(4)); // - } - ); + auto to_remove_range = m_solution.packages_to_remove() // + | views::transform( + [](const auto& pkg) + { return to_remove_type::value_type(pkg.channel, pkg.filename); } + ); + // TODO(C++23): std::ranges::to + auto to_remove_structured = to_remove_type(to_remove_range.begin(), to_remove_range.end()); + + auto to_install_range = m_solution.packages_to_install() // + | views::transform( + [](const auto& pkg) + { + return to_install_type::value_type( + pkg.channel, + pkg.filename, + nl::json(pkg).dump(4) + ); + } + ); + // TODO(C++23): std::ranges::to + auto to_install_structured = to_install_type(to_install_range.begin(), to_install_range.end()); to_specs_type specs; std::get<0>(specs) = m_history_entry.update; @@ -480,27 +475,22 @@ namespace mamba void MTransaction::log_json() { - std::vector to_fetch, to_link, to_unlink; + namespace views = std::ranges::views; - for_each_to_install( - m_solution.actions, - [&](const auto& pkg) - { - if (need_pkg_download(pkg, m_multi_cache)) - { - to_fetch.push_back(nl::json(pkg)); - } - to_link.push_back(nl::json(pkg)); - } - ); + // TODO(C++23): std::ranges::to + auto to_fetch_range = m_solution.packages_to_install() + | views::filter([this](const auto& pkg) + { return need_pkg_download(pkg, m_multi_cache); }); + auto to_fetch = std::vector(to_fetch_range.begin(), to_fetch_range.end()); - for_each_to_remove( - m_solution.actions, - [&](const auto& pkg) - { - to_unlink.push_back(nl::json(pkg)); // - } - ); + + // TODO(C++23): std::ranges::to + auto to_link_range = m_solution.packages_to_install(); + auto to_link = std::vector(to_link_range.begin(), to_link_range.end()); + + // TODO(C++23): std::ranges::to + auto to_unlink_range = m_solution.packages_to_remove(); + auto to_unlink = std::vector(to_unlink_range.begin(), to_unlink_range.end()); auto add_json = [](const auto& jlist, const char* s) { @@ -540,67 +530,59 @@ namespace mamba { LOG_INFO << "Content trust is enabled, package(s) signatures will be verified"; } - for_each_to_install( - solution.actions, - [&](const auto& pkg) + for (const auto& pkg : solution.packages_to_install()) + { + if (ctx.validation_params.verify_artifacts) { - if (ctx.validation_params.verify_artifacts) + LOG_INFO << "Creating RepoChecker..."; + auto repo_checker_store = RepoCheckerStore::make(ctx, channel_context, multi_cache); + for (auto& chan : channel_context.make_channel(pkg.channel)) { - LOG_INFO << "Creating RepoChecker..."; - auto repo_checker_store = RepoCheckerStore::make( - ctx, - channel_context, - multi_cache - ); - for (auto& chan : channel_context.make_channel(pkg.channel)) + auto repo_checker = repo_checker_store.find_checker(chan); + if (repo_checker) { - auto repo_checker = repo_checker_store.find_checker(chan); - if (repo_checker) - { - LOG_INFO << "RepoChecker successfully created."; - repo_checker->generate_index_checker(); - repo_checker->verify_package( - pkg.json_signable(), - std::string_view(pkg.signatures) - ); - } - else - { - LOG_ERROR << "Could not create a valid RepoChecker."; - throw std::runtime_error(fmt::format( - R"(Could not verify "{}". Please make sure the package signatures are available and 'trusted-channels' are configured correctly. Alternatively, try downloading without '--verify-artifacts' flag.)", - pkg.name - )); - } - } - LOG_INFO << "'" << pkg.name << "' trusted from '" << pkg.channel << "'"; - } - - // FIXME: only do this for micromamba for now - if (ctx.command_params.is_mamba_exe) - { - using Credentials = typename specs::CondaURL::Credentials; - auto l_pkg = pkg; - { - auto channels = channel_context.make_channel(pkg.package_url); - assert(channels.size() == 1); // A URL can only resolve to one channel - l_pkg.package_url = channels.front().platform_urls().at(0).str( - Credentials::Show + LOG_INFO << "RepoChecker successfully created."; + repo_checker->generate_index_checker(); + repo_checker->verify_package( + pkg.json_signable(), + std::string_view(pkg.signatures) ); } + else { - auto channels = channel_context.make_channel(pkg.channel); - assert(channels.size() == 1); // A URL can only resolve to one channel - l_pkg.channel = channels.front().id(); + LOG_ERROR << "Could not create a valid RepoChecker."; + throw std::runtime_error(fmt::format( + R"(Could not verify "{}". Please make sure the package signatures are available and 'trusted-channels' are configured correctly. Alternatively, try downloading without '--verify-artifacts' flag.)", + pkg.name + )); } - fetchers.emplace_back(l_pkg, multi_cache); - } - else - { - fetchers.emplace_back(pkg, multi_cache); } + LOG_INFO << "'" << pkg.name << "' trusted from '" << pkg.channel << "'"; } - ); + + // FIXME: only do this for micromamba for now + if (ctx.command_params.is_mamba_exe) + { + using Credentials = typename specs::CondaURL::Credentials; + auto l_pkg = pkg; + { + auto channels = channel_context.make_channel(pkg.package_url); + assert(channels.size() == 1); // A URL can only resolve to one channel + l_pkg.package_url = channels.front().platform_urls().at(0).str(Credentials::Show + ); + } + { + auto channels = channel_context.make_channel(pkg.channel); + assert(channels.size() == 1); // A URL can only resolve to one channel + l_pkg.channel = channels.front().id(); + } + fetchers.emplace_back(l_pkg, multi_cache); + } + else + { + fetchers.emplace_back(pkg, multi_cache); + } + } if (ctx.validation_params.verify_artifacts) { diff --git a/libmamba/src/solver/helpers.cpp b/libmamba/src/solver/helpers.cpp index 9a2d826cf..e1b7457fb 100644 --- a/libmamba/src/solver/helpers.cpp +++ b/libmamba/src/solver/helpers.cpp @@ -4,6 +4,8 @@ // // The full license is in the file LICENSE, distributed with this software. +#include + #include "solver/helpers.hpp" namespace mamba::solver @@ -11,20 +13,13 @@ namespace mamba::solver auto find_new_python_in_solution(const Solution& solution) -> std::optional> { - auto out = std::optional>{}; - for_each_to_install( - solution.actions, - [&](const auto& pkg) - { - if (pkg.name == "python") - { - out = std::cref(pkg); - return util::LoopControl::Break; - } - return util::LoopControl::Continue; - } - ); - return out; + auto packages = solution.packages_to_install(); + auto it = std::ranges::find_if(packages, [](const auto& pkg) { return pkg.name == "python"; }); + if (it != packages.end()) + { + return std::cref(*it); + } + return std::nullopt; } auto python_binary_compatible(const specs::Version& older, const specs::Version& newer) -> bool diff --git a/libmamba/tests/src/solver/test_solution.cpp b/libmamba/tests/src/solver/test_solution.cpp index 1e5117098..1031295b8 100644 --- a/libmamba/tests/src/solver/test_solution.cpp +++ b/libmamba/tests/src/solver/test_solution.cpp @@ -29,80 +29,108 @@ namespace Solution::Install{ PackageInfo("install") }, } }; - SECTION("Iterate over packages") + constexpr auto as_const = [](const auto& x) -> decltype(auto) { return x; }; + + SECTION("Const iterate over packages") { - auto remove_count = std::size_t(0); - for_each_to_remove( - solution.actions, - [&](const PackageInfo& pkg) + SECTION("Packages to remove") + { + auto remove_count = std::size_t(0); + for (const PackageInfo& pkg : as_const(solution).packages_to_remove()) { remove_count++; const auto has_remove = util::ends_with(pkg.name, "remove") || (pkg.name == "reinstall"); REQUIRE(has_remove); } - ); - REQUIRE(remove_count == 5); + REQUIRE(remove_count == 5); + } - auto install_count = std::size_t(0); - for_each_to_install( - solution.actions, - [&](const PackageInfo& pkg) + SECTION("Packages to install") + { + auto install_count = std::size_t(0); + for (const PackageInfo& pkg : as_const(solution).packages_to_install()) { install_count++; const auto has_install = util::ends_with(pkg.name, "install") || (pkg.name == "reinstall"); REQUIRE(has_install); } - ); - REQUIRE(install_count == 5); + REQUIRE(install_count == 5); + } - auto omit_count = std::size_t(0); - for_each_to_omit( - solution.actions, - [&](const PackageInfo& pkg) + SECTION("Packages to omit") + { + auto omit_count = std::size_t(0); + for (const PackageInfo& pkg : as_const(solution).packages_to_omit()) { omit_count++; REQUIRE(util::ends_with(pkg.name, "omit")); } - ); - REQUIRE(omit_count == 1); + REQUIRE(omit_count == 1); + } + + SECTION("All packages") + { + auto count = std::size_t(0); + for (const PackageInfo& pkg : as_const(solution).packages()) + { + count++; + REQUIRE(!pkg.name.empty()); + } + REQUIRE(count == 10); + } } - SECTION("Iterate over packages and break") + SECTION("Ref iterate over packages") { - auto remove_count = std::size_t(0); - for_each_to_remove( - solution.actions, - [&](const PackageInfo&) + SECTION("Packages to remove") + { + for (PackageInfo& pkg : solution.packages_to_remove()) { - remove_count++; - return util::LoopControl::Break; + pkg.name = ""; } - ); - REQUIRE(remove_count == 1); + for (const PackageInfo& pkg : solution.packages_to_remove()) + { + CHECK(pkg.name == ""); + } + } - auto install_count = std::size_t(0); - for_each_to_install( - solution.actions, - [&](const PackageInfo&) + SECTION("Packages to install") + { + for (PackageInfo& pkg : solution.packages_to_install()) { - install_count++; - return util::LoopControl::Break; + pkg.name = ""; } - ); - REQUIRE(install_count == 1); + for (const PackageInfo& pkg : solution.packages_to_install()) + { + CHECK(pkg.name == ""); + } + } - auto omit_count = std::size_t(0); - for_each_to_omit( - solution.actions, - [&](const PackageInfo&) + SECTION("Packages to omit") + { + for (PackageInfo& pkg : solution.packages_to_omit()) { - omit_count++; - return util::LoopControl::Break; + pkg.name = ""; } - ); - REQUIRE(omit_count == 1); + for (const PackageInfo& pkg : solution.packages_to_omit()) + { + CHECK(pkg.name == ""); + } + } + + SECTION("All packages") + { + for (PackageInfo& pkg : solution.packages()) + { + pkg.name = ""; + } + for (const PackageInfo& pkg : solution.packages()) + { + CHECK(pkg.name == ""); + } + } } } } diff --git a/libmambapy/src/libmambapy/bindings/solver.cpp b/libmambapy/src/libmambapy/bindings/solver.cpp index 7e7176dd3..6bc6510c3 100644 --- a/libmambapy/src/libmambapy/bindings/solver.cpp +++ b/libmambapy/src/libmambapy/bindings/solver.cpp @@ -301,37 +301,45 @@ namespace mambapy } )) .def_readwrite("actions", &Solution::actions) + .def( + "packages", + [](const Solution& solution) -> std::vector + { + // TODO(C++23): std::ranges::to + auto out = std::vector{}; + out.reserve(solution.actions.size()); // Lower bound + for (const auto& pkg : solution.packages()) + { + out.push_back(pkg); + } + return out; + } + ) .def( "to_install", [](const Solution& solution) -> std::vector { - auto out = std::vector{}; - out.reserve(solution.actions.size()); // Upper bound - for_each_to_install( - solution.actions, - [&](const auto& pkg) { out.push_back(pkg); } - ); - return out; + // TODO(C++23): std::ranges::to + auto range = solution.packages_to_install(); + return { range.begin(), range.end() }; } ) .def( "to_remove", [](const Solution& solution) -> std::vector { - auto out = std::vector{}; - out.reserve(solution.actions.size()); // Upper bound - for_each_to_remove(solution.actions, [&](const auto& pkg) { out.push_back(pkg); }); - return out; + // TODO(C++23): std::ranges::to + auto range = solution.packages_to_remove(); + return { range.begin(), range.end() }; } ) .def( "to_omit", [](const Solution& solution) -> std::vector { - auto out = std::vector{}; - out.reserve(solution.actions.size()); // Upper bound - for_each_to_omit(solution.actions, [&](const auto& pkg) { out.push_back(pkg); }); - return out; + // TODO(C++23): std::ranges::to + auto range = solution.packages_to_omit(); + return { range.begin(), range.end() }; } ) .def("__copy__", ©)