diff --git a/internal/system/arch.go b/internal/system/arch.go index 880fef3..80ce687 100644 --- a/internal/system/arch.go +++ b/internal/system/arch.go @@ -40,8 +40,50 @@ func (a arch) PackageInstalled(ctx context.Context, run Runner, name string) (bo } func (arch) InstallPackage(ctx context.Context, run Runner, name string) error { - _, err := run(ctx, "pacman", "-S", "--noconfirm", "--needed", name) - return err + out, err := run(ctx, "pacman", "-S", "--noconfirm", "--needed", name) + if err == nil { + return nil + } + + // **The package manager's own words, and a name for the case that looks like a bug in the + // declaration and is not.** A stale index asks the mirrors for a version they have already + // superseded and gets a 404 from every one of them — so the package exists, the declaration is + // correct, and the machine's idea of what exists is old (novox/hq 04-ISSUES/002). + // + // **It is not fixed by syncing here.** `pacman -Sy ` installs a package built against + // libraries this machine does not have: a partial upgrade, which Arch does not support and + // which breaks the machine in a way that surfaces much later as something unrelated. The + // remedy is a full upgrade, and it is a decision about the whole machine rather than + // something to do silently in the middle of applying one resource. + // + // So this says which of the two it is looking at. A declaration that is wrong and a machine + // that is out of date fail identically otherwise, and they are fixed in completely different + // places. + if staleIndex(out) { + return fmt.Errorf( + "%s could not be fetched from any mirror, which is what a stale package index looks "+ + "like: this machine is asking for a version the mirrors have replaced. The "+ + "package and the declaration are probably both fine. It is fixed by upgrading "+ + "the machine, not by this host syncing one package — that would be a partial "+ + "upgrade, which this distribution does not support.\n\n%s", + name, strings.TrimSpace(out)) + } + return fmt.Errorf("%w\n\n%s", err, strings.TrimSpace(out)) +} + +// staleIndex reports whether a failed install looks like the machine's view being old rather than +// the package being wrong. +// +// By what the package manager said, because there is nothing else to go on: the exit code is the +// same for both. +func staleIndex(out string) bool { + said := strings.ToLower(out) + if !strings.Contains(said, "failed retrieving file") && !strings.Contains(said, "404") { + return false + } + // Every mirror, not one. A single mirror failing is an ordinary transient thing and retrying + // is the answer; every one of them saying the file is gone is the index being old. + return strings.Contains(said, "error") || strings.Count(said, "404") > 1 } // ServiceState reads what systemd says about a unit. diff --git a/internal/system/system_test.go b/internal/system/system_test.go index 303a3ac..41702ee 100644 --- a/internal/system/system_test.go +++ b/internal/system/system_test.go @@ -345,3 +345,72 @@ func TestAPartialHostRefusesUsersAndAllowsArchives(t *testing.T) { t.Error("a partial host created a user") } } + +// A stale index and a wrong declaration fail identically, and are fixed in completely different +// places. +// +// novox/hq 04-ISSUES/002: the package exists, the declaration is correct, and the machine is +// asking the mirrors for a version they have already replaced. Reported as a generic install +// failure it sends somebody to check the manifest, which is the one thing that is right. +func TestAStaleIndexIsNamedRatherThanReportedAsAFailedInstall(t *testing.T) { + said := "error: failed retrieving file 'dnsmasq-2.90-1-x86_64.pkg.tar.zst' from mirror.one : " + + "The requested URL returned error: 404\n" + + "error: failed retrieving file 'dnsmasq-2.90-1-x86_64.pkg.tar.zst' from mirror.two : " + + "The requested URL returned error: 404\n" + + "error: failed to commit transaction (failed to retrieve some files)" + run := func(context.Context, string, ...string) (string, error) { + return said, errors.New("exit status 1") + } + err := (arch{}).InstallPackage(context.Background(), run, "dnsmasq") + if err == nil { + t.Fatal("an install that failed reported success") + } + for _, want := range []string{"stale package index", "upgrading the machine", "partial upgrade"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("the failure does not say %q, so it reads as a wrong declaration:\n%v", want, err) + } + } + // And the package manager's own words, which were being thrown away entirely. + if !strings.Contains(err.Error(), "404") { + t.Fatalf("what the package manager said was discarded:\n%v", err) + } +} + +// An ordinary failure is not dressed up as a stale index: saying "upgrade the machine" about a +// package that does not exist sends somebody to do something large and useless. +func TestAnOrdinaryInstallFailureIsNotCalledAStaleIndex(t *testing.T) { + run := func(context.Context, string, ...string) (string, error) { + return "error: target not found: nosuchpackage", errors.New("exit status 1") + } + err := (arch{}).InstallPackage(context.Background(), run, "nosuchpackage") + if err == nil { + t.Fatal("an install that failed reported success") + } + if strings.Contains(err.Error(), "stale package index") { + t.Fatalf("a package that does not exist was called a stale index:\n%v", err) + } + if !strings.Contains(err.Error(), "target not found") { + t.Fatalf("what the package manager said was discarded:\n%v", err) + } +} + +// One mirror failing is transient and retrying is the answer. Every mirror saying the file is gone +// is the index being old. +func TestOneMirrorFailingIsNotAStaleIndex(t *testing.T) { + run := func(context.Context, string, ...string) (string, error) { + return "warning: failed retrieving file 'x.pkg.tar.zst' from mirror.one : timeout", + errors.New("exit status 1") + } + err := (arch{}).InstallPackage(context.Background(), run, "x") + if err != nil && strings.Contains(err.Error(), "stale package index") { + t.Fatalf("one mirror timing out was called a stale index:\n%v", err) + } +} + +// And an install that works still works. +func TestAnInstallThatSucceedsSaysNothing(t *testing.T) { + run := func(context.Context, string, ...string) (string, error) { return "installed", nil } + if err := (arch{}).InstallPackage(context.Background(), run, "dnsmasq"); err != nil { + t.Fatalf("a successful install reported a failure: %v", err) + } +}