fix(34): WR-02 undo a failed plugin:install or plugin:update

A failure after the first change left the application half-modified: a
checkout moved into plugins/<vendor>/<name> but not registered, go.mod
edited, submodules without summer.yaml entries (and the next install
refused the leftover directory), or some submodules updated and others
not. Both commands now snapshot go.mod, go.sum, go.work, go.work.sum,
summer.yaml and .gitmodules (with its index entry) before applying; a
failure restores them, removes the submodules the run added (index
entry, submodule config, absorbed git dir, checkout; registered right
after the move) and checks moved submodules out where they were. What
cannot be undone is named in the error.

internal/build/install.go

@@ -70,6 +70,8 @@ type InstallPlan struct {
 	// whatever is left of them is removed at the end.
 	temps       []string
 	interactive bool
+	// snap is what ApplyInstall restores when it fails.
+	snap *appSnapshot
 }
 
 // InstallStep is one plugin to add.
@@ -667,7 +669,10 @@ func displayVersion(v string) string {
 // overrides the workspace needs. Module steps go through one go get, so
 // minimal version selection sees the whole set. The summer.yaml entries
 // are appended last, in plan order. It does not run go mod tidy: until
-// summer build imports the new packages, tidy would drop them.
+// summer build imports the new packages, tidy would drop them. When a
+// step fails, the files it edits (go.mod, go.sum, go.work, summer.yaml,
+// .gitmodules) are restored and the submodules this run added are
+// removed again; what cannot be undone is named in the error.
 func ApplyInstall(ctx context.Context, plan *InstallPlan, out io.Writer) error {
 	if plan == nil || len(plan.Steps) == 0 {
 		return fmt.Errorf("build: nothing to install")
@@ -696,6 +701,13 @@ func ApplyInstall(ctx context.Context, plan *InstallPlan, out io.Writer) error {
 			submodules = true
 		}
 	}
+	snap, err := snapshotApp(ctx, plan.AppDir)
+	if err != nil {
+		return err
+	}
+	plan.snap = snap
+	// A failure from here on puts the application back (WR-02).
+	fail := func(err error) error { return snap.undo(ctx, out, err) }
 	if submodules {
 		if err := requireGitRepo(ctx, plan.AppDir); err != nil {
 			return err
@@ -707,22 +719,22 @@ func ApplyInstall(ctx context.Context, plan *InstallPlan, out io.Writer) error {
 				continue
 			}
 			if err := applySubmodule(ctx, plan, s, plan.interactive, out); err != nil {
-				return err
+				return fail(err)
 			}
 			done[s.Module] = true
 		}
 		if err := refreshWorkOverrides(ctx, plan.AppDir, out); err != nil {
-			return err
+			return fail(err)
 		}
 	}
 	if len(getArgs) > 0 {
 		fmt.Fprintf(out, "go get %s\n", strings.Join(getArgs, " "))
 		if err := runGo(ctx, plan.AppDir, append([]string{"get", "--"}, getArgs...)...); err != nil {
-			return err
+			return fail(err)
 		}
 	}
 	if err := appendManifestPlugins(filepath.Join(plan.AppDir, manifestName), entries); err != nil {
-		return err
+		return fail(err)
 	}
 	ids := make([]string, len(plan.Steps))
 	for i, s := range plan.Steps {

internal/build/install_smoke_test.go

@@ -851,6 +851,89 @@ func TestInstallSmoke(t *testing.T) {
 		}
 	})
 
+	t.Run("rollback", func(t *testing.T) {
+		if testing.Short() {
+			t.Skip("clones fixture repositories")
+		}
+		gitTestEnv(t)
+		blogRepo := pluginRepo(t, "acme.blog", blogModule, []fixtureRelease{{Tag: "v0.1.0", Framework: "v0.5.0"}}, nil)
+		app := gitApp(t)
+		reg := fakeRegistry(t, map[string]fakePlugin{
+			"acme.blog": regPlugin("acme.blog", blogRepo, regVersion("v0.1.0", "v0.5.0", blogModule, blogModule, nil)),
+		})
+		files := []string{"go.mod", "go.sum", "go.work", manifestName, ".gitmodules"}
+		read := func() map[string]string {
+			got := map[string]string{}
+			for _, f := range files {
+				raw, err := os.ReadFile(filepath.Join(app, f))
+				if err != nil {
+					got[f] = "<absent>"
+					continue
+				}
+				got[f] = string(raw)
+			}
+			got["status"] = gitOut(t, app, "status", "--porcelain")
+			return got
+		}
+		before := read()
+		plan, err := PlanInstall(t.Context(), InstallOptions{StartDir: app, Arg: "acme.blog", Source: true, Yes: true, Registry: reg.Client})
+		if err != nil {
+			t.Fatal(err)
+		}
+		// A module step whose go get fails (GOPROXY=off) after the
+		// submodule was added.
+		plan.Steps = append(plan.Steps, InstallStep{ID: "acme.gone", Version: "v0.1.0", Module: "example.com/acme/gone", Package: "example.com/acme/gone", Mode: ModeModule})
+		var out bytes.Buffer
+		err = ApplyInstall(t.Context(), plan, &out)
+		if err == nil || !strings.Contains(err.Error(), "go get") || !strings.Contains(out.String(), "Undid the changes of this run.") {
+			t.Fatalf("apply: %v\n%s", err, out.String())
+		}
+		after := read()
+		for k, v := range before {
+			if after[k] != v {
+				t.Errorf("%s not restored:\nbefore %q\nafter  %q", k, v, after[k])
+			}
+		}
+		if dirExists(filepath.Join(app, "plugins", "acme", "blog")) || dirExists(filepath.Join(app, ".git", "modules", "plugins", "acme", "blog")) {
+			t.Error("the added submodule is still there")
+		}
+		// The next run is not refused by leftovers.
+		plan, err = PlanInstall(t.Context(), InstallOptions{StartDir: app, Arg: "acme.blog", Source: true, Yes: true, Registry: reg.Client})
+		if err != nil {
+			t.Fatalf("plan after the rollback: %v", err)
+		}
+		if err := ApplyInstall(t.Context(), plan, nil); err != nil {
+			t.Fatalf("apply after the rollback: %v", err)
+		}
+		assertSubmodule(t, app, "plugins/acme/blog", "v0.1.0")
+
+		// plugin:update: the submodule moves, then go get of a module
+		// plugin fails; both go back.
+		tagRelease(t, blogRepo, "acme.blog", blogModule, fixtureRelease{Tag: "v0.1.1", Framework: "v0.5.0"})
+		reg.set("acme.blog", regPlugin("acme.blog", blogRepo,
+			regVersion("v0.1.1", "v0.5.0", blogModule, blogModule, nil),
+			regVersion("v0.1.0", "v0.5.0", blogModule, blogModule, nil)))
+		reg.set("acme.gone", regPlugin("acme.gone", "", regVersion("v0.2.0", "v0.5.0", "example.com/acme/gone", "example.com/acme/gone", nil)))
+		if err := runGo(t.Context(), app, "mod", "edit", "-require=example.com/acme/gone@v0.1.0"); err != nil {
+			t.Fatal(err)
+		}
+		if err := appendManifestPlugins(filepath.Join(app, manifestName), []Plugin{{ID: "acme.gone", Module: "example.com/acme/gone"}}); err != nil {
+			t.Fatal(err)
+		}
+		gomod, _ := os.ReadFile(filepath.Join(app, "go.mod"))
+		out.Reset()
+		err = Update(t.Context(), UpdateOptions{StartDir: app, Yes: true, Registry: reg.Client, Out: &out})
+		if err == nil || !strings.Contains(out.String(), "Undid the changes of this run.") {
+			t.Fatalf("update: %v\n%s", err, out.String())
+		}
+		if got := gitOut(t, filepath.Join(app, "plugins", "acme", "blog"), "describe", "--tags"); got != "v0.1.0" {
+			t.Errorf("submodule at %s after the rollback, want v0.1.0", got)
+		}
+		if after, _ := os.ReadFile(filepath.Join(app, "go.mod")); !bytes.Equal(gomod, after) {
+			t.Errorf("go.mod not restored:\n%s", after)
+		}
+	})
+
 	t.Run("update_prompt", func(t *testing.T) {
 		if testing.Short() {
 			t.Skip("clones fixture repositories")

internal/build/rollback.go

@@ -0,0 +1,182 @@
+package build
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// appSnapshot keeps what plugin:install and plugin:update change in an
+// application, so a run that fails half-way puts it back (WR-02): the
+// files they edit (go.mod, go.sum, go.work, go.work.sum, summer.yaml and
+// .gitmodules, with the .gitmodules index entry), the submodules a run
+// added and the checkouts it moved.
+type appSnapshot struct {
+	appDir string
+	// files maps a path to its content before the run; nil means the
+	// file did not exist.
+	files map[string][]byte
+	// top is the git work tree of the application ("" outside git) and
+	// gitmodulesIndex the `git ls-files -s` entry of .gitmodules there.
+	top             string
+	gitmodulesIndex string
+
+	added []addedSubmodule
+	moved []movedCheckout
+}
+
+// addedSubmodule is a checkout a run moved to plugins/<vendor>/<name>
+// (and then added as a submodule).
+type addedSubmodule struct {
+	dir string
+}
+
+// movedCheckout is a submodule a run checked out at another ref; ref is
+// where it was before: a branch name, or a commit when detached.
+type movedCheckout struct {
+	dir, ref string
+}
+
+// snapshotApp records the files of appDir that a run may edit.
+func snapshotApp(ctx context.Context, appDir string) (*appSnapshot, error) {
+	s := &appSnapshot{appDir: appDir, files: map[string][]byte{}}
+	paths := []string{
+		filepath.Join(appDir, "go.mod"), filepath.Join(appDir, "go.sum"),
+		filepath.Join(appDir, manifestName),
+	}
+	gw := findGoWork(appDir)
+	if gw == "" {
+		gw = filepath.Join(appDir, "go.work")
+	}
+	paths = append(paths, gw, gw+".sum")
+	if out, err := gitOutput(ctx, appDir, "rev-parse", "--show-toplevel"); err == nil {
+		s.top = strings.TrimSpace(string(out))
+		paths = append(paths, filepath.Join(s.top, ".gitmodules"))
+		if idx, err := gitOutput(ctx, s.top, "ls-files", "-s", "--", ".gitmodules"); err == nil {
+			s.gitmodulesIndex = strings.TrimSpace(string(idx))
+		}
+	}
+	for _, p := range paths {
+		raw, err := os.ReadFile(p)
+		switch {
+		case errors.Is(err, os.ErrNotExist):
+			s.files[p] = nil
+		case err != nil:
+			return nil, fmt.Errorf("build: read %s: %w", p, err)
+		default:
+			s.files[p] = raw
+		}
+	}
+	return s, nil
+}
+
+// addedCheckout records a checkout moved into the application; call it
+// right after the move, before anything else can fail.
+func (s *appSnapshot) addedCheckout(dir string) {
+	if s != nil {
+		s.added = append(s.added, addedSubmodule{dir: dir})
+	}
+}
+
+// movingCheckout records where the submodule in dir is before it is
+// checked out elsewhere.
+func (s *appSnapshot) movingCheckout(ctx context.Context, dir string) error {
+	if s == nil {
+		return nil
+	}
+	ref := ""
+	if out, err := gitOutput(ctx, dir, "symbolic-ref", "--quiet", "--short", "HEAD"); err == nil {
+		ref = strings.TrimSpace(string(out))
+	} else if out, err := gitOutput(ctx, dir, "rev-parse", "--verify", "HEAD"); err == nil {
+		ref = strings.TrimSpace(string(out))
+	} else {
+		return fmt.Errorf("build: read the checkout of %s: %w", dir, err)
+	}
+	s.moved = append(s.moved, movedCheckout{dir: dir, ref: ref})
+	return nil
+}
+
+// restore puts the application back as the snapshot found it and prints
+// what it did. What it could not undo is returned as one error naming
+// the commands that finish the job.
+func (s *appSnapshot) restore(ctx context.Context, out io.Writer) error {
+	if s == nil {
+		return nil
+	}
+	// Undo even when the run was cancelled.
+	ctx = context.WithoutCancel(ctx)
+	var left []string
+	for i := len(s.moved) - 1; i >= 0; i-- {
+		m := s.moved[i]
+		if _, err := runCLIGit(ctx, m.dir, false, "checkout", "--quiet", m.ref); err != nil {
+			left = append(left, fmt.Sprintf("git -C %s checkout %s", m.dir, m.ref))
+		}
+	}
+	for i := len(s.added) - 1; i >= 0; i-- {
+		a := s.added[i]
+		if s.top != "" {
+			rel, err := filepath.Rel(s.top, a.dir)
+			if err == nil {
+				rel = filepath.ToSlash(rel)
+				_, _ = gitOutput(ctx, s.top, "update-index", "--force-remove", "--", rel)
+				_, _ = gitOutput(ctx, s.top, "config", "--remove-section", "submodule."+rel)
+				if gitDir, err := gitOutput(ctx, s.top, "rev-parse", "--path-format=absolute", "--git-common-dir"); err == nil {
+					modules := filepath.Join(strings.TrimSpace(string(gitDir)), "modules")
+					if target := filepath.Join(modules, filepath.FromSlash(rel)); underRoot(modules, target) && target != modules {
+						_ = os.RemoveAll(target)
+					}
+				}
+			}
+		}
+		if underRoot(s.appDir, a.dir) {
+			if err := os.RemoveAll(a.dir); err != nil {
+				left = append(left, "rm -r "+a.dir)
+			}
+		}
+	}
+	for p, raw := range s.files {
+		var err error
+		if raw == nil {
+			err = os.Remove(p)
+			if errors.Is(err, os.ErrNotExist) {
+				err = nil
+			}
+		} else {
+			err = os.WriteFile(p, raw, 0o644)
+		}
+		if err != nil {
+			left = append(left, "restore "+p+" from git")
+		}
+	}
+	if s.top != "" {
+		if s.gitmodulesIndex == "" {
+			_, _ = gitOutput(ctx, s.top, "rm", "--cached", "--quiet", "--ignore-unmatch", "--", ".gitmodules")
+		} else if mode, rest, ok := strings.Cut(s.gitmodulesIndex, " "); ok {
+			if sha, _, ok := strings.Cut(rest, " "); ok {
+				if _, err := gitOutput(ctx, s.top, "update-index", "--cacheinfo", mode+","+sha+",.gitmodules"); err != nil {
+					left = append(left, "git -C "+s.top+" add .gitmodules")
+				}
+			}
+		}
+	}
+	if len(left) > 0 {
+		return fmt.Errorf("build: could not undo every change; finish with: %s", strings.Join(left, "; "))
+	}
+	if out != nil {
+		fmt.Fprintln(out, "Undid the changes of this run.")
+	}
+	return nil
+}
+
+// undo restores the snapshot after err and returns err, with the cleanup
+// that could not be done attached.
+func (s *appSnapshot) undo(ctx context.Context, out io.Writer, err error) error {
+	if rerr := s.restore(ctx, out); rerr != nil {
+		return fmt.Errorf("%w\n%v", err, rerr)
+	}
+	return err
+}

internal/build/source.go

@@ -457,6 +457,8 @@ func applySubmodule(ctx context.Context, plan *InstallPlan, s *InstallStep, inte
 	if err := os.Rename(dir, target); err != nil {
 		return fmt.Errorf("build: move checkout to %s: %w", target, err)
 	}
+	// From here on a failure must take the checkout away again.
+	plan.snap.addedCheckout(target)
 	relTarget, err := filepath.Rel(appDir, target)
 	if err != nil {
 		return fmt.Errorf("build: %w", err)

internal/build/update.go

@@ -499,19 +499,29 @@ func (u *updater) report() {
 
 // apply checks submodules out at their new tags, requires the tags in
 // go.mod, runs one go get for the module updates and refreshes the
-// go.work overrides.
+// go.work overrides. A failure checks the submodules out where they were
+// and restores go.mod, go.sum and go.work.
 func (u *updater) apply() error {
+	snap, err := snapshotApp(u.ctx, u.appDir)
+	if err != nil {
+		return err
+	}
+	// A failure puts every checkout and file back (WR-02).
+	fail := func(err error) error { return snap.undo(u.ctx, u.out, err) }
 	var getArgs []string
 	for _, s := range u.steps {
 		switch s.Src.Kind {
 		case ModeModule:
 			getArgs = append(getArgs, s.Src.Module+"@"+s.To.Version)
 		case ModeSubmodule:
+			if err := snap.movingCheckout(u.ctx, s.Src.Dir); err != nil {
+				return fail(err)
+			}
 			if err := checkout(u.ctx, s.Src.Dir, s.To.Version); err != nil {
-				return err
+				return fail(err)
 			}
 			if err := runGo(u.ctx, u.appDir, "mod", "edit", "-require="+s.Src.Module+"@"+s.To.Version); err != nil {
-				return err
+				return fail(err)
 			}
 			fmt.Fprintf(u.out, "Checked out %s at %s\n", s.ID, s.To.Version)
 		}
@@ -519,11 +529,11 @@ func (u *updater) apply() error {
 	if len(getArgs) > 0 {
 		fmt.Fprintf(u.out, "go get %s\n", strings.Join(getArgs, " "))
 		if err := runGo(u.ctx, u.appDir, append([]string{"get", "--"}, getArgs...)...); err != nil {
-			return err
+			return fail(err)
 		}
 	}
 	if err := refreshWorkOverrides(u.ctx, u.appDir, u.out); err != nil {
-		return err
+		return fail(err)
 	}
 	fmt.Fprintln(u.out, "Updated; run summer build")
 	return nil