Sfoglia il codice sorgente

Ctrl+C rolls back without prompting; fix the update-pair poison order that deadlocked statum

Follow-ups to the 0.1 round, both from live use:

- The interrupt prompt is gone: a Ctrl+C during a transaction now rolls
  the journaled transaction straight back, narrated by the rollback
  description. The interrupt_decided signal stays as the programmatic
  opt-out for embedders; the CLI connects nothing.
- The removal-poisoning rule was applied as a blanket prefix, so an
  update pair poisoned its provides FOREVER: a resource provided by a
  package being removed could never be satisfied again, even after its
  replacement installed in an earlier lot — updating statum together
  with astralis/inversion deadlocked every lot after the first
  ("could not be scheduled", with the explainer agreeing with the
  stale on-disk state, i.e. no reason at all). The earlier-lot
  replacement now DOMINATES the poison, and the non-poisoned disk
  state is the fallback; the deadlock explainer uses the same
  satisfier and gains a "going away" cause naming the removed
  provider and the poisoned resource.

usm-tests: 408 passed, 0 failed (update-pair lot-split chain, removed-
provider reason). MANIFEST 0.1 -> 0.1.1.
clanker 4 settimane fa
parent
commit
7115d8231d

+ 1 - 1
MANIFEST.usm

@@ -1,6 +1,6 @@
 {
   "name": "usm",
-  "version": "0.1",
+  "version": "0.1.1",
   "summary": "Universal Source Manifest",
   "licences": [ {"name": "GPLv3", "category": "libre", "text": "src/LICENSE"} ],
   "flags": ["ninjaStyleProgress"],

+ 0 - 21
src/cli/Confirm.vala

@@ -38,25 +38,4 @@ namespace Usm.Cli {
         return builder.str;
     }
 
-    /**
-     * Parses one interrupt-prompt reply; see
-     * {@link Usm.TransactionInterrupt.parse_reply}.
-     */
-    public bool parse_interrupt_reply(string? reply) {
-        return Usm.TransactionInterrupt.parse_reply(reply);
-    }
-
-    /**
-     * The {@link Usm.Transaction.InterruptDecision} hook: narrates where
-     * the Ctrl+C landed and asks whether to roll back or keep going.
-     */
-    public bool interrupt_decision_prompt(string subject, string phase_verb) {
-        printerr("\nInterrupt received while %s \"%s\".\n", phase_verb, subject);
-        stdout.printf("Roll back or keep going? [r]oll back / [c]ontinue (Enter = roll back): ");
-        stdout.flush();
-        var keep_going = parse_interrupt_reply(confirm_read_line());
-        printerr(keep_going ? "Continuing — the interrupted operation re-runs.\n"
-                            : "Rolling the transaction back.\n");
-        return keep_going;
-    }
 }

+ 0 - 1
src/cli/Downgrade.vala

@@ -97,7 +97,6 @@ private int downgrade_main(string[] args) {
             install_order = downgrade_order,
             state = state
         };
-        transaction.interrupt_decided.connect((sender, subject, phase_verb) => Usm.Cli.interrupt_decision_prompt(subject, phase_verb));
 
         // Planning before the prompt populates the rebuildDependants
         // entries so the summary shows the whole transaction

+ 0 - 1
src/cli/Install.vala

@@ -178,7 +178,6 @@ private int install_main(string[] args) {
             system_packages = resolution.system_packages,
             system_install_plan = system_install_plan
         };
-        transaction.interrupt_decided.connect((sender, subject, phase_verb) => Usm.Cli.interrupt_decision_prompt(subject, phase_verb));
 
         // Planning before the prompt populates the rebuildDependants
         // entries so the summary shows the whole transaction

+ 0 - 1
src/cli/Rebuild.vala

@@ -61,7 +61,6 @@ private int rebuild_main(string[] args) {
             resource_finder = new Usm.ResourceFinder(paths),
             state = state
         };
-        transaction.interrupt_decided.connect((sender, subject, phase_verb) => Usm.Cli.interrupt_decision_prompt(subject, phase_verb));
         transaction.progress_updated.connect(progress.on_transaction_progress);
         transaction.rebuild_package(target);
         progress.finish();

+ 0 - 1
src/cli/Remove.vala

@@ -82,7 +82,6 @@ private int remove_main(string[] args) {
             to_remove = to_remove,
             state = state
         };
-        transaction.interrupt_decided.connect((sender, subject, phase_verb) => Usm.Cli.interrupt_decision_prompt(subject, phase_verb));
 
         // Planning before the prompt lets the removal order (dependents
         // before providers, orphans last) be computed and shown

+ 0 - 1
src/cli/Update.vala

@@ -242,7 +242,6 @@ private int update_main(string[] args) {
             system_packages = resolution.system_packages,
             system_install_plan = system_install_plan
         };
-        transaction.interrupt_decided.connect((sender, subject, phase_verb) => Usm.Cli.interrupt_decision_prompt(subject, phase_verb));
 
         // Planning before the prompt populates the rebuildDependants
         // entries so the summary shows the whole transaction

+ 3 - 17
src/lib/Interrupt.vala

@@ -6,10 +6,9 @@ namespace Usm {
      * each per-package operation and whenever an operation fails. The
      * terminal's Ctrl+C reaches the whole foreground process group, so the
      * usual outcome is the running child (build, test) dying with the
-     * signal, the operation erroring, and the checkpoint resolving the
-     * interrupt: roll the journaled transaction back, or clear the flag
-     * and keep going (the failed operation re-runs; every operation is
-     * restartable).
+     * signal, the operation erroring, and the checkpoint rolling the
+     * journaled transaction straight back — narrated by the journal's
+     * rollback description, no prompt.
      *
      * The handler is installed only for the duration of a transaction —
      * {@link Transaction.run} and {@link Transaction.rebuild_package} call
@@ -44,18 +43,5 @@ namespace Usm {
             Posix.signal(Posix.Signal.INT, Posix.SIG_DFL);
             requested = false;
         }
-
-        /**
-         * Parses one interrupt-prompt reply: `c`/`continue` keep the
-         * transaction going; anything else — including EOF and an empty
-         * line — rolls back, the safe default.
-         */
-        public static bool parse_reply(string? reply) {
-            if(reply == null) {
-                return false;
-            }
-            var answer = reply.strip().down();
-            return answer == "c" || answer == "continue";
-        }
     }
 }

+ 61 - 32
src/lib/Transaction.vala

@@ -160,11 +160,11 @@ namespace Usm {
         public Vector<RebuildEntry> rebuilds { get; private set; default = new Vector<RebuildEntry>(); }
 
         /**
-         * Emitted when a Ctrl+C lands mid-transaction: handlers answer
-         * whether to keep going (true — the interrupted operation re-runs)
-         * or roll the journaled transaction back (false). With no handler
-         * connected the signal's default return is false — the rollback
-         * behaviour non-interactive contexts want.
+         * Emitted when a Ctrl+C lands mid-transaction: a true return
+         * keeps the transaction going (the interrupted operation
+         * re-runs), false rolls the journaled transaction back. No
+         * handler is connected by the CLI — a Ctrl+C always rolls back
+         * — leaving this as the programmatic opt-out for embedders.
          */
         public signal bool interrupt_decided(string subject, string phase_verb);
 
@@ -499,24 +499,31 @@ namespace Usm {
                             }
                         });
                         // Build-phase refs must be satisfied by resources
-                        // that SURVIVE the removal phase and are already
-                        // present or installed by an EARLIER lot: a
-                        // same-lot provide is not installed yet when this
-                        // package builds, so a build dependency on it defers
-                        // the package to the next lot
-                        PredicateDelegate<ResourceRef> buildtime_satisfied = d => !removed_by_plan(d) && (
-                            resource_finder.has_resource(d)
-                            || preinstalled_resources.any(r => d.satisfied_by(r))
-                            || installed_by_earlier_lots.any(r => d.satisfied_by(r)));
-                        // Manage-phase executables run at install time, when
-                        // everything this lot has already admitted (in
-                        // topological order) is installed — but nothing the
-                        // removal phase takes away
-                        PredicateDelegate<ResourceRef> installtime_satisfied = d => !removed_by_plan(d) && (
-                            resource_finder.has_resource(d)
-                            || preinstalled_resources.any(r => d.satisfied_by(r))
-                            || installed_by_earlier_lots.any(r => d.satisfied_by(r))
-                            || installed_by_this_lot.any(r => d.satisfied_by(r)));
+                        // that are in place when this package builds:
+                        // either installed by an EARLIER lot (the
+                        // replacement of an updated pair — this dominates,
+                        // the removal is already undone by then) or
+                        // present on the destination and NOT provided by
+                        // a package this transaction removes (those go
+                        // away before the first build, so the stale
+                        // on-disk state satisfies nothing). A same-lot
+                        // provide is not installed yet and defers the
+                        // package to the next lot.
+                        PredicateDelegate<ResourceRef> buildtime_satisfied = d =>
+                            installed_by_earlier_lots.any(r => d.satisfied_by(r))
+                            || (!removed_by_plan(d) && (
+                                resource_finder.has_resource(d)
+                                || preinstalled_resources.any(r => d.satisfied_by(r))));
+                        // Manage-phase executables run at install time,
+                        // when everything this lot has already admitted
+                        // (in topological order) is installed — the same
+                        // dominance and poison rules as the build phase
+                        PredicateDelegate<ResourceRef> installtime_satisfied = d =>
+                            installed_by_earlier_lots.any(r => d.satisfied_by(r))
+                            || installed_by_this_lot.any(r => d.satisfied_by(r))
+                            || (!removed_by_plan(d) && (
+                                resource_finder.has_resource(d)
+                                || preinstalled_resources.any(r => d.satisfied_by(r))));
                         if(manifest.dependencies.manage.is_satisfied(installtime_satisfied) &&
                                 manifest.dependencies.build.is_satisfied(buildtime_satisfied)) {
                             lot.add(package);
@@ -614,9 +621,24 @@ namespace Usm {
         private string describe_install_deadlock(Enumerable<CachedPackage> stuck, Set<ResourceRef> installed_by_earlier_lots, Set<CachedPackage> planned_install) {
             var stuck_sorted = stuck.sort((a, b) => a.package_name.collate(b.package_name)).to_vector();
 
-            PredicateDelegate<ResourceRef> satisfier = r => resource_finder.has_resource(r)
-                || preinstalled_resources.any(p => r.satisfied_by(p))
-                || installed_by_earlier_lots.any(p => r.satisfied_by(p));
+            // The same satisfier the scheduler used, poison included: a
+            // resource provided by a package this transaction removes is
+            // going away before the first build, so explaining a stall
+            // against the stale on-disk state would clear every blocker
+            // and produce a dead diagnosis
+            PredicateDelegate<ResourceRef> removed_by_plan = r => planned_removal.any(p => {
+                try {
+                    return p.get_manifest().satisfies(r);
+                }
+                catch(Error e) {
+                    return false;
+                }
+            });
+            PredicateDelegate<ResourceRef> satisfier = r =>
+                installed_by_earlier_lots.any(p => r.satisfied_by(p))
+                || (!removed_by_plan(r) && (
+                    resource_finder.has_resource(r)
+                    || preinstalled_resources.any(p => r.satisfied_by(p))));
 
             var builder = new StringBuilder();
             builder.append_printf("Could not build a transaction strategy: %u package(s) could not be scheduled for installation", stuck_sorted.length);
@@ -634,8 +656,8 @@ namespace Usm {
                     builder.append_printf("    (manifest could not be read: %s)\n", e.message);
                     continue;
                 }
-                append_blocking_refs(builder, manifest.dependencies.manage, "manage", satisfier, planned_install, package);
-                append_blocking_refs(builder, manifest.dependencies.build, "build", satisfier, planned_install, package);
+                append_blocking_refs(builder, manifest.dependencies.manage, "manage", satisfier, removed_by_plan, planned_install, package);
+                append_blocking_refs(builder, manifest.dependencies.build, "build", satisfier, removed_by_plan, planned_install, package);
             }
             return builder.str.chomp();
         }
@@ -645,14 +667,14 @@ namespace Usm {
          * provides. Flat phases list their refs directly; grouped phases
          * report every candidate group that is not viable.
          */
-        private void append_blocking_refs(StringBuilder builder, DependencyPhase phase, string phase_name, PredicateDelegate<ResourceRef> satisfier, Set<CachedPackage> planned_install, CachedPackage package) {
+        private void append_blocking_refs(StringBuilder builder, DependencyPhase phase, string phase_name, PredicateDelegate<ResourceRef> satisfier, PredicateDelegate<ResourceRef> removed_by_plan, Set<CachedPackage> planned_install, CachedPackage package) {
             if(phase == null) {
                 return;
             }
             if(!phase.is_grouped) {
                 foreach(var resource in phase.ordered_required()) {
                     if(!satisfier(resource)) {
-                        append_blocking_ref(builder, phase_name, resource, planned_install, package);
+                        append_blocking_ref(builder, phase_name, resource, removed_by_plan, planned_install, package);
                     }
                 }
                 return;
@@ -665,7 +687,7 @@ namespace Usm {
                 }
                 builder.append_printf("    [%s phase, candidate group %u of %u:]\n", phase_name, index + 1, groups.length);
                 foreach(var resource in blocking) {
-                    append_blocking_ref(builder, phase_name, resource, planned_install, package);
+                    append_blocking_ref(builder, phase_name, resource, removed_by_plan, planned_install, package);
                 }
             }
         }
@@ -675,7 +697,7 @@ namespace Usm {
          * transaction members provide it, their names — distinguishing
          * unmet dependencies from dependency cycles.
          */
-        private void append_blocking_ref(StringBuilder builder, string phase_name, ResourceRef resource, Set<CachedPackage> planned_install, CachedPackage package) {
+        private void append_blocking_ref(StringBuilder builder, string phase_name, ResourceRef resource, PredicateDelegate<ResourceRef> removed_by_plan, Set<CachedPackage> planned_install, CachedPackage package) {
             var providers = new Vector<string>();
             foreach(var candidate in planned_install) {
                 if(candidate == package) {
@@ -691,6 +713,13 @@ namespace Usm {
                 }
             }
             if(providers.length == 0) {
+                if(removed_by_plan(resource)) {
+                    builder.append_printf(
+                        "    %s (%s phase) is going away: a package this transaction removes currently provides it, and nothing in this transaction replaces it\n",
+                        resource.to_string(), phase_name
+                    );
+                    return;
+                }
                 builder.append_printf("    %s (%s phase) is unmet: it is not present on this system, no planned system package provides it, and no package in this transaction provides it\n", resource.to_string(), phase_name);
                 return;
             }

+ 81 - 18
src/tests/TestMain.vala

@@ -1457,6 +1457,85 @@ esac
         Usm.Util.delete_tree(scratch);
     }
 
+    /**
+     * An update pair chain — every package both removed (its old
+     * version) and installed (its new one) — must split at build
+     * boundaries exactly like a fresh chain: the removals run before
+     * any build, so a dependent's build-phase ref on a removed
+     * package's provide is satisfied only once the replacement
+     * installs (an earlier lot). Regression: statum/astralis/inversion
+     * updated in one transaction used to plan into one lot against
+     * the stale pre-removal disk state.
+     */
+    void test_transaction_update_pairs_split_at_build_boundaries() throws Error {
+        var scratch = make_scratch();
+        make_build_package_archive(scratch, "x", "pc:lib-upd-x.pc", {});
+        make_build_package_archive(scratch, "y", "pc:lib-upd-y.pc", { "pc:lib-upd-x.pc" });
+        make_build_package_archive(scratch, "z", "pc:lib-upd-z.pc", { "pc:lib-upd-y.pc" });
+
+        var to_install = new HashSet<Usm.CachedPackage>();
+        var to_remove = new HashSet<Usm.CachedPackage>();
+        foreach(var name in new string[] { "x", "y", "z" }) {
+            var cached = cache_package(scratch, name);
+            to_install.add(cached);
+            to_remove.add(cached);
+        }
+
+        var transaction = new Usm.Transaction() {
+            paths = new Usm.Paths(),
+            resource_finder = new Usm.ResourceFinder(),
+            to_install = to_install,
+            to_remove = to_remove
+        };
+        transaction.strategise();
+
+        check(transaction.install_lots.length == 3, "an update-pair chain splits into one lot per build level");
+        check(transaction.install_lots[0].to_string(p => p.package_name, ",") == "cache-pkg-x-1.0.0",
+            "the base of the update chain installs before its dependents build");
+        check(transaction.install_lots[1].to_string(p => p.package_name, ",") == "cache-pkg-y-1.0.0",
+            "a dependent builds against the freshly installed replacement, not the removed old version");
+        check(transaction.install_lots[2].to_string(p => p.package_name, ",") == "cache-pkg-z-1.0.0",
+            "the chain tip waits for the middle update pair");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
+    /**
+     * A build-phase ref whose only provider is being REMOVED without a
+     * replacement in the transaction deadlocks the strategy — and the
+     * thrown message must say so: the explainer evaluates the same
+     * poison-aware satisfier the scheduler used, never the stale
+     * on-disk state (which showed the ref as present and produced a
+     * dead, reasonless diagnosis).
+     */
+    void test_transaction_removed_provider_reason() throws Error {
+        var scratch = make_scratch();
+        make_build_package_archive(scratch, "gone", "pc:lib-removed.pc", {});
+        make_build_package_archive(scratch, "needy", "pc:lib-needy.pc", { "pc:lib-removed.pc" });
+
+        var to_install = new HashSet<Usm.CachedPackage>();
+        var to_remove = new HashSet<Usm.CachedPackage>();
+        to_install.add(cache_package(scratch, "needy"));
+        to_remove.add(cache_package(scratch, "gone"));
+
+        var transaction = new Usm.Transaction() {
+            paths = new Usm.Paths(),
+            resource_finder = new Usm.ResourceFinder(),
+            to_install = to_install,
+            to_remove = to_remove
+        };
+        try {
+            transaction.strategise();
+            check(false, "a removed provider without replacement deadlocks the strategy");
+        }
+        catch(Usm.TransactionError e) {
+            check("going away" in e.message, "the deadlock reason names the removed provider");
+            check("pc:lib-removed.pc" in e.message, "the deadlock reason names the poisoned resource");
+        }
+
+        Usm.Util.delete_tree(scratch);
+    }
+
     void test_transaction_manage_deps_admit_same_lot() throws Error {
         var scratch = make_scratch();
         make_package_archive(scratch, "m1", {});
@@ -2798,23 +2877,6 @@ echo "Installing libmeson-pkg.so to /usr/lib"
         check(v01.compare(v01_again) == 0, "0.1 == 0.1");
     }
 
-    /**
-     * The interrupt prompt's reply parsing: only c/continue keep the
-     * transaction going; everything else — empty, junk, EOF — rolls
-     * back, the safe default.
-     */
-    void test_interrupt_reply_parsing() {
-        check(Usm.TransactionInterrupt.parse_reply("c"), "c continues");
-        check(Usm.TransactionInterrupt.parse_reply("C"), "C continues");
-        check(Usm.TransactionInterrupt.parse_reply("continue"), "continue continues");
-        check(Usm.TransactionInterrupt.parse_reply("  continue  "), "whitespace-trimmed continue continues");
-        check(!Usm.TransactionInterrupt.parse_reply("r"), "r rolls back");
-        check(!Usm.TransactionInterrupt.parse_reply("rollback"), "rollback rolls back");
-        check(!Usm.TransactionInterrupt.parse_reply(""), "empty rolls back");
-        check(!Usm.TransactionInterrupt.parse_reply("junk"), "junk rolls back");
-        check(!Usm.TransactionInterrupt.parse_reply(null), "EOF rolls back");
-    }
-
     /**
      * The SIGINT flag and its install/restore cycle: the flag is what the
      * transaction checkpoints poll, and restore clears it with the
@@ -2829,7 +2891,6 @@ echo "Installing libmeson-pkg.so to /usr/lib"
 
     int main() {
         test_default_ignore();
-        test_interrupt_reply_parsing();
         test_interrupt_flag_cycle();
         try {
             test_version_ordering();
@@ -2912,6 +2973,8 @@ echo "Installing libmeson-pkg.so to /usr/lib"
             test_system_package_manager_install_stub();
             test_transaction_accepts_orders();
             test_transaction_lots_split_at_build_boundaries();
+            test_transaction_update_pairs_split_at_build_boundaries();
+            test_transaction_removed_provider_reason();
             test_transaction_manage_deps_admit_same_lot();
             test_transaction_preinstalled_resources_satisfy_build_deps();
             test_transaction_deadlock_message_details_causes();