Revert "job: Don't mark as redundant if deps are relevant"

This reverts commit 097537f07a2fab3cb73aef7bc59f2a66aa93f533.

At least Fedora and Debian have already reverted this at the distro
level because it causes more problems than it solves. Arch is debating
reverting it as well [0] but would strongly prefer that this happens
upstream first. Fixes #15188.

[0] https://bugs.archlinux.org/task/66458

diff --git a/src/core/job.c b/src/core/job.c
index d518ac896936971ccf9b6db4c18096b5d3aedda3..ff49136d70350f65f17fc1f22de79aa36be24ca7 100644 (file)
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -387,62 +387,25 @@ JobType job_type_lookup_merge(JobType a, JobType b) {
         return job_merging_table[(a - 1) * a / 2 + b];
 }
 
-bool job_later_link_matters(Job *j, JobType type, unsigned generation) {
-        JobDependency *l;
-
-        assert(j);
-
-        j->generation = generation;
-
-        LIST_FOREACH(subject, l, j->subject_list) {
-                UnitActiveState state = _UNIT_ACTIVE_STATE_INVALID;
-
-                /* Have we seen this before? */
-                if (l->object->generation == generation)
-                        continue;
-
-                state = unit_active_state(l->object->unit);
-                switch (type) {
-
-                case JOB_START:
-                        return IN_SET(state, UNIT_INACTIVE, UNIT_FAILED) ||
-                               job_later_link_matters(l->object, type, generation);
-
-                case JOB_STOP:
-                        return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING) ||
-                               job_later_link_matters(l->object, type, generation);
-
-                default:
-                        assert_not_reached("Invalid job type");
-                }
-        }
-
-        return false;
-}
-
-bool job_is_redundant(Job *j, unsigned generation) {
-
-        assert(j);
-
-        UnitActiveState state = unit_active_state(j->unit);
-        switch (j->type) {
+bool job_type_is_redundant(JobType a, UnitActiveState b) {
+        switch (a) {
 
         case JOB_START:
-                return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING) && !job_later_link_matters(j, JOB_START, generation);
+                return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
 
         case JOB_STOP:
-                return IN_SET(state, UNIT_INACTIVE, UNIT_FAILED) && !job_later_link_matters(j, JOB_STOP, generation);
+                return IN_SET(b, UNIT_INACTIVE, UNIT_FAILED);
 
         case JOB_VERIFY_ACTIVE:
-                return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING);
+                return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
 
         case JOB_RELOAD:
                 return
-                        state == UNIT_RELOADING;
+                        b == UNIT_RELOADING;
 
         case JOB_RESTART:
                 return
-                        state == UNIT_ACTIVATING;
+                        b == UNIT_ACTIVATING;
 
         case JOB_NOP:
                 return true;

diff --git a/src/core/job.h b/src/core/job.h
index 02b057ee0641709d7c26c3a3ea331628c4ed3536..03ad640618e7bf047f9139a2d9df59fa4134dd76 100644 (file)
--- a/src/core/job.h
+++ b/src/core/job.h
@@ -196,8 +196,7 @@ _pure_ static inline bool job_type_is_superset(JobType a, JobType b) {
         return a == job_type_lookup_merge(a, b);
 }
 
-bool job_later_link_matters(Job *j, JobType type, unsigned generation);
-bool job_is_redundant(Job *j, unsigned generation);
+bool job_type_is_redundant(JobType a, UnitActiveState b) _pure_;
 
 /* Collapses a state-dependent job type into a simpler type by observing
  * the state of the unit which it is going to be applied to. */

diff --git a/src/core/transaction.c b/src/core/transaction.c
index 0fb52372f16a02561e2fe7a93e4975f17856a5f6..ef6470656ce22ec5a242a6ee2adf8c1862d8b7fe 100644 (file)
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -279,7 +279,7 @@ static int transaction_merge_jobs(Transaction *tr, sd_bus_error *e) {
         return 0;
 }
 
-static void transaction_drop_redundant(Transaction *tr, unsigned generation) {
+static void transaction_drop_redundant(Transaction *tr) {
         bool again;
 
         /* Goes through the transaction and removes all jobs of the units whose jobs are all noops. If not
@@ -299,7 +299,7 @@ static void transaction_drop_redundant(Transaction *tr, unsigned generation) {
 
                         LIST_FOREACH(transaction, k, j)
                                 if (tr->anchor_job == k ||
-                                    !job_is_redundant(k, generation) ||
+                                    !job_type_is_redundant(k->type, unit_active_state(k->unit)) ||
                                     (k->unit->job && job_type_is_conflicting(k->type, k->unit->job->type))) {
                                         keep = true;
                                         break;
@@ -735,7 +735,7 @@ int transaction_activate(
                 transaction_minimize_impact(tr);
 
         /* Third step: Drop redundant jobs */
-        transaction_drop_redundant(tr, generation++);
+        transaction_drop_redundant(tr);
 
         for (;;) {
                 /* Fourth step: Let's remove unneeded jobs that might
@@ -777,7 +777,7 @@ int transaction_activate(
         }
 
         /* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */
-        transaction_drop_redundant(tr, generation++);
+        transaction_drop_redundant(tr);
 
         /* Ninth step: check whether we can actually apply this */
         r = transaction_is_destructive(tr, mode, e);