diff --git a/dnf5/commands/module/module_enable.cpp b/dnf5/commands/module/module_enable.cpp index c9b023ae2..4edd90600 100644 --- a/dnf5/commands/module/module_enable.cpp +++ b/dnf5/commands/module/module_enable.cpp @@ -44,6 +44,7 @@ void ModuleEnableCommand::set_argument_parser() { keys->set_complete_hook_func([&ctx](const char * arg) { return match_specs(ctx, arg, false, true, true, false); }); cmd.register_positional_arg(keys); + auto skip_broken = std::make_unique(*this); auto skip_unavailable = std::make_unique(*this); } diff --git a/include/libdnf5/base/goal_elements.hpp b/include/libdnf5/base/goal_elements.hpp index abd52262d..54cd42959 100644 --- a/include/libdnf5/base/goal_elements.hpp +++ b/include/libdnf5/base/goal_elements.hpp @@ -81,7 +81,8 @@ enum class GoalProblem : uint32_t { ALREADY_INSTALLED = (1 << 12), SOLVER_PROBLEM_STRICT_RESOLVEMENT = (1 << 13), WRITE_DEBUG = (1 << 14), - UNSUPPORTED_ACTION = (1 << 15) + UNSUPPORTED_ACTION = (1 << 15), + MULTIPLE_STREAMS = (1 << 16) }; /// Types of Goal actions diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 28bd0a629..71011b53d 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -569,9 +569,25 @@ GoalProblem Goal::Impl::add_module_specs_to_goal(base::Transaction & transaction for (auto & [action, spec, settings] : module_specs) { try { switch (action) { - case GoalAction::ENABLE: - module_sack.p_impl->enable(spec); + case GoalAction::ENABLE: { + bool skip_broken = settings.resolve_skip_broken(base->get_config()); + auto log_level = skip_broken ? libdnf5::Logger::Level::WARNING : libdnf5::Logger::Level::ERROR; + const auto & enable_ret = module_sack.p_impl->enable(spec); + if (!enable_ret.second.empty()) { + transaction.p_impl->add_resolve_log( + action, + GoalProblem::MULTIPLE_STREAMS, + GoalJobSettings(), + libdnf5::transaction::TransactionItemType::MODULE, + spec, + enable_ret.second, + log_level); + if (!skip_broken) { + ret |= GoalProblem::MULTIPLE_STREAMS; + } + } break; + } case GoalAction::DISABLE: module_sack.p_impl->disable(spec); break; diff --git a/libdnf5/base/log_event.cpp b/libdnf5/base/log_event.cpp index 62273c4b3..04dbf09f1 100644 --- a/libdnf5/base/log_event.cpp +++ b/libdnf5/base/log_event.cpp @@ -163,6 +163,27 @@ std::string LogEvent::to_string( case GoalProblem::UNSUPPORTED_ACTION: return ret.append(utils::sformat( _("{} action for argument \"{}\" is not supported."), goal_action_to_string(action), *spec)); + case GoalProblem::MULTIPLE_STREAMS: { + // Create module dict { name : { stream } } out of the additional data. + std::map> module_dict; + for (const auto & module_stream : additional_data) { + const auto pos = module_stream.find(":"); + module_dict[module_stream.substr(0, pos)].insert(module_stream.substr(pos + 1)); + } + // Create the error message describing all the streams of modules that were matched. + std::string error_message; + for (const auto & module_dict_iter : module_dict) { + error_message.append(utils::sformat( + _("\n - Argument '{}' matches {} streams ('{}') of module '{}', but none of the streams are " + "enabled or " + "default."), + *spec, + module_dict_iter.second.size(), + utils::string::join(module_dict_iter.second, "', '"), + module_dict_iter.first)); + } + return ret.append(utils::sformat(_("Unable to resolve argument '{}':{}"), *spec, error_message)); + } } return ret; } diff --git a/libdnf5/module/module_sack.cpp b/libdnf5/module/module_sack.cpp index 2176a2944..adfcd86ba 100644 --- a/libdnf5/module/module_sack.cpp +++ b/libdnf5/module/module_sack.cpp @@ -777,18 +777,96 @@ bool ModuleSack::Impl::enable(const std::string & name, const std::string & stre } -bool ModuleSack::Impl::enable(const std::string & module_spec, bool count) { +// module dict { name : { stream : [solvable id] } } +static std::unordered_map> create_module_dict( + const ModuleQuery & module_query) { + std::unordered_map> module_dict; + for (const auto & module_item : module_query.list()) { + module_dict[module_item.get_name()][module_item.get_stream()].push_back(module_item.get_id().id); + } + return module_dict; +} + + +std::set ModuleSack::Impl::prune_module_dict( + std::unordered_map> & module_dict) { + // Vector of module names with multiple streams to enable + std::set multiple_stream_modules; + + for (auto & module_dict_iter : module_dict) { + auto name = module_dict_iter.first; + auto & stream_dict = module_dict_iter.second; + auto module_status = module_db->get_status(name); + + // Multiple streams match the requested spec + if (stream_dict.size() > 1) { + // Get stream that is either enabled (for ENABLED module), or default (otherwise) + std::string enabled_or_default_stream; + if (module_status == ModuleStatus::ENABLED) { + enabled_or_default_stream = module_db->get_enabled_stream(name); + } else { + enabled_or_default_stream = module_sack->get_default_stream(name); + } + + // Module doesn't have any enabled nor default stream + if (enabled_or_default_stream.empty()) { + for (const auto & stream_pair : stream_dict) { + multiple_stream_modules.insert(fmt::format("{}:{}", name, stream_pair.first)); + } + continue; + } + + // The enabled or default stream is not one of the possible streams + if (stream_dict.find(enabled_or_default_stream) == stream_dict.end()) { + for (const auto & stream_pair : stream_dict) { + multiple_stream_modules.insert(fmt::format("{}:{}", name, stream_pair.first)); + } + continue; + } + + // Remove all streams except for the enabled_or_default_stream + for (auto iter = stream_dict.begin(); iter != stream_dict.end();) { + if (iter->first != enabled_or_default_stream) { + stream_dict.erase(iter++); + } else { + ++iter; + } + } + } + } + return multiple_stream_modules; +} + + +std::pair> ModuleSack::Impl::enable(const std::string & module_spec, bool count) { module_db->initialize(); + // For the given module_spec, create a dict { name : {stream : [solvable id] } + auto module_dict = create_module_dict(module_spec_to_query(base, module_spec)); + // Keep only enabled or default streams if possible + auto multiple_stream_modules = prune_module_dict(module_dict); + + // If there are any modules with multiple streams to be enabled, return immediately the set of these + // module:stream strings. + if (!multiple_stream_modules.empty()) { + return std::make_pair(false, multiple_stream_modules); + } + bool changed = false; libdnf5::solv::IdQueue queue; - for (const auto & module_item : module_spec_to_query(base, module_spec)) { - queue.push_back(module_item.get_id().id); - changed |= enable(module_item.get_name(), module_item.get_stream(), count); + for (const auto & module_dict_iter : module_dict) { + std::string name = module_dict_iter.first; + for (const auto & stream_dict_iter : module_dict_iter.second) { + // Enable this stream + changed |= enable(name, stream_dict_iter.first, count); + // Create a queue of ids for the stream to be enabled, because it better matches user requirements + // than just "module(name:stream)" provides. (E.g. user might have requested specific context or version.) + queue += stream_dict_iter.second; + } } modules_to_enable.push_back(queue); - return changed; + return std::make_pair(changed, multiple_stream_modules); } diff --git a/libdnf5/module/module_sack_impl.hpp b/libdnf5/module/module_sack_impl.hpp index eb423f284..c292d6c47 100644 --- a/libdnf5/module/module_sack_impl.hpp +++ b/libdnf5/module/module_sack_impl.hpp @@ -129,11 +129,11 @@ class ModuleSack::Impl { /// Enable module stream. /// @param module_spec module to be enabled. /// @param count if `true`, count the change towards the limit of module status modifications. - /// @return `true` if requested change really triggers a change in the ModuleDB, `false` otherwise. - /// @throw EnableMultipleStreamsError in case of conflicting enable requests. + /// @return `true` if requested change realy triggers a change in the ModuleDB, `false` otherwise, + /// and a set of module:stream strings for modules with multiple streams and no enabled or default one. /// @throw NoModuleError if the module doesn't exist. /// @since 5.0.14 - bool enable(const std::string & module_spec, bool count = true); + std::pair> enable(const std::string & module_spec, bool count = true); /// Disable module. /// @param module_spec module to be disabled. /// @param count if `true`, count the change towards the limit of module status modifications. @@ -188,6 +188,12 @@ class ModuleSack::Impl { /// @return If platform id was detected, it returns a pair where the first item is the platform /// module name and second is the platform stream. Otherwise std::nullopt is returned. std::optional> detect_platform_name_and_stream() const; + + /// @brief Keep only one stream for each module. If more than one stream is originally there, keep only + // the enabled or default one if possible. + /// @return Set of module:stream strings for modules with multiple streams and no enabled or default one. + std::set prune_module_dict( + std::unordered_map> & module_dict); }; inline const std::vector> & ModuleSack::Impl::get_modules() {