diff --git a/cmd/finish.go b/cmd/finish.go index 7435c68..c2822ab 100644 --- a/cmd/finish.go +++ b/cmd/finish.go @@ -195,7 +195,7 @@ func executeFinish(branchType string, name string, continueOp bool, abortOp bool resolvedOptions := config.ResolveFinishOptions(cfg, branchType, shortName, tagOptions, retentionOptions, mergeOptions, fetch, noVerify) // Perform fetch if enabled (only on initial finish, not continue) - if resolvedOptions.ShouldFetch { + if resolvedOptions.ShouldFetch && git.RemoteExists(cfg.Remote) { fmt.Printf("Fetching from remote '%s'...\n", cfg.Remote) // Fetch base branch if err := git.FetchBranch(cfg.Remote, branchConfig.Parent); err != nil { @@ -350,11 +350,11 @@ func executeSteps(cfg *config.Config, state *mergestate.MergeState, branchConfig case stepMerge: err = handleMergeStep(cfg, state, branchConfig, resolvedOptions) case stepCreateTag: - err = handleCreateTagStep(state, resolvedOptions) + err = handleCreateTagStep(cfg, state, resolvedOptions) case stepUpdateChildren: err = handleUpdateChildrenStep(cfg, state, branchConfig, resolvedOptions) case stepDeleteBranch: - return handleDeleteBranchStep(state, resolvedOptions) // Final step + return handleDeleteBranchStep(cfg, state, resolvedOptions) // Final step default: return &errors.GitError{Operation: fmt.Sprintf("unknown step '%s'", state.CurrentStep), Err: nil} } @@ -667,7 +667,7 @@ func handleMergeStep(cfg *config.Config, state *mergestate.MergeState, branchCon } // handleCreateTagStep handles the tag creation step -func handleCreateTagStep(state *mergestate.MergeState, resolvedOptions *config.ResolvedFinishOptions) error { +func handleCreateTagStep(cfg *config.Config, state *mergestate.MergeState, resolvedOptions *config.ResolvedFinishOptions) error { if resolvedOptions.ShouldTag { // Apply tag message filter for any branch type configured with tagging // The filter script (filter-flow-{branchType}-finish-tag-message) decides what to do @@ -676,12 +676,7 @@ func handleCreateTagStep(state *mergestate.MergeState, resolvedOptions *config.R return &errors.GitError{Operation: "get git directory", Err: err} } - // Load config to get remote name - cfg, cfgErr := config.LoadConfig() - remote := "origin" - if cfgErr == nil { - remote = cfg.Remote - } + remote := cfg.Remote ctx := hooks.FilterContext{ BranchType: state.BranchType, @@ -746,7 +741,7 @@ func handleUpdateChildrenStep(cfg *config.Config, state *mergestate.MergeState, } // handleDeleteBranchStep handles branch deletion -func handleDeleteBranchStep(state *mergestate.MergeState, resolvedOptions *config.ResolvedFinishOptions) error { +func handleDeleteBranchStep(cfg *config.Config, state *mergestate.MergeState, resolvedOptions *config.ResolvedFinishOptions) error { // Ensure we're on the parent branch before deletion if err := git.Checkout(state.ParentBranch); err != nil { return &errors.GitError{Operation: fmt.Sprintf("checkout parent branch '%s'", state.ParentBranch), Err: err} @@ -763,7 +758,7 @@ func handleDeleteBranchStep(state *mergestate.MergeState, resolvedOptions *confi // Delete branches based on settings // Use force delete since we've already merged the branch forceDelete := true - if err := deleteBranchesIfNeeded(state, keepRemote, keepLocal, forceDelete); err != nil { + if err := deleteBranchesIfNeeded(state, cfg.Remote, keepRemote, keepLocal, forceDelete); err != nil { return err } @@ -785,19 +780,12 @@ func handleDeleteBranchStep(state *mergestate.MergeState, resolvedOptions *confi // Run post-hook after successful completion gitDir, err := git.GetGitDir() if err == nil { - // Get remote from config for hook context - cfg, cfgErr := config.LoadConfig() - remote := "origin" - if cfgErr == nil { - remote = cfg.Remote - } - hookCtx := hooks.HookContext{ BranchType: state.BranchType, BranchName: state.BranchName, FullBranch: state.FullBranchName, BaseBranch: state.ParentBranch, - Origin: remote, + Origin: cfg.Remote, ExitCode: 0, // Success } // Set version for branches configured with tagging @@ -935,13 +923,13 @@ func updateChildBranch(cfg *config.Config, branchName string, state *mergestate. } // deleteBranchesIfNeeded deletes branches based on retention settings -func deleteBranchesIfNeeded(state *mergestate.MergeState, keepRemote, keepLocal, forceDelete bool) error { +func deleteBranchesIfNeeded(state *mergestate.MergeState, remote string, keepRemote, keepLocal, forceDelete bool) error { // Delete remote branch if not keeping it and if remote branch exists if !keepRemote { // Only attempt to delete if the remote branch actually exists - if git.RemoteBranchExists("origin", state.FullBranchName) { - remoteBranch := fmt.Sprintf("origin/%s", state.FullBranchName) - if err := git.DeleteRemoteBranch("origin", state.FullBranchName); err != nil { + if git.RemoteBranchExists(remote, state.FullBranchName) { + remoteBranch := fmt.Sprintf("%s/%s", remote, state.FullBranchName) + if err := git.DeleteRemoteBranch(remote, state.FullBranchName); err != nil { return &errors.GitError{Operation: fmt.Sprintf("delete remote branch '%s'", remoteBranch), Err: err} } } diff --git a/docs/git-flow-finish.1.md b/docs/git-flow-finish.1.md index e6f7f82..79f336e 100644 --- a/docs/git-flow-finish.1.md +++ b/docs/git-flow-finish.1.md @@ -133,7 +133,7 @@ The operation maintains a persistent state file that allows it to resume after c ### Remote Fetch Options **--fetch** -: Fetch from remote before finishing the branch (default). This fetches both the base branch and the topic branch to ensure the latest remote changes are known before merging. Overrides git config setting `gitflow..finish.fetch`. +: Fetch from remote before finishing the branch (default). This fetches both the base branch and the topic branch to ensure the latest remote changes are known before merging. Overrides git config setting `gitflow..finish.fetch`. If no remote is configured, the fetch is automatically skipped. **--no-fetch** : Don't fetch from remote before finishing. Disables the default fetch behavior. Overrides git config setting `gitflow..finish.fetch`. diff --git a/internal/git/repo.go b/internal/git/repo.go index ea01ea4..95347a6 100644 --- a/internal/git/repo.go +++ b/internal/git/repo.go @@ -327,6 +327,13 @@ func DeleteRemoteBranch(remote, branch string) error { return nil } +// RemoteExists checks if a remote is configured in the local git config. +// This is a local-only check (no network call). +func RemoteExists(remote string) bool { + cmd := exec.Command("git", "remote", "get-url", remote) + return cmd.Run() == nil +} + // RemoteBranchExists checks if a remote branch exists func RemoteBranchExists(remote, branch string) bool { // Check if the remote tracking branch exists diff --git a/test/cmd/finish_fetch_test.go b/test/cmd/finish_fetch_test.go index 23857ca..1b24e4d 100644 --- a/test/cmd/finish_fetch_test.go +++ b/test/cmd/finish_fetch_test.go @@ -547,3 +547,75 @@ func TestFinishFeatureBranchContinueDoesNotFetch(t *testing.T) { t.Error("Expected resolved changes to be in develop branch") } } + +// TestFinishFeatureBranchNoRemote tests that finish skips fetch silently when no remote exists. +// Steps: +// 1. Sets up a test repository (no remote) and initializes git-flow with defaults +// 2. Creates a feature branch and adds a commit +// 3. Runs 'git flow feature finish' (default fetch=true, but no remote configured) +// 4. Verifies output does NOT contain "Fetching from remote" (fetch skipped silently) +// 5. Verifies output does NOT contain "does not appear to be a git repository" (no confusing error) +// 6. Verifies finish completes successfully (branch merged and deleted) +func TestFinishFeatureBranchNoRemote(t *testing.T) { + // Setup test repository without remote + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + // Initialize git-flow with defaults + _, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v", err) + } + + // Create a feature branch + _, err = testutil.RunGitFlow(t, dir, "feature", "start", "no-remote-test") + if err != nil { + t.Fatalf("Failed to create feature branch: %v", err) + } + + // Create a test file and commit + testutil.WriteFile(t, dir, "no-remote-test.txt", "test content") + _, err = testutil.RunGit(t, dir, "add", "no-remote-test.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Add no-remote test file") + if err != nil { + t.Fatalf("Failed to commit file: %v", err) + } + + // Finish the feature branch (fetch defaults to true, but no remote exists) + output, err := testutil.RunGitFlow(t, dir, "feature", "finish", "no-remote-test") + if err != nil { + t.Fatalf("Failed to finish feature branch: %v\nOutput: %s", err, output) + } + + // Verify fetch was skipped silently (no fetch output at all) + if strings.Contains(output, "Fetching from remote") { + t.Error("Expected fetch to be skipped silently when no remote exists") + } + + // Verify no confusing error messages about missing remote + if strings.Contains(output, "does not appear to be a git repository") { + t.Error("Expected no confusing error about missing remote") + } + + // Verify finish completed successfully + if !strings.Contains(output, "Successfully finished") { + t.Error("Expected successful finish message") + } + + // Verify feature branch is deleted + if testutil.BranchExists(t, dir, "feature/no-remote-test") { + t.Error("Expected feature branch to be deleted") + } + + // Verify changes are merged into develop + _, err = testutil.RunGit(t, dir, "checkout", "develop") + if err != nil { + t.Fatalf("Failed to checkout develop: %v", err) + } + if !testutil.FileExists(t, dir, "no-remote-test.txt") { + t.Error("Expected no-remote-test.txt to exist in develop branch") + } +} diff --git a/test/cmd/finish_retention_test.go b/test/cmd/finish_retention_test.go index af4db5c..6edeb74 100644 --- a/test/cmd/finish_retention_test.go +++ b/test/cmd/finish_retention_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os/exec" "path/filepath" + "strings" "testing" "github.com/gittower/git-flow-next/test/testutil" @@ -326,3 +327,95 @@ func TestFinishFeatureBranchKeepRemote(t *testing.T) { t.Errorf("Expected file content to be 'feature content', got '%s'", content) } } + +// TestFinishDeleteBranchUsesConfiguredRemote tests that branch deletion uses the configured remote name. +// Steps: +// 1. Sets up a test repository and initializes git-flow with defaults +// 2. Adds a remote named "upstream" (not "origin") +// 3. Configures gitflow.origin=upstream so git-flow uses the custom remote +// 4. Creates a feature branch and adds a commit +// 5. Publishes the feature branch to "upstream" +// 6. Runs 'git flow feature finish' +// 7. Verifies the remote branch is deleted from "upstream" +// 8. Verifies finish completes successfully +func TestFinishDeleteBranchUsesConfiguredRemote(t *testing.T) { + // Setup test repository + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + // Initialize git-flow with defaults + _, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v", err) + } + + // Add a remote named "upstream" (not "origin") + remoteDir, err := testutil.AddRemote(t, dir, "upstream", true) + if err != nil { + t.Fatalf("Failed to add upstream remote: %v", err) + } + defer testutil.CleanupTestRepo(t, remoteDir) + + // Configure git-flow to use "upstream" as the remote + _, err = testutil.RunGit(t, dir, "config", "gitflow.origin", "upstream") + if err != nil { + t.Fatalf("Failed to configure gitflow.origin: %v", err) + } + + // Create a feature branch + _, err = testutil.RunGitFlow(t, dir, "feature", "start", "custom-remote-test") + if err != nil { + t.Fatalf("Failed to create feature branch: %v", err) + } + + // Create a test file and commit + testutil.WriteFile(t, dir, "custom-remote-test.txt", "test content") + _, err = testutil.RunGit(t, dir, "add", "custom-remote-test.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Add custom remote test file") + if err != nil { + t.Fatalf("Failed to commit file: %v", err) + } + + // Publish the feature branch to "upstream" + _, err = testutil.RunGitFlow(t, dir, "feature", "publish", "custom-remote-test") + if err != nil { + t.Fatalf("Failed to publish feature branch: %v", err) + } + + // Verify the remote branch exists on "upstream" before finish + _, err = testutil.RunGit(t, dir, "fetch", "upstream") + if err != nil { + t.Fatalf("Failed to fetch from upstream: %v", err) + } + if !testutil.RemoteBranchExists(t, dir, "upstream", "feature/custom-remote-test") { + t.Fatal("Expected remote branch to exist on upstream before finish") + } + + // Finish the feature branch + output, err := testutil.RunGitFlow(t, dir, "feature", "finish", "custom-remote-test") + if err != nil { + t.Fatalf("Failed to finish feature branch: %v\nOutput: %s", err, output) + } + + // Verify finish completed successfully + if !strings.Contains(output, "Successfully finished") { + t.Error("Expected successful finish message") + } + + // Verify local branch is deleted + if testutil.BranchExists(t, dir, "feature/custom-remote-test") { + t.Error("Expected local feature branch to be deleted") + } + + // Fetch from upstream and verify remote branch is deleted + _, err = testutil.RunGit(t, dir, "fetch", "upstream", "--prune") + if err != nil { + t.Fatalf("Failed to fetch from upstream: %v", err) + } + if testutil.RemoteBranchExists(t, dir, "upstream", "feature/custom-remote-test") { + t.Error("Expected remote branch to be deleted from upstream") + } +}