localrepo: do not force push by default

localrepo's Push force pushes by default. This commit makes it an
optional behavior. Updates also the only call site of the function in
UpdateRemoteMirror to always push with force to keep the existing
behavior, until changed in a later commit.

3 files changed, 43 insertions, 24 deletions

diff --git a/internal/git/localrepo/remote.go b/internal/git/localrepo/remote.go
index bb659f6d2..be87ab495 100644
--- a/internal/git/localrepo/remote.go
+++ b/internal/git/localrepo/remote.go
@@ -283,6 +283,8 @@ type PushOptions struct {
 	// SSHCommand is the command line to use for git's SSH invocation. The command line is used
 	// as is and must be verified by the caller to be safe.
 	SSHCommand string
+	// Force decides whether to force push all of the refspecs.
+	Force bool
 	// Config is the Git configuration which gets passed to the git-push(1) invocation.
 	// Configuration is set up via `WithConfigEnv()`, so potential credentials won't be leaked
 	// via the command line.
@@ -300,11 +302,16 @@ func (repo *Repo) Push(ctx context.Context, remote string, refspecs []string, op
 		env = append(env, envGitSSHCommand(options.SSHCommand))
 	}
 
+	var flags []git.Option
+	if options.Force {
+		flags = append(flags, git.Flag{Name: "--force"})
+	}
+
 	stderr := &bytes.Buffer{}
 	if err := repo.ExecAndWait(ctx,
 		git.SubCmd{
 			Name:  "push",
-			Flags: []git.Option{git.Flag{Name: "--force"}},
+			Flags: flags,
 			Args:  append([]string{remote}, refspecs...),
 		},
 		git.WithStderr(stderr),
diff --git a/internal/git/localrepo/remote_test.go b/internal/git/localrepo/remote_test.go
index aa1d30e35..d653716d4 100644
--- a/internal/git/localrepo/remote_test.go
+++ b/internal/git/localrepo/remote_test.go
@@ -477,11 +477,33 @@ func TestRepo_Push(t *testing.T) {
 		return NewTestRepo(t, cfg, repoProto), repopath, nil
 	}
 
+	setupDivergedRepo := func(t testing.TB) (*Repo, string, []git.ConfigPair) {
+		repoProto, repoPath, _ := gittest.InitBareRepoAt(t, cfg, cfg.Storages[0])
+		repo := NewTestRepo(t, cfg, repoProto)
+
+		// set up master as a divergin ref in push repo
+		sourceMaster, err := sourceRepo.GetReference(ctx, "refs/heads/master")
+		require.NoError(t, err)
+
+		require.NoError(t, sourceRepo.Push(ctx, repoPath, []string{"refs/*"}, PushOptions{}))
+		divergedMaster := gittest.WriteCommit(t, cfg, repoPath,
+			gittest.WithBranch("master"),
+			gittest.WithParents(git.ObjectID(sourceMaster.Target)),
+		)
+
+		master, err := repo.GetReference(ctx, "refs/heads/master")
+		require.NoError(t, err)
+		require.Equal(t, master.Target, divergedMaster.String())
+
+		return repo, repoPath, nil
+	}
+
 	for _, tc := range []struct {
 		desc           string
 		setupPushRepo  func(testing.TB) (*Repo, string, []git.ConfigPair)
 		config         []git.ConfigPair
 		sshCommand     string
+		force          bool
 		refspecs       []string
 		errorMessage   string
 		expectedFilter []string
@@ -504,28 +526,16 @@ func TestRepo_Push(t *testing.T) {
 			expectedFilter: []string{"refs/heads/master"},
 		},
 		{
-			desc:     "force pushes over diverged refs",
-			refspecs: []string{"refs/heads/master"},
-			setupPushRepo: func(t testing.TB) (*Repo, string, []git.ConfigPair) {
-				repoProto, repoPath, _ := gittest.InitBareRepoAt(t, cfg, cfg.Storages[0])
-				repo := NewTestRepo(t, cfg, repoProto)
-
-				// set up master as a divergin ref in push repo
-				sourceMaster, err := sourceRepo.GetReference(ctx, "refs/heads/master")
-				require.NoError(t, err)
-
-				require.NoError(t, sourceRepo.Push(ctx, repoPath, []string{"refs/*"}, PushOptions{}))
-				divergedMaster := gittest.WriteCommit(t, cfg, repoPath,
-					gittest.WithBranch("master"),
-					gittest.WithParents(git.ObjectID(sourceMaster.Target)),
-				)
-
-				master, err := repo.GetReference(ctx, "refs/heads/master")
-				require.NoError(t, err)
-				require.Equal(t, master.Target, divergedMaster.String())
-
-				return repo, repoPath, nil
-			},
+			desc:          "doesn't force push over diverged refs with Force unset",
+			refspecs:      []string{"refs/heads/master"},
+			setupPushRepo: setupDivergedRepo,
+			errorMessage:  "Updates were rejected because the remote contains work that you do",
+		},
+		{
+			desc:          "force pushes over diverged refs with Force set",
+			refspecs:      []string{"refs/heads/master"},
+			force:         true,
+			setupPushRepo: setupDivergedRepo,
 		},
 		{
 			desc:          "push all refs",
@@ -564,10 +574,11 @@ func TestRepo_Push(t *testing.T) {
 
 			err := sourceRepo.Push(ctx, remote, tc.refspecs, PushOptions{
 				SSHCommand: tc.sshCommand,
+				Force:      tc.force,
 				Config:     remoteConfig,
 			})
 			if tc.errorMessage != "" {
-				require.EqualError(t, err, tc.errorMessage)
+				require.Contains(t, err.Error(), tc.errorMessage)
 				return
 			}
 			require.NoError(t, err)
diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go
index ff884fce6..96b29a647 100644
--- a/internal/gitaly/service/remote/update_remote_mirror.go
+++ b/internal/gitaly/service/remote/update_remote_mirror.go
@@ -197,6 +197,7 @@ func (s *server) updateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi
 		// we received in the original fetch. https://gitlab.com/gitlab-org/gitaly/-/issues/3505
 		if err := repo.Push(ctx, remoteName, batch, localrepo.PushOptions{
 			SSHCommand: sshCommand,
+			Force:      true,
 			Config:     remoteConfig,
 		}); err != nil {
 			return fmt.Errorf("push to mirror: %w", err)