From b012da20fe72d15afebfd4d5d9b10cef1548d3f4 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 12 Mar 2020 13:47:59 -0500
Subject: [PATCH] Don't push divergent remote branches with keep_divergent_refs

Previously, if a remote branch had completely diverged from the source
(i.e., it is no longer an ancestor of the source), we'd still attempt to
push to it and would only realize it failed when the server came back
with an error.

Now when selecting local branches to push to the remote, the
`keep_divergent_refs` option allows us to limit the list to only
branches that have no divergence.
---
 .../rs-keep-divergent-refs-for-push.yml       |  5 ++++
 .../remote/update_remote_mirror_test.go       | 26 +++++++++++++++++++
 ruby/lib/gitlab/git/remote_mirror.rb          | 18 ++++++++++++-
 3 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/rs-keep-divergent-refs-for-push.yml

diff --git a/changelogs/unreleased/rs-keep-divergent-refs-for-push.yml b/changelogs/unreleased/rs-keep-divergent-refs-for-push.yml
new file mode 100644
index 0000000000..d736230ac5
--- /dev/null
+++ b/changelogs/unreleased/rs-keep-divergent-refs-for-push.yml
@@ -0,0 +1,5 @@
+---
+title: Don't push divergent remote branches with keep_divergent_refs
+merge_request: 1915
+author:
+type: added
diff --git a/internal/service/remote/update_remote_mirror_test.go b/internal/service/remote/update_remote_mirror_test.go
index 87e51eb8cd..0cb8524a93 100644
--- a/internal/service/remote/update_remote_mirror_test.go
+++ b/internal/service/remote/update_remote_mirror_test.go
@@ -195,6 +195,10 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T)
 		{"remote", "add", remoteName, mirrorPath},
 		{"fetch", remoteName},
 
+		// Create a divergence by moving `master` to the HEAD of another branch
+		// ba3faa7d only exists on `after-create-delete-modify-move`
+		{"update-ref", "refs/heads/master", "ba3faa7dbecdb555c748b36e8bc0f427e69de5e7"},
+
 		// Delete a branch and a tag to ensure they're kept around in the mirror
 		{"branch", "-D", "not-merged-branch"},
 		{"tag", "-d", "v2.0.0"},
@@ -224,8 +228,30 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T)
 
 	mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref"))
 
+	// Verify `master` didn't get updated, since its HEAD is no longer an ancestor of remote's version
+	require.Contains(t, mirrorRefs, "1e292f8fedd741b75372e19097c76d327140c312 commit\trefs/heads/master")
+
+	// Verify refs deleted on the source stick around on the mirror
 	require.Contains(t, mirrorRefs, "refs/heads/not-merged-branch")
 	require.Contains(t, mirrorRefs, "refs/tags/v2.0.0")
+
+	// Re-run mirroring without KeepDivergentRefs
+	firstRequest.KeepDivergentRefs = false
+
+	stream, err = client.UpdateRemoteMirror(ctx)
+	require.NoError(t, err)
+	require.NoError(t, stream.Send(firstRequest))
+
+	_, err = stream.CloseAndRecv()
+	require.NoError(t, err)
+
+	mirrorRefs = string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref"))
+
+	// Verify `master` gets overwritten with the value from the source
+	require.Contains(t, mirrorRefs, "ba3faa7dbecdb555c748b36e8bc0f427e69de5e7 commit\trefs/heads/master")
+
+	// Verify a branch only on the mirror is now deleted
+	require.NotContains(t, mirrorRefs, "refs/heads/not-merged-branch")
 }
 
 func TestFailedUpdateRemoteMirrorRequestDueToValidation(t *testing.T) {
diff --git a/ruby/lib/gitlab/git/remote_mirror.rb b/ruby/lib/gitlab/git/remote_mirror.rb
index 9311a8b256..2ad02bb261 100644
--- a/ruby/lib/gitlab/git/remote_mirror.rb
+++ b/ruby/lib/gitlab/git/remote_mirror.rb
@@ -60,7 +60,23 @@ module Gitlab
         local_refs.select do |ref_name, ref|
           remote_ref = remote_refs[ref_name]
 
-          remote_ref.nil? || ref.dereferenced_target != remote_ref.dereferenced_target
+          # Ref doesn't exist on the remote, it should be created
+          next true if remote_ref.nil?
+
+          local_target = ref.dereferenced_target
+          remote_target = remote_ref.dereferenced_target
+
+          if local_target == remote_target
+            # Ref is identical on the remote, no point mirroring
+            false
+          elsif @keep_divergent_refs
+            # Mirror the ref if its remote counterpart hasn't diverged
+            repository.ancestor?(remote_target&.id, local_target&.id)
+          else
+            # Attempt to overwrite whatever's on the remote; push rules and
+            # protected branches may still prevent this
+            true
+          end
         end
       end
 
-- 
GitLab