From 1d303e12a89fabf949855f02e4b893e0ed1cc407 Mon Sep 17 00:00:00 2001 From: Kate Travers <8152930+ktravers@users.noreply.github.com> Date: Sat, 17 Oct 2026 04:50:55 +0000 Subject: [PATCH 1/2] first pass on including pull.stacked option on push --- cmd/rebase_test.go | 2 ++ internal/git/gitops.go | 3 ++- internal/git/gitops_test.go | 43 +++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 8feda3c..0172600 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -2273,6 +2273,7 @@ func setupAmendedParentRepo(t *testing.T, corruptBase bool) amendedParentRepo { cloneDir := filepath.Join(t.TempDir(), "clone") issue250Git(t, ".", "-c", "safe.bareRepository=all", "init", "--bare", "-b", "main", remoteDir) + issue250Git(t, ".", "-c", "safe.bareRepository=all", "-C", remoteDir, "config", "receive.advertisePushOptions", "true") issue250Git(t, ".", "clone", remoteDir, cloneDir) issue250Git(t, cloneDir, "config", "user.name", "Test") issue250Git(t, cloneDir, "config", "user.email", "test@example.com") @@ -2410,6 +2411,7 @@ func TestIntegration_AdoptedBranchRebasesFromCommonAncestor(t *testing.T) { cloneDir := filepath.Join(t.TempDir(), "clone") issue250Git(t, ".", "-c", "safe.bareRepository=all", "init", "--bare", "-b", "main", remoteDir) + issue250Git(t, ".", "-c", "safe.bareRepository=all", "-C", remoteDir, "config", "receive.advertisePushOptions", "true") issue250Git(t, ".", "clone", remoteDir, cloneDir) issue250Git(t, cloneDir, "config", "user.name", "Test") issue250Git(t, cloneDir, "config", "user.email", "test@example.com") diff --git a/internal/git/gitops.go b/internal/git/gitops.go index 1f37c16..4a04a9e 100644 --- a/internal/git/gitops.go +++ b/internal/git/gitops.go @@ -254,8 +254,9 @@ func (d *defaultOps) CreateBranch(name, base string) error { return d.runSilent("branch", name, base) } +// TODO send pull.stacked option on force push after rebase only? func (d *defaultOps) Push(remote string, branches []string, force, atomic bool) error { - args := []string{"push", remote} + args := []string{"push", "-o", "pull.stacked", remote} if force { // Build explicit per-branch leases and refspecs. This removes // dependence on push.default / upstream configuration and diff --git a/internal/git/gitops_test.go b/internal/git/gitops_test.go index 0966112..4d55645 100644 --- a/internal/git/gitops_test.go +++ b/internal/git/gitops_test.go @@ -63,6 +63,10 @@ func setupBareAndClone(t *testing.T) (string, string) { out, err := cmd.CombinedOutput() require.NoError(t, err, "git init --bare: %s", string(out)) + cfgCmd := exec.Command("git", "-c", "safe.bareRepository=all", "-C", bareDir, "config", "receive.advertisePushOptions", "true") + out, err = cfgCmd.CombinedOutput() + require.NoError(t, err, "git config: %s", string(out)) + cloneDir := filepath.Join(t.TempDir(), "clone") gitExec(t, ".", "clone", bareDir, cloneDir) @@ -1703,3 +1707,42 @@ func TestIntegration_WorktreeIgnoresCallerRepositoryEnvironment(t *testing.T) { require.NoError(t, err) assert.Equal(t, "feature", branch) } + +// Every push must send the "pull.stacked" push option, for both forced and atomic pushes. +// TODO send option on force push rebase only? +func TestIntegration_Push_SendsStackPushOption(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses a shell pre-receive hook") + } + + for _, tc := range []struct { + name string + force, atomic bool + }{ + {"force", true, false}, + {"atomic", false, true}, + {"plain", false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + bareDir, cloneDir := setupBareAndClone(t) + restore := withGitDir(t, cloneDir) + defer restore() + + optsFile := filepath.Join(t.TempDir(), "opts") + hook := fmt.Sprintf("#!/bin/sh\nenv | grep '^GIT_PUSH_OPTION' | sort > %q\n", optsFile) + require.NoError(t, os.WriteFile(filepath.Join(bareDir, "hooks", "pre-receive"), []byte(hook), 0o755)) + + gitExec(t, cloneDir, "checkout", "-b", "b1") + writeFile(t, cloneDir, "b1.txt", "v1") + gitExec(t, cloneDir, "add", ".") + gitExec(t, cloneDir, "commit", "-m", "b1") + + d := &defaultOps{} + require.NoError(t, d.Push("origin", []string{"b1"}, tc.force, tc.atomic)) + + got, err := os.ReadFile(optsFile) + require.NoError(t, err) + assert.Equal(t, "GIT_PUSH_OPTION_0=pull.stacked\nGIT_PUSH_OPTION_COUNT=1\n", string(got)) + }) + } +} From 91dbf5c98d2f1d4e9978cf0310a13f5324396ebf Mon Sep 17 00:00:00 2001 From: Kate Travers <8152930+ktravers@users.noreply.github.com> Date: Sat, 17 Oct 2026 04:56:50 +0000 Subject: [PATCH 2/2] add fallback for remotes that don't accept push options --- internal/git/gitops.go | 11 +++++++++-- internal/git/gitops_test.go | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/internal/git/gitops.go b/internal/git/gitops.go index 4a04a9e..8575007 100644 --- a/internal/git/gitops.go +++ b/internal/git/gitops.go @@ -256,7 +256,7 @@ func (d *defaultOps) CreateBranch(name, base string) error { // TODO send pull.stacked option on force push after rebase only? func (d *defaultOps) Push(remote string, branches []string, force, atomic bool) error { - args := []string{"push", "-o", "pull.stacked", remote} + args := []string{"push", remote} if force { // Build explicit per-branch leases and refspecs. This removes // dependence on push.default / upstream configuration and @@ -287,7 +287,14 @@ func (d *defaultOps) Push(remote string, branches []string, force, atomic bool) for _, b := range branches { args = append(args, fmt.Sprintf("refs/heads/%s:refs/heads/%s", b, b)) } - return d.runSilent(args...) + // Send the "pull.stacked" push option, falling back to a plain push for + // remotes that don't support push options. + withOpt := append([]string{"push", "-o", "pull.stacked"}, args[1:]...) + err := d.runSilent(withOpt...) + if err != nil && strings.Contains(err.Error(), "does not support push options") { + return d.runSilent(args...) + } + return err } // ResolveRemote determines the remote for pushing a branch. It checks git diff --git a/internal/git/gitops_test.go b/internal/git/gitops_test.go index 4d55645..c8a25bb 100644 --- a/internal/git/gitops_test.go +++ b/internal/git/gitops_test.go @@ -1746,3 +1746,23 @@ func TestIntegration_Push_SendsStackPushOption(t *testing.T) { }) } } + +// Remotes that don't advertise push options must still accept pushes. +func TestIntegration_Push_FallsBackWithoutPushOptionSupport(t *testing.T) { + bareDir, cloneDir := setupBareAndClone(t) + restore := withGitDir(t, cloneDir) + defer restore() + + out, err := exec.Command("git", "-c", "safe.bareRepository=all", "-C", bareDir, + "config", "receive.advertisePushOptions", "false").CombinedOutput() + require.NoError(t, err, string(out)) + + gitExec(t, cloneDir, "checkout", "-b", "b1") + writeFile(t, cloneDir, "b1.txt", "v1") + gitExec(t, cloneDir, "add", ".") + gitExec(t, cloneDir, "commit", "-m", "b1") + + d := &defaultOps{} + require.NoError(t, d.Push("origin", []string{"b1"}, true, true)) + assert.Equal(t, gitExec(t, cloneDir, "rev-parse", "b1"), remoteBranchSHA(t, bareDir, "b1")) +}