diff --git a/internal/cli/checkout.go b/internal/cli/checkout.go index b843a4f..908fd10 100644 --- a/internal/cli/checkout.go +++ b/internal/cli/checkout.go @@ -38,7 +38,7 @@ Three modes of operation: Checks out in ALL repos where it exists. Repos where the branch is not found are skipped with a warning. -Repositories with uncommitted tracked changes are always skipped. +Repositories are skipped when uncommitted changes conflict with the target branch. Use --repo to limit the operation to specific repositories by alias.`, Example: ` gitm checkout @@ -94,23 +94,10 @@ func runCheckoutDefault(repos []*db.Repository) error { fmt.Printf("Checking out default branch and pulling for %d repositories…\n\n", len(repos)) results := runner.Run(repos, func(repo *db.Repository) (string, string, error) { - dirty, err := git.IsDirtyTrackedOnly(repo.Path) - if err != nil { - return "", "", fmt.Errorf("git status failed: %w", err) - } - if dirty { - files, filesErr := git.DirtyFiles(repo.Path) - if filesErr != nil { - return "", "", fmt.Errorf("list dirty files: %w", filesErr) - } - reason := fmt.Sprintf("uncommitted changes (%d file(s))", len(files)) - if len(files) > 0 && len(files) <= 3 { - reason += ": " + strings.Join(files, ", ") - } - return "", reason, nil - } - if checkoutErr := git.Checkout(repo.Path, repo.DefaultBranch); checkoutErr != nil { + if skip, reason := checkoutConflictSkip(repo.Path, checkoutErr); skip { + return "", reason, nil + } return "", "", fmt.Errorf("checkout %s: %w", repo.DefaultBranch, checkoutErr) } @@ -173,23 +160,6 @@ func runCheckoutInteractive(repos []*db.Repository, ui ui) error { // checkoutBranchInRepo performs the dirty check, branch-existence check, // checkout, and pull for a single repo. Shared by both specific and interactive modes. func checkoutBranchInRepo(repo *db.Repository, branch string) (string, string, error) { - // Skip if tracked files are dirty. - dirty, err := git.IsDirtyTrackedOnly(repo.Path) - if err != nil { - return "", "", fmt.Errorf("git status failed: %w", err) - } - if dirty { - files, filesErr := git.DirtyFiles(repo.Path) - if filesErr != nil { - return "", "", fmt.Errorf("list dirty files: %w", filesErr) - } - reason := fmt.Sprintf("uncommitted changes (%d file(s))", len(files)) - if len(files) > 0 && len(files) <= 3 { - reason += ": " + strings.Join(files, ", ") - } - return "", reason, nil - } - // Check branch existence: local first, then remote. localExists := git.BranchExists(repo.Path, branch) remoteExists := false @@ -210,6 +180,9 @@ func checkoutBranchInRepo(repo *db.Repository, branch string) (string, string, e } if checkoutErr := git.Checkout(repo.Path, branch); checkoutErr != nil { + if skip, reason := checkoutConflictSkip(repo.Path, checkoutErr); skip { + return "", reason, nil + } return "", "", fmt.Errorf("checkout: %w", checkoutErr) } @@ -221,6 +194,28 @@ func checkoutBranchInRepo(repo *db.Repository, branch string) (string, string, e return fmt.Sprintf("on %s — %s", branch, summarisePull(out)), "", nil } +func isCheckoutConflict(err error) bool { + msg := err.Error() + return strings.Contains(msg, "Your local changes") || + strings.Contains(msg, "overwritten by checkout") || + strings.Contains(msg, "overwritten by merge") +} + +func checkoutConflictSkip(repoPath string, checkoutErr error) (bool, string) { + if !isCheckoutConflict(checkoutErr) { + return false, "" + } + files, filesErr := git.DirtyFiles(repoPath) + if filesErr != nil { + return true, "uncommitted changes conflict with target branch" + } + reason := fmt.Sprintf("uncommitted changes conflict (%d file(s))", len(files)) + if len(files) > 0 && len(files) <= 3 { + reason += ": " + strings.Join(files, ", ") + } + return true, reason +} + // summarisePull condenses git pull output into a short message. func summarisePull(out string) string { out = strings.TrimSpace(out) diff --git a/internal/cli/checkout_run_test.go b/internal/cli/checkout_run_test.go index 43827d4..34a9ac3 100644 --- a/internal/cli/checkout_run_test.go +++ b/internal/cli/checkout_run_test.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "os" "path/filepath" "strings" @@ -286,3 +287,221 @@ func TestRunCheckoutDefault_ReturnsErrorOnFailure(t *testing.T) { t.Errorf("error = %q, want to contain \"failed to checkout\"", err.Error()) } } + +func TestRunCheckoutDefault_NonConflictingDirtyFile(t *testing.T) { + database = setupTestDB(t) + dir, _, _ := initRepoWithRemote(t) + + writeFile(t, dir, "config.txt", "original\n") + mustRunGit(t, dir, "add", "config.txt") + mustRunGit(t, dir, "commit", "-m", "add config") + mustRunGit(t, dir, "push") + + mustRunGit(t, dir, "checkout", "-b", "feature") + mustRunGit(t, dir, "push", "--set-upstream", "origin", "feature") + mustRunGit(t, dir, "commit", "--allow-empty", "-m", "feature work") + + writeFile(t, dir, "config.txt", "local modification\n") + + repo := &db.Repository{ID: 1, Alias: "repo1", Path: dir, DefaultBranch: "main"} + if err := runCheckoutDefault([]*db.Repository{repo}); err != nil { + t.Fatalf("should succeed with non-conflicting dirty file: %v", err) + } + + if head := gitCurrentBranch(t, dir); head != "main" { + t.Fatalf("head = %q, want main", head) + } +} + +func TestRunCheckoutDefault_ConflictingDirtyFile(t *testing.T) { + database = setupTestDB(t) + dir := initRepo(t) + + writeFile(t, dir, "conflict.txt", "main-version\n") + mustRunGit(t, dir, "add", "conflict.txt") + mustRunGit(t, dir, "commit", "-m", "add conflict on main") + + mustRunGit(t, dir, "checkout", "-b", "feature") + writeFile(t, dir, "conflict.txt", "feature-version\n") + mustRunGit(t, dir, "add", "conflict.txt") + mustRunGit(t, dir, "commit", "-m", "change conflict on feature") + + writeFile(t, dir, "conflict.txt", "local dirty\n") + + repo := &db.Repository{ID: 1, Alias: "repo1", Path: dir, DefaultBranch: "main"} + if err := runCheckoutDefault([]*db.Repository{repo}); err != nil { + t.Fatalf("should skip (not error) for conflicting dirty file: %v", err) + } + + if head := gitCurrentBranch(t, dir); head != "feature" { + t.Fatalf("should stay on feature (skipped), got %s", head) + } +} + +func TestCheckoutBranchInRepo_NonConflictingDirtyFile(t *testing.T) { + dir, _, _ := initRepoWithRemote(t) + + writeFile(t, dir, "safe.txt", "original\n") + mustRunGit(t, dir, "add", "safe.txt") + mustRunGit(t, dir, "commit", "-m", "add safe file") + mustRunGit(t, dir, "push") + + mustRunGit(t, dir, "checkout", "-b", "target") + mustRunGit(t, dir, "push", "--set-upstream", "origin", "target") + mustRunGit(t, dir, "commit", "--allow-empty", "-m", "target commit") + mustRunGit(t, dir, "checkout", "main") + + writeFile(t, dir, "safe.txt", "dirty but safe\n") + + repo := &db.Repository{ID: 1, Alias: "repo1", Path: dir, DefaultBranch: "main"} + msg, skipReason, err := checkoutBranchInRepo(repo, "target") + if err != nil { + t.Fatalf("should succeed: %v", err) + } + if skipReason != "" { + t.Fatalf("should not skip, got: %s", skipReason) + } + if msg == "" { + t.Fatal("expected success message") + } + + if head := gitCurrentBranch(t, dir); head != "target" { + t.Fatalf("head = %q, want target", head) + } +} + +func TestCheckoutBranchInRepo_ConflictingDirtyFile(t *testing.T) { + dir := initRepo(t) + + writeFile(t, dir, "conflict.txt", "main-version\n") + mustRunGit(t, dir, "add", "conflict.txt") + mustRunGit(t, dir, "commit", "-m", "add conflict on main") + + mustRunGit(t, dir, "checkout", "-b", "target") + writeFile(t, dir, "conflict.txt", "target-version\n") + mustRunGit(t, dir, "add", "conflict.txt") + mustRunGit(t, dir, "commit", "-m", "change conflict on target") + + mustRunGit(t, dir, "checkout", "main") + writeFile(t, dir, "conflict.txt", "local dirty\n") + + repo := &db.Repository{ID: 1, Alias: "repo1", Path: dir, DefaultBranch: "main"} + _, skipReason, err := checkoutBranchInRepo(repo, "target") + if err != nil { + t.Fatalf("should skip, not error: %v", err) + } + if skipReason == "" { + t.Fatal("expected skip reason for conflicting dirty file") + } + if !strings.Contains(skipReason, "conflict") { + t.Errorf("skip reason = %q, want to contain 'conflict'", skipReason) + } + + if head := gitCurrentBranch(t, dir); head != "main" { + t.Fatalf("should stay on main (skipped), got %s", head) + } +} + +func TestCheckoutInteractive_NonConflictingDirtyFile(t *testing.T) { + database = setupTestDB(t) + dir, _, _ := initRepoWithRemote(t) + + writeFile(t, dir, "tracked.txt", "original\n") + mustRunGit(t, dir, "add", "tracked.txt") + mustRunGit(t, dir, "commit", "-m", "add tracked file") + mustRunGit(t, dir, "push") + + mustRunGit(t, dir, "checkout", "-b", "target") + mustRunGit(t, dir, "push", "--set-upstream", "origin", "target") + mustRunGit(t, dir, "commit", "--allow-empty", "-m", "target commit") + mustRunGit(t, dir, "checkout", "main") + + writeFile(t, dir, "tracked.txt", "dirty but non-conflicting\n") + + repo := &db.Repository{ID: 1, Alias: "repo1", Path: dir, DefaultBranch: "main"} + ui := fakeUI{selectRepos: []*db.Repository{repo}, branchName: "target"} + + if err := runCheckoutInteractive([]*db.Repository{repo}, ui); err != nil { + t.Fatalf("interactive checkout should succeed with non-conflicting dirty: %v", err) + } + + if head := gitCurrentBranch(t, dir); head != "target" { + t.Fatalf("head = %q, want target", head) + } +} + +func TestCheckoutDefault_MultipleReposMixedDirtyState(t *testing.T) { + database = setupTestDB(t) + + cleanDir, _, _ := initRepoWithRemote(t) + mustRunGit(t, cleanDir, "checkout", "-b", "feature") + mustRunGit(t, cleanDir, "push", "--set-upstream", "origin", "feature") + mustRunGit(t, cleanDir, "commit", "--allow-empty", "-m", "feature") + + conflictDir, _, _ := initRepoWithRemote(t) + writeFile(t, conflictDir, "conflict.txt", "v1\n") + mustRunGit(t, conflictDir, "add", "conflict.txt") + mustRunGit(t, conflictDir, "commit", "-m", "add conflict") + mustRunGit(t, conflictDir, "push") + mustRunGit(t, conflictDir, "checkout", "-b", "feature") + mustRunGit(t, conflictDir, "push", "--set-upstream", "origin", "feature") + writeFile(t, conflictDir, "conflict.txt", "v2\n") + mustRunGit(t, conflictDir, "add", "conflict.txt") + mustRunGit(t, conflictDir, "commit", "-m", "change conflict") + writeFile(t, conflictDir, "conflict.txt", "local dirty\n") + + nonConflictDir, _, _ := initRepoWithRemote(t) + writeFile(t, nonConflictDir, "safe.txt", "original\n") + mustRunGit(t, nonConflictDir, "add", "safe.txt") + mustRunGit(t, nonConflictDir, "commit", "-m", "add safe") + mustRunGit(t, nonConflictDir, "push") + mustRunGit(t, nonConflictDir, "checkout", "-b", "feature") + mustRunGit(t, nonConflictDir, "push", "--set-upstream", "origin", "feature") + mustRunGit(t, nonConflictDir, "commit", "--allow-empty", "-m", "feature") + writeFile(t, nonConflictDir, "safe.txt", "dirty but safe\n") + + repos := []*db.Repository{ + {ID: 1, Alias: "clean", Path: cleanDir, DefaultBranch: "main"}, + {ID: 2, Alias: "conflict", Path: conflictDir, DefaultBranch: "main"}, + {ID: 3, Alias: "non-conflict", Path: nonConflictDir, DefaultBranch: "main"}, + } + + if err := runCheckoutDefault(repos); err != nil { + t.Fatalf("runCheckoutDefault failed: %v", err) + } + + if b := gitCurrentBranch(t, cleanDir); b != "main" { + t.Errorf("clean repo: expected main, got %s", b) + } + if b := gitCurrentBranch(t, conflictDir); b != "feature" { + t.Errorf("conflict repo: expected to stay on feature, got %s", b) + } + if b := gitCurrentBranch(t, nonConflictDir); b != "main" { + t.Errorf("non-conflict repo: expected main (dirty carried forward), got %s", b) + } +} + +func TestIsCheckoutConflict(t *testing.T) { + tests := []struct { + msg string + want bool + }{ + {"error: Your local changes to the following files would be overwritten by checkout", true}, + {"error: Your local changes to the following files would be overwritten by merge", true}, + {"error: pathspec 'nonexistent' did not match any file(s) known to git", false}, + {"exit status 1", false}, + } + for _, tt := range tests { + got := isCheckoutConflict(fmt.Errorf("%s", tt.msg)) + if got != tt.want { + t.Errorf("isCheckoutConflict(%q) = %v, want %v", tt.msg, got, tt.want) + } + } +} + +func TestCheckoutConflictSkip_NonConflictError(t *testing.T) { + skip, _ := checkoutConflictSkip("/nonexistent", fmt.Errorf("pathspec not found")) + if skip { + t.Error("should not skip for non-conflict errors") + } +} diff --git a/internal/e2e/checkout_test.go b/internal/e2e/checkout_test.go index 237f35a..a331d0f 100644 --- a/internal/e2e/checkout_test.go +++ b/internal/e2e/checkout_test.go @@ -68,10 +68,10 @@ func TestCheckout_NonExistentBranch(t *testing.T) { } } -func TestCheckout_DirtyRepo_Skips(t *testing.T) { +func TestCheckout_DirtyRepo_NonConflicting_Succeeds(t *testing.T) { e := newTestEnv(t) - repo, _ := e.initRepoWithRemote("co-dirty") - e.runGitm("repo", "add", repo, "--alias", "co-dirty") + repo, _ := e.initRepoWithRemote("co-dirty-ok") + e.runGitm("repo", "add", repo, "--alias", "co-dirty-ok") e.mustGit(repo, "checkout", "-b", "feat/dirty-target") e.mustGit(repo, "push", "--set-upstream", "origin", "feat/dirty-target") @@ -79,14 +79,42 @@ func TestCheckout_DirtyRepo_Skips(t *testing.T) { e.writeFile(repo, "README.md", "# dirty content\n") - r := e.runGitm("checkout", "feat/dirty-target", "--repo", "co-dirty") + r := e.runGitm("checkout", "feat/dirty-target", "--repo", "co-dirty-ok") + e.assertExitCode(r, 0) + + branch := e.currentBranch(repo) + if branch != "feat/dirty-target" { + t.Errorf("non-conflicting dirty repo should switch branches, but stayed on %s", branch) + } +} + +func TestCheckout_DirtyRepo_Conflicting_Skips(t *testing.T) { + e := newTestEnv(t) + repo, _ := e.initRepoWithRemote("co-dirty-conflict") + e.runGitm("repo", "add", repo, "--alias", "co-dirty-conflict") + + e.writeFile(repo, "conflict.txt", "main-version\n") + e.mustGit(repo, "add", "conflict.txt") + e.mustGit(repo, "commit", "-m", "add conflict file on main") + e.mustGit(repo, "push") + + e.mustGit(repo, "checkout", "-b", "feat/conflict-target") + e.writeFile(repo, "conflict.txt", "feature-version\n") + e.mustGit(repo, "add", "conflict.txt") + e.mustGit(repo, "commit", "-m", "change conflict file on feature") + e.mustGit(repo, "push", "--set-upstream", "origin", "feat/conflict-target") + e.mustGit(repo, "checkout", "main") + + e.writeFile(repo, "conflict.txt", "local dirty change\n") + + r := e.runGitm("checkout", "feat/conflict-target", "--repo", "co-dirty-conflict") e.assertExitCode(r, 0) branch := e.currentBranch(repo) if branch != "main" { - t.Errorf("dirty repo should not switch branches, but now on %s", branch) + t.Errorf("conflicting dirty repo should stay on current branch, but switched to %s", branch) } - e.assertContains(r, "co-dirty") // Should mention the repo + e.assertContains(r, "co-dirty-conflict") } func TestCheckout_UntrackedFiles_ShouldNotSkip(t *testing.T) {