modules: Report module solver problems

include/libdnf5/base/goal_elements.hpp

@@ -107,7 +107,14 @@ enum class GoalProblem : uint32_t {
     SOLVER_PROBLEM_STRICT_RESOLVEMENT = (1 << 13),
     WRITE_DEBUG = (1 << 14),
     UNSUPPORTED_ACTION = (1 << 15),
-    MULTIPLE_STREAMS = (1 << 16)
+    /// Multiple streams of the same module match the enable request
+    MULTIPLE_STREAMS = (1 << 16),
+    /// Error in module defaults detected during resolvement of module dependencies
+    MODULE_SOLVER_ERROR_DEFAULTS = (1 << 17),
+    /// Problem with latest modules during resolvement of module dependencies
+    MODULE_SOLVER_ERROR_LATEST = (1 << 18),
+    /// Error detected during resolvement of module dependencies
+    MODULE_SOLVER_ERROR = (1 << 19)
 };
 
 /// Types of Goal actions

include/libdnf5/module/module_sack.hpp

@@ -64,22 +64,6 @@ enum class ModuleStatus { AVAILABLE, ENABLED, DISABLED };
 /// Container with data and methods related to modules
 class ModuleSack {
 public:
-    enum class ModuleErrorType {
-        NO_ERROR = 0,
-        INFO,
-        /// Error in module defaults detected during resolvement of module dependencies
-        ERROR_IN_DEFAULTS,
-        /// Error detected during resolvement of module dependencies
-        ERROR,
-        /// Error detected during resolvement of module dependencies - Unexpected error!!!
-        CANNOT_RESOLVE_MODULES,
-        CANNOT_RESOLVE_MODULE_SPEC,
-        CANNOT_ENABLE_MULTIPLE_STREAMS,
-        CANNOT_MODIFY_MULTIPLE_TIMES_MODULE_STATUS,
-        /// Problem with latest modules during resolvement of module dependencies
-        ERROR_IN_LATEST
-    };
-
     ~ModuleSack();
 
     ModuleSackWeakPtr get_weak_ptr();
@@ -104,7 +88,7 @@ class ModuleSack {
     ///
     /// @return A pair of problems in resolving to report and ModuleErrorType.
     /// @since 5.0
-    std::pair<base::SolverProblems, ModuleErrorType> resolve_active_module_items();
+    std::pair<base::SolverProblems, GoalProblem> resolve_active_module_items();
 
 private:
     friend class libdnf5::Base;

libdnf5/base/goal.cpp

@@ -29,6 +29,7 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "rpm/solv/goal_private.hpp"
 #include "solv/id_queue.hpp"
 #include "solv/pool.hpp"
+#include "solver_problems_internal.hpp"
 #include "transaction/transaction_sr.hpp"
 #include "transaction_impl.hpp"
 #include "utils/string.hpp"
@@ -2316,15 +2317,31 @@ base::Transaction Goal::resolve() {
     sack->p_impl->make_provides_ready();
 
 
-    // TODO(jmracek) Apply modules first
     module::ModuleSack & module_sack = p_impl->base->module_sack;
     ret |= p_impl->add_module_specs_to_goal(transaction);
     // Resolve modules
-    auto module_error = module_sack.resolve_active_module_items().second;
-    if (module_error != libdnf5::module::ModuleSack::ModuleErrorType::NO_ERROR) {
-        throw module::ModuleResolveError(M_("Failed to resolve modules."));
+    auto result = module_sack.resolve_active_module_items();
+    auto module_solver_problems = result.first;
+    auto module_error = result.second;
+
+    // Report problems from the module solver
+    if (module_error != GoalProblem::NO_PROBLEM && !module_solver_problems.get_problems().empty()) {
+        transaction.p_impl->add_resolve_log(module_error, module_solver_problems);
+    }
+
+    // Return immediately if there is a fatal error in module resolvement or if there is an error
+    // in latest and best is true, don't even set the transaction, only store the problems.
+    if (module_error == GoalProblem::MODULE_SOLVER_ERROR ||
+        (module_error == GoalProblem::MODULE_SOLVER_ERROR_LATEST &&
+         p_impl->base->get_config().get_best_option().get_value())) {
+        ret |= module_error;
+        transaction.p_impl->problems = ret;
+        return transaction;
     }
+
     module_sack.p_impl->enable_dependent_modules();
+
+
     // TODO(jmracek) Apply comps second or later
     // TODO(jmracek) Reset rpm_goal, setup rpm-goal flags according to conf, (allow downgrade), obsoletes, vendor, ...
     ret |= p_impl->add_specs_to_goal(transaction);

libdnf5/base/log_event.cpp

@@ -80,19 +80,29 @@ LogEvent::LogEvent(
     : p_impl(std::make_unique<Impl>(action, problem, additional_data, settings, spec_type, spec)) {
     libdnf_assert(
         !(problem == libdnf5::GoalProblem::SOLVER_ERROR ||
-          problem == libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT),
-        "LogEvent::LogEvent() called with incorrect problem, the constructor does not allow"
-        "libdnf5::GoalProblem::SOLVER_ERROR or libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT. With those "
-        "problems it is necessary to provide SolverProblems in constructor");
+          problem == libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT ||
+          problem == libdnf5::GoalProblem::MODULE_SOLVER_ERROR ||
+          problem == libdnf5::GoalProblem::MODULE_SOLVER_ERROR_LATEST ||
+          problem == libdnf5::GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS),
+        "LogEvent::LogEvent() called with incorrect problem, the constructor does not allow these problems: "
+        "libdnf5::GoalProblem::SOLVER_ERROR, libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT, "
+        "libdnf5::GoalProblem::MODULE_SOLVER_ERROR, libdnf5::GoalProblem::MODULE_SOLVER_ERROR_LATEST, "
+        "libdnf5::GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS. "
+        "With those problems it is necessary to provide SolverProblems in constructor");
 }
 
 LogEvent::LogEvent(libdnf5::GoalProblem problem, const SolverProblems & solver_problems)
     : p_impl(std::make_unique<Impl>(libdnf5::GoalAction::RESOLVE, problem, solver_problems)) {
     libdnf_assert(
         problem == libdnf5::GoalProblem::SOLVER_ERROR ||
-            problem == libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT,
-        "LogEvent::LogEvent() called with incorrect problem, only libdnf5::GoalProblem::SOLVER_ERROR or "
-        "libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT is supported");
+            problem == libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT ||
+            problem == libdnf5::GoalProblem::MODULE_SOLVER_ERROR ||
+            problem == libdnf5::GoalProblem::MODULE_SOLVER_ERROR_LATEST ||
+            problem == libdnf5::GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS,
+        "LogEvent::LogEvent() called with incorrect problem, supported problems are only: "
+        "libdnf5::GoalProblem::SOLVER_ERROR, libdnf5::GoalProblem::SOLVER_PROBLEM_STRICT_RESOLVEMENT, "
+        "libdnf5::GoalProblem::MODULE_SOLVER_ERROR, libdnf5::GoalProblem::MODULE_SOLVER_ERROR_LATEST, "
+        "libdnf5::GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS.");
 }
 
 LogEvent::~LogEvent() = default;
@@ -236,6 +246,27 @@ std::string LogEvent::to_string(
             }
             return ret.append(utils::sformat(_("Unable to resolve argument '{}':{}"), *spec, error_message));
         }
+        case GoalProblem::MODULE_SOLVER_ERROR: {
+            if (!solver_problems) {
+                throw std::invalid_argument("Missing SolverProblems to convert event into string");
+            }
+            ret.append(utils::sformat(_("Modular dependency problems:\n")));
+            return ret.append(solver_problems->to_string());
+        }
+        case GoalProblem::MODULE_SOLVER_ERROR_LATEST: {
+            if (!solver_problems) {
+                throw std::invalid_argument("Missing SolverProblems to convert event into string");
+            }
+            ret.append(utils::sformat(_("Modular dependency problems with the latest modules:\n")));
+            return ret.append(solver_problems->to_string());
+        }
+        case GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS: {
+            if (!solver_problems) {
+                throw std::invalid_argument("Missing SolverProblems to convert event into string");
+            }
+            ret.append(utils::sformat(_("Modular dependency problems with the defaults:\n")));
+            return ret.append(solver_problems->to_string());
+        }
     }
     return ret;
 }

libdnf5/module/module_sack.cpp

@@ -463,12 +463,12 @@ void ModuleSack::Impl::enable_dependent_modules() {
 }
 
 
-std::pair<std::vector<std::vector<std::tuple<ProblemRules, Id, Id, Id, std::string>>>, ModuleSack::ModuleErrorType>
+std::pair<std::vector<std::vector<std::tuple<ProblemRules, Id, Id, Id, std::string>>>, GoalProblem>
 ModuleSack::Impl::module_solve(std::vector<ModuleItem *> module_items) {
     std::vector<std::vector<std::tuple<ProblemRules, Id, Id, Id, std::string>>> problems;
     if (module_items.empty()) {
         active_modules.clear();
-        return std::make_pair(problems, ModuleSack::ModuleErrorType::NO_ERROR);
+        return std::make_pair(problems, GoalProblem::NO_PROBLEM);
     }
 
     recompute_considered_in_pool();
@@ -519,7 +519,7 @@ ModuleSack::Impl::module_solve(std::vector<ModuleItem *> module_items) {
 
     if (ret == libdnf5::GoalProblem::NO_PROBLEM) {
         set_active_modules(goal_strict);
-        return make_pair(problems, ModuleSack::ModuleErrorType::NO_ERROR);
+        return make_pair(problems, GoalProblem::NO_PROBLEM);
     }
 
     problems = goal_strict.get_problems();
@@ -528,14 +528,14 @@ ModuleSack::Impl::module_solve(std::vector<ModuleItem *> module_items) {
 
     if (ret == libdnf5::GoalProblem::NO_PROBLEM) {
         set_active_modules(goal_best);
-        return make_pair(problems, ModuleSack::ModuleErrorType::ERROR_IN_DEFAULTS);
+        return make_pair(problems, GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS);
     }
 
     ret = goal.resolve();
 
     if (ret == libdnf5::GoalProblem::NO_PROBLEM) {
         set_active_modules(goal);
-        return make_pair(problems, ModuleSack::ModuleErrorType::ERROR_IN_LATEST);
+        return make_pair(problems, GoalProblem::MODULE_SOLVER_ERROR_LATEST);
     }
 
     // Conflicting modules has to be removed otherwise it could result than one of them will be active
@@ -547,14 +547,14 @@ ModuleSack::Impl::module_solve(std::vector<ModuleItem *> module_items) {
 
     if (ret == libdnf5::GoalProblem::NO_PROBLEM) {
         set_active_modules(goal_weak);
-        return make_pair(problems, ModuleSack::ModuleErrorType::ERROR);
+        return make_pair(problems, GoalProblem::MODULE_SOLVER_ERROR);
     }
 
     auto logger = base->get_logger();
     logger->critical("Modularity filtering totally broken\n");
 
     active_modules.clear();
-    return make_pair(problems, ModuleSack::ModuleErrorType::CANNOT_RESOLVE_MODULES);
+    return make_pair(problems, GoalProblem::MODULE_SOLVER_ERROR);
 }
 
 
@@ -727,7 +727,7 @@ std::optional<std::pair<std::string, std::string>> ModuleSack::Impl::detect_plat
 }
 
 
-std::pair<base::SolverProblems, ModuleSack::ModuleErrorType> ModuleSack::resolve_active_module_items() {
+std::pair<base::SolverProblems, GoalProblem> ModuleSack::resolve_active_module_items() {
     p_impl->considered_uptodate = false;
     p_impl->excludes.reset(new libdnf5::solv::SolvMap(p_impl->pool->nsolvables));
     p_impl->module_db->initialize();

libdnf5/module/module_sack_impl.hpp

@@ -114,7 +114,7 @@ class ModuleSack::Impl {
     /// @param module_items Module Items to resolve.
     /// @return `std::pair` of problems in resolving and ModuleErrorType.
     /// @since 5.0
-    std::pair<std::vector<std::vector<std::tuple<ProblemRules, Id, Id, Id, std::string>>>, ModuleSack::ModuleErrorType> module_solve(
+    std::pair<std::vector<std::vector<std::tuple<ProblemRules, Id, Id, Id, std::string>>>, GoalProblem> module_solve(
         std::vector<ModuleItem *> module_items);
 
     /// Enable module stream.
@@ -154,6 +154,7 @@ class ModuleSack::Impl {
 
 private:
     friend class libdnf5::base::Transaction;
+    friend class libdnf5::Goal;
     friend ModuleSack;
     friend ModuleItem;
     friend ModuleGoalPrivate;

test/libdnf5/module/test_module.cpp

@@ -29,6 +29,7 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "libdnf5/utils/fs/file.hpp"
 
 #include <libdnf5/base/goal.hpp>
+#include <libdnf5/base/goal_elements.hpp>
 #include <libdnf5/module/module_errors.hpp>
 #include <libdnf5/module/module_item.hpp>
 #include <libdnf5/module/module_query.hpp>
@@ -105,7 +106,7 @@ void ModuleTest::test_resolve() {
 
     auto module_sack = base.get_module_sack();
 
-    CPPUNIT_ASSERT_EQUAL(ModuleSack::ModuleErrorType::NO_ERROR, module_sack->resolve_active_module_items().second);
+    CPPUNIT_ASSERT_EQUAL(libdnf5::GoalProblem::NO_PROBLEM, module_sack->resolve_active_module_items().second);
 
     std::vector<std::string> expected_active_module_specs{
         "berries:main:4:6c81f848:x86_64", "gooseberry:5.5:2:72aaf46b6:x86_64", "gooseberry:5.5:3:72aaf46b6:x86_64"};
@@ -125,7 +126,7 @@ void ModuleTest::test_resolve_broken_defaults() {
     auto module_sack = base.get_module_sack();
 
     CPPUNIT_ASSERT_EQUAL(
-        ModuleSack::ModuleErrorType::ERROR_IN_DEFAULTS, module_sack->resolve_active_module_items().second);
+        libdnf5::GoalProblem::MODULE_SOLVER_ERROR_DEFAULTS, module_sack->resolve_active_module_items().second);
 
     std::vector<std::string> expected_active_module_specs{
         "berries:main:3:72aaf46b6:x86_64", "gooseberry:5.5:3:72aaf46b6:x86_64"};