Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 12 additions & 24 deletions cmd/finish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix. The same treatment is needed one case above: handleCreateTagStep(state, resolvedOptions) at line 353 still calls config.LoadConfig() internally with an "origin" fallback (lines 679-684). Since cfg is right here, thread it through the same way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Alex - its updated now to pass the config parameter.

default:
return &errors.GitError{Operation: fmt.Sprintf("unknown step '%s'", state.CurrentStep), Err: nil}
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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}
Expand All @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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}
}
}
Expand Down
2 changes: 1 addition & 1 deletion docs/git-flow-finish.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<type>.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.<type>.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.<type>.finish.fetch`.
Expand Down
7 changes: 7 additions & 0 deletions internal/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions test/cmd/finish_fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
93 changes: 93 additions & 0 deletions test/cmd/finish_retention_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"os/exec"
"path/filepath"
"strings"
"testing"

"github.com/gittower/git-flow-next/test/testutil"
Expand Down Expand Up @@ -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")
}
}