From 6e5485617e5f610778055573c31e13769ec801e6 Mon Sep 17 00:00:00 2001
From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Thu, 26 Aug 2021 16:44:37 +0100
Subject: [PATCH] core: Add information on which condition failed to the job
 done message

When a job is skipped, it's useful to know exactly which condition failed so
let's add this information to the error message.

To avoid having to dynamically generate a format string, we special case the
formatting of condition failed messages.
---
 src/core/job.c  | 54 +++++++++++++++++++++++++++++++++++++++----------
 src/core/unit.c | 19 +++++++++++++++++
 src/core/unit.h |  2 ++
 3 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/src/core/job.c b/src/core/job.c
index dd16a0b2804..6eb135785b1 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -705,19 +705,51 @@ static void job_emit_done_message(Unit *u, uint32_t job_id, JobType t, JobResult
 
         if (!console_only) {  /* Skip printing if output goes to the console, and job_print_status_message()
                                * will actually print something to the console. */
-
+                Condition *c;
                 const char *mid = job_done_mid(t, result);  /* mid may be NULL. log_unit_struct() will ignore it. */
-                const char *msg_fmt = strjoina("MESSAGE=", format);
 
-                DISABLE_WARNING_FORMAT_NONLITERAL;
-                log_unit_struct(u, job_done_messages[result].log_level,
-                                msg_fmt, ident,
-                                "JOB_ID=%" PRIu32, job_id,
-                                "JOB_TYPE=%s", job_type_to_string(t),
-                                "JOB_RESULT=%s", job_result_to_string(result),
-                                LOG_UNIT_INVOCATION_ID(u),
-                                mid);
-                REENABLE_WARNING;
+                c = t == JOB_START && result == JOB_DONE ? unit_find_failed_condition(u) : NULL;
+                if (c) {
+                        /* Special case units that were skipped because of a failed condition check so that
+                         * we can add more information to the message. */
+                        if (c->trigger)
+                                log_unit_struct(
+                                        u,
+                                        job_done_messages[result].log_level,
+                                        "MESSAGE=%s was skipped because all trigger condition checks failed.",
+                                        ident,
+                                        "JOB_ID=%" PRIu32, job_id,
+                                        "JOB_TYPE=%s", job_type_to_string(t),
+                                        "JOB_RESULT=%s", job_result_to_string(result),
+                                        LOG_UNIT_INVOCATION_ID(u),
+                                        mid);
+                        else
+                                log_unit_struct(
+                                        u,
+                                        job_done_messages[result].log_level,
+                                        "MESSAGE=%s was skipped because of a failed condition check (%s=%s%s).",
+                                        ident,
+                                        condition_type_to_string(c->type),
+                                        c->negate ? "!" : "",
+                                        c->parameter,
+                                        "JOB_ID=%" PRIu32, job_id,
+                                        "JOB_TYPE=%s", job_type_to_string(t),
+                                        "JOB_RESULT=%s", job_result_to_string(result),
+                                        LOG_UNIT_INVOCATION_ID(u),
+                                        mid);
+                } else {
+                        const char *msg_fmt = strjoina("MESSAGE=", format);
+
+                        DISABLE_WARNING_FORMAT_NONLITERAL;
+                        log_unit_struct(u, job_done_messages[result].log_level,
+                                        msg_fmt, ident,
+                                        "JOB_ID=%" PRIu32, job_id,
+                                        "JOB_TYPE=%s", job_type_to_string(t),
+                                        "JOB_RESULT=%s", job_result_to_string(result),
+                                        LOG_UNIT_INVOCATION_ID(u),
+                                        mid);
+                        REENABLE_WARNING;
+                }
         }
 
         if (do_console) {
diff --git a/src/core/unit.c b/src/core/unit.c
index 03cd28826b0..1a76abf8a16 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -5848,6 +5848,25 @@ int unit_thaw_vtable_common(Unit *u) {
         return unit_cgroup_freezer_action(u, FREEZER_THAW);
 }
 
+Condition *unit_find_failed_condition(Unit *u) {
+        Condition *c, *failed_trigger = NULL;
+        bool has_succeeded_trigger = false;
+
+        if (u->condition_result)
+                return NULL;
+
+        LIST_FOREACH(conditions, c, u->conditions)
+                if (c->trigger) {
+                        if (c->result == CONDITION_SUCCEEDED)
+                                 has_succeeded_trigger = true;
+                        else if (!failed_trigger)
+                                 failed_trigger = c;
+                } else if (c->result != CONDITION_SUCCEEDED)
+                        return c;
+
+        return failed_trigger && !has_succeeded_trigger ? failed_trigger : NULL;
+}
+
 static const char* const collect_mode_table[_COLLECT_MODE_MAX] = {
         [COLLECT_INACTIVE] = "inactive",
         [COLLECT_INACTIVE_OR_FAILED] = "inactive-or-failed",
diff --git a/src/core/unit.h b/src/core/unit.h
index b689f29f8fd..80d56aefc1c 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -984,6 +984,8 @@ void unit_thawed(Unit *u);
 int unit_freeze_vtable_common(Unit *u);
 int unit_thaw_vtable_common(Unit *u);
 
+Condition *unit_find_failed_condition(Unit *u);
+
 /* Macros which append UNIT= or USER_UNIT= to the message */
 
 #define log_unit_full_errno_zerook(unit, level, error, ...)             \