diff options
author | John Cai <jcai@gitlab.com> | 2020-05-26 02:51:09 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2020-05-26 02:51:09 +0300 |
commit | 715f33073b49b9ed5fe0e90598954eef7ef318ca (patch) | |
tree | d0f5d7f921e4441249b7a878a9225436b37a83d7 | |
parent | a08561eebea15f32dd4217ca122ce0fc2364a2d6 (diff) | |
parent | 0899b440c91bb6952a3cae51a1fcd7a168851aff (diff) |
Merge branch 'rs-ls-remote-toggle' into 'master'
Remote branches via ls-remote is now a toggle
See merge request gitlab-org/gitaly!2183
-rw-r--r-- | changelogs/unreleased/rs-ls-remote-toggle.yml | 5 | ||||
-rw-r--r-- | internal/service/remote/update_remote_mirror_test.go | 415 | ||||
-rw-r--r-- | ruby/lib/gitlab/git/repository_mirroring.rb | 66 |
3 files changed, 310 insertions, 176 deletions
diff --git a/changelogs/unreleased/rs-ls-remote-toggle.yml b/changelogs/unreleased/rs-ls-remote-toggle.yml new file mode 100644 index 000000000..efc7fc59c --- /dev/null +++ b/changelogs/unreleased/rs-ls-remote-toggle.yml @@ -0,0 +1,5 @@ +--- +title: Remote branches via ls-remote is now a toggle +merge_request: 2183 +author: +type: changed diff --git a/internal/service/remote/update_remote_mirror_test.go b/internal/service/remote/update_remote_mirror_test.go index 618f5f13c..40225d754 100644 --- a/internal/service/remote/update_remote_mirror_test.go +++ b/internal/service/remote/update_remote_mirror_test.go @@ -19,6 +19,110 @@ func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) { client, conn := NewRemoteClient(t, serverSocketPath) defer conn.Close() + ctx, cancel := testhelper.Context() + defer cancel() + + for _, tt := range []struct { + name string + ctx context.Context + }{ + { + "ls-remote", + featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote), + }, + { + "fetch-remote", + ctx, + }, + } { + t.Run(tt.name, func(t *testing.T) { + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + _, mirrorPath, mirrorCleanupFn := testhelper.NewTestRepo(t) + defer mirrorCleanupFn() + + remoteName := "remote_mirror_1" + + testhelper.CreateTag(t, mirrorPath, "v0.0.1", "master", nil) // I needed another tag for the tests + testhelper.CreateTag(t, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98", nil) + testhelper.CreateTag(t, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9", &testhelper.CreateTagOpts{ + Message: "Overriding tag", Force: true}) + + setupCommands := [][]string{ + // Preconditions + {"config", "user.email", "gitalytest@example.com"}, + {"remote", "add", remoteName, mirrorPath}, + {"fetch", remoteName}, + // Updates + {"branch", "new-branch", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch + {"branch", "ignored-branch", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch not matching branch list + {"update-ref", "refs/heads/empty-branch", "0b4bc9a49b562e85de7cc9e834518ea6828729b9"}, // Update branch + {"branch", "-D", "not-merged-branch"}, // Delete branch + // Scoped to the project, so will be removed after + {"tag", "-d", "v0.0.1"}, // Delete tag + + // Catch bug https://gitlab.com/gitlab-org/gitaly/issues/1421 (reliance + // on 'HEAD' as the default branch). By making HEAD point to something + // invalid, we ensure this gets handled correctly. + {"symbolic-ref", "HEAD", "refs/does/not/exist"}, + {"tag", "--delete", "v1.1.0"}, // v1.1.0 is ambiguous, maps to a branch and a tag in gitlab-test repository + } + + for _, args := range setupCommands { + gitArgs := []string{"-C", testRepoPath} + gitArgs = append(gitArgs, args...) + testhelper.MustRunCommand(t, nil, "git", gitArgs...) + } + + newTagOid := string(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "v1.0.0")) + newTagOid = strings.TrimSpace(newTagOid) + require.NotEqual(t, newTagOid, "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8") // Sanity check that the tag did in fact change + + firstRequest := &gitalypb.UpdateRemoteMirrorRequest{ + Repository: testRepo, + RefName: remoteName, + OnlyBranchesMatching: nil, + } + matchingRequest1 := &gitalypb.UpdateRemoteMirrorRequest{ + OnlyBranchesMatching: [][]byte{[]byte("new-branch"), []byte("empty-branch")}, + } + matchingRequest2 := &gitalypb.UpdateRemoteMirrorRequest{ + OnlyBranchesMatching: [][]byte{[]byte("not-merged-branch"), []byte("matcher-without-matches")}, + } + + stream, err := client.UpdateRemoteMirror(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(firstRequest)) + require.NoError(t, stream.Send(matchingRequest1)) + require.NoError(t, stream.Send(matchingRequest2)) + + response, err := stream.CloseAndRecv() + require.NoError(t, err) + require.Empty(t, response.DivergentRefs) + + mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + + require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/new-branch") + require.NotContains(t, mirrorRefs, "refs/heads/ignored-branch") + require.Contains(t, mirrorRefs, "0b4bc9a49b562e85de7cc9e834518ea6828729b9 commit\trefs/heads/empty-branch") + require.NotContains(t, mirrorRefs, "refs/heads/not-merged-branch") + require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/tags/new-tag") + require.Contains(t, mirrorRefs, newTagOid+" tag\trefs/tags/v1.0.0") + require.NotContains(t, mirrorRefs, "refs/tags/v0.0.1") + require.Contains(t, mirrorRefs, "refs/heads/v1.1.0") + require.NotContains(t, mirrorRefs, "refs/tags/v1.1.0") + }) + } +} + +func TestSuccessfulUpdateRemoteMirrorRequestWithLsRemote(t *testing.T) { + serverSocketPath, stop := runRemoteServiceServer(t) + defer stop() + + client, conn := NewRemoteClient(t, serverSocketPath) + defer conn.Close() + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() @@ -32,18 +136,23 @@ func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) { testhelper.CreateTag(t, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9", &testhelper.CreateTagOpts{ Message: "Overriding tag", Force: true}) + // Create a commit that only exists in the mirror + mirrorOnlyCommitOid := testhelper.CreateCommit(t, mirrorPath, "master", nil) + require.NotEmpty(t, mirrorOnlyCommitOid) + setupCommands := [][]string{ // Preconditions {"config", "user.email", "gitalytest@example.com"}, {"remote", "add", remoteName, mirrorPath}, - {"fetch", remoteName}, + + // NOTE: We are explicitly *not* performing a fetch + // {"fetch", remoteName}, + // Updates {"branch", "new-branch", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch {"branch", "ignored-branch", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch not matching branch list {"update-ref", "refs/heads/empty-branch", "0b4bc9a49b562e85de7cc9e834518ea6828729b9"}, // Update branch {"branch", "-D", "not-merged-branch"}, // Delete branch - // Scoped to the project, so will be removed after - {"tag", "-d", "v0.0.1"}, // Delete tag // Catch bug https://gitlab.com/gitlab-org/gitaly/issues/1421 (reliance // on 'HEAD' as the default branch). By making HEAD point to something @@ -65,7 +174,7 @@ func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - // Ensure this flag doesn't alter existing behavior + // Enable the flag to use ls-remote ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote) firstRequest := &gitalypb.UpdateRemoteMirrorRequest{ @@ -86,11 +195,16 @@ func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) { require.NoError(t, stream.Send(matchingRequest1)) require.NoError(t, stream.Send(matchingRequest2)) - _, err = stream.CloseAndRecv() + response, err := stream.CloseAndRecv() require.NoError(t, err) + require.Empty(t, response.DivergentRefs) - mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + // Ensure the local repository still has no reference to the mirror-only commit + localRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "for-each-ref")) + require.NotContains(t, localRefs, mirrorOnlyCommitOid) + mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + require.Contains(t, mirrorRefs, mirrorOnlyCommitOid) require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/new-branch") require.NotContains(t, mirrorRefs, "refs/heads/ignored-branch") require.Contains(t, mirrorRefs, "0b4bc9a49b562e85de7cc9e834518ea6828729b9 commit\trefs/heads/empty-branch") @@ -109,80 +223,93 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithWildcards(t *testing.T) { client, conn := NewRemoteClient(t, serverSocketPath) defer conn.Close() - testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) - defer cleanupFn() - - _, mirrorPath, mirrorCleanupFn := testhelper.NewTestRepo(t) - defer mirrorCleanupFn() - - remoteName := "remote_mirror_2" - - setupCommands := [][]string{ - // Preconditions - {"config", "user.email", "gitalytest@example.com"}, - {"remote", "add", remoteName, mirrorPath}, - {"fetch", remoteName}, - // Updates - {"branch", "11-0-stable", "60ecb67744cb56576c30214ff52294f8ce2def98"}, - {"branch", "11-1-stable", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch - {"branch", "ignored-branch", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch not matching branch list - {"update-ref", "refs/heads/some-branch", "0b4bc9a49b562e85de7cc9e834518ea6828729b9"}, // Update branch - {"update-ref", "refs/heads/feature", "0b4bc9a49b562e85de7cc9e834518ea6828729b9"}, // Update branch - // Scoped to the project, so will be removed after - {"branch", "-D", "not-merged-branch"}, // Delete branch - {"tag", "--delete", "v1.1.0"}, // v1.1.0 is ambiguous, maps to a branch and a tag in gitlab-test repository - } - - testhelper.CreateTag(t, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98", nil) // Add tag - testhelper.CreateTag(t, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9", - &testhelper.CreateTagOpts{Message: "Overriding tag", Force: true}) // Update tag - - for _, args := range setupCommands { - gitArgs := []string{"-C", testRepoPath} - gitArgs = append(gitArgs, args...) - testhelper.MustRunCommand(t, nil, "git", gitArgs...) - } - - // Workaround for https://gitlab.com/gitlab-org/gitaly/issues/1439 - // Create a tag on the remote to ensure it gets deleted later - testhelper.CreateTag(t, mirrorPath, "v1.2.0", "master", nil) - - newTagOid := string(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "v1.0.0")) - newTagOid = strings.TrimSpace(newTagOid) - require.NotEqual(t, newTagOid, "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8") // Sanity check that the tag did in fact change - ctx, cancel := testhelper.Context() defer cancel() - // Ensure this flag doesn't alter existing behavior - ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote) - - firstRequest := &gitalypb.UpdateRemoteMirrorRequest{ - Repository: testRepo, - RefName: remoteName, - OnlyBranchesMatching: [][]byte{[]byte("*-stable"), []byte("feature")}, - } - - stream, err := client.UpdateRemoteMirror(ctx) - require.NoError(t, err) - require.NoError(t, stream.Send(firstRequest)) - - _, err = stream.CloseAndRecv() - require.NoError(t, err) + for _, tt := range []struct { + name string + ctx context.Context + }{ + { + "ls-remote", + featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote), + }, + { + "fetch-remote", + ctx, + }, + } { + t.Run(tt.name, func(t *testing.T) { + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + _, mirrorPath, mirrorCleanupFn := testhelper.NewTestRepo(t) + defer mirrorCleanupFn() + + remoteName := "remote_mirror_2" + + setupCommands := [][]string{ + // Preconditions + {"config", "user.email", "gitalytest@example.com"}, + {"remote", "add", remoteName, mirrorPath}, + {"fetch", remoteName}, + // Updates + {"branch", "11-0-stable", "60ecb67744cb56576c30214ff52294f8ce2def98"}, + {"branch", "11-1-stable", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch + {"branch", "ignored-branch", "60ecb67744cb56576c30214ff52294f8ce2def98"}, // Add branch not matching branch list + {"update-ref", "refs/heads/some-branch", "0b4bc9a49b562e85de7cc9e834518ea6828729b9"}, // Update branch + {"update-ref", "refs/heads/feature", "0b4bc9a49b562e85de7cc9e834518ea6828729b9"}, // Update branch + // Scoped to the project, so will be removed after + {"branch", "-D", "not-merged-branch"}, // Delete branch + {"tag", "--delete", "v1.1.0"}, // v1.1.0 is ambiguous, maps to a branch and a tag in gitlab-test repository + } + + testhelper.CreateTag(t, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98", nil) // Add tag + testhelper.CreateTag(t, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9", + &testhelper.CreateTagOpts{Message: "Overriding tag", Force: true}) // Update tag + + for _, args := range setupCommands { + gitArgs := []string{"-C", testRepoPath} + gitArgs = append(gitArgs, args...) + testhelper.MustRunCommand(t, nil, "git", gitArgs...) + } + + // Workaround for https://gitlab.com/gitlab-org/gitaly/issues/1439 + // Create a tag on the remote to ensure it gets deleted later + testhelper.CreateTag(t, mirrorPath, "v1.2.0", "master", nil) + + newTagOid := string(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "v1.0.0")) + newTagOid = strings.TrimSpace(newTagOid) + require.NotEqual(t, newTagOid, "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8") // Sanity check that the tag did in fact change + firstRequest := &gitalypb.UpdateRemoteMirrorRequest{ + Repository: testRepo, + RefName: remoteName, + OnlyBranchesMatching: [][]byte{[]byte("*-stable"), []byte("feature")}, + } - mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + stream, err := client.UpdateRemoteMirror(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(firstRequest)) - require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/11-0-stable") - require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/11-1-stable") - require.Contains(t, mirrorRefs, "0b4bc9a49b562e85de7cc9e834518ea6828729b9 commit\trefs/heads/feature") - require.NotContains(t, mirrorRefs, "refs/heads/ignored-branch") - require.NotContains(t, mirrorRefs, "refs/heads/some-branch") - require.Contains(t, mirrorRefs, "refs/heads/not-merged-branch") - require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/tags/new-tag") - require.Contains(t, mirrorRefs, newTagOid+" tag\trefs/tags/v1.0.0") - require.NotContains(t, mirrorRefs, "refs/tags/v1.2.0") - require.Contains(t, mirrorRefs, "refs/heads/v1.1.0") - require.NotContains(t, mirrorRefs, "refs/tags/v1.1.0") + response, err := stream.CloseAndRecv() + require.NoError(t, err) + require.Empty(t, response.DivergentRefs) + + mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + + require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/11-0-stable") + require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/11-1-stable") + require.Contains(t, mirrorRefs, "0b4bc9a49b562e85de7cc9e834518ea6828729b9 commit\trefs/heads/feature") + require.NotContains(t, mirrorRefs, "refs/heads/ignored-branch") + require.NotContains(t, mirrorRefs, "refs/heads/some-branch") + require.Contains(t, mirrorRefs, "refs/heads/not-merged-branch") + require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/tags/new-tag") + require.Contains(t, mirrorRefs, newTagOid+" tag\trefs/tags/v1.0.0") + require.NotContains(t, mirrorRefs, "refs/tags/v1.2.0") + require.Contains(t, mirrorRefs, "refs/heads/v1.1.0") + require.NotContains(t, mirrorRefs, "refs/tags/v1.1.0") + }) + } } func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T) { @@ -192,83 +319,95 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T) client, conn := NewRemoteClient(t, serverSocketPath) defer conn.Close() - testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) - defer cleanupFn() - - _, mirrorPath, mirrorCleanupFn := testhelper.NewTestRepo(t) - defer mirrorCleanupFn() - - remoteName := "remote_mirror_1" - - testhelper.CreateTag(t, mirrorPath, "v2.0.0", "master", nil) - - setupCommands := [][]string{ - // Preconditions - {"config", "user.email", "gitalytest@example.com"}, - {"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"}, - } - - for _, args := range setupCommands { - gitArgs := []string{"-C", testRepoPath} - gitArgs = append(gitArgs, args...) - testhelper.MustRunCommand(t, nil, "git", gitArgs...) - } - ctx, cancel := testhelper.Context() defer cancel() - // Ensure this flag doesn't alter existing behavior - ctx = featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote) - - firstRequest := &gitalypb.UpdateRemoteMirrorRequest{ - Repository: testRepo, - RefName: remoteName, - KeepDivergentRefs: true, - } + for _, tt := range []struct { + name string + ctx context.Context + }{ + { + "ls-remote", + featureflag.OutgoingCtxWithFeatureFlag(ctx, featureflag.RemoteBranchesLsRemote), + }, + { + "fetch-remote", + ctx, + }, + } { + t.Run(tt.name, func(t *testing.T) { + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) + defer cleanupFn() + + _, mirrorPath, mirrorCleanupFn := testhelper.NewTestRepo(t) + defer mirrorCleanupFn() + + remoteName := "remote_mirror_1" + + testhelper.CreateTag(t, mirrorPath, "v2.0.0", "master", nil) + + setupCommands := [][]string{ + // Preconditions + {"config", "user.email", "gitalytest@example.com"}, + {"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"}, + } + + for _, args := range setupCommands { + gitArgs := []string{"-C", testRepoPath} + gitArgs = append(gitArgs, args...) + testhelper.MustRunCommand(t, nil, "git", gitArgs...) + } + firstRequest := &gitalypb.UpdateRemoteMirrorRequest{ + Repository: testRepo, + RefName: remoteName, + KeepDivergentRefs: true, + } - stream, err := client.UpdateRemoteMirror(ctx) - require.NoError(t, err) - require.NoError(t, stream.Send(firstRequest)) + stream, err := client.UpdateRemoteMirror(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(firstRequest)) - response, err := stream.CloseAndRecv() - require.NoError(t, err) - require.ElementsMatch(t, response.DivergentRefs, [][]byte{[]byte("refs/heads/master")}) + response, err := stream.CloseAndRecv() + require.NoError(t, err) + require.ElementsMatch(t, response.DivergentRefs, [][]byte{[]byte("refs/heads/master")}) - mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + 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 `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") + // 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 + // Re-run mirroring without KeepDivergentRefs + firstRequest.KeepDivergentRefs = false - stream, err = client.UpdateRemoteMirror(ctx) - require.NoError(t, err) - require.NoError(t, stream.Send(firstRequest)) + stream, err = client.UpdateRemoteMirror(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(firstRequest)) - _, err = stream.CloseAndRecv() - require.NoError(t, err) + _, err = stream.CloseAndRecv() + require.NoError(t, err) - mirrorRefs = string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref")) + 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 `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") + // 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/repository_mirroring.rb b/ruby/lib/gitlab/git/repository_mirroring.rb index 0e7f9f15b..aab59b120 100644 --- a/ruby/lib/gitlab/git/repository_mirroring.rb +++ b/ruby/lib/gitlab/git/repository_mirroring.rb @@ -13,37 +13,24 @@ module Gitlab }.freeze def remote_branches(remote_name, env:) - branches = [] - - rugged.references.each("refs/remotes/#{remote_name}/*").map do |ref| - name = ref.name.sub(%r{\Arefs/remotes/#{remote_name}/}, '') - - begin - target_commit = Gitlab::Git::Commit.find(self, ref.target.oid) - branches << Gitlab::Git::Branch.new(self, name, ref.target, target_commit) - rescue Rugged::ReferenceError - # Omit invalid branch - end - end - - # Run an experiment to see if ls-remote gives us the same data - # - # See https://gitlab.com/gitlab-org/gitaly/-/issues/2670 if feature_enabled?(:remote_branches_ls_remote) - ls_remote_branches = experimental_remote_branches(remote_name, env: env) - - control_refs = branches.collect(&:name) - experiment_refs = ls_remote_branches.collect(&:name) - - diff = experiment_refs.difference(control_refs).map { |r| "+#{r}" } - diff.concat(control_refs.difference(experiment_refs).map { |r| "-#{r}" }) - - if diff.any? - Rails.logger.warn("experimental_remote_branches returned differing values from control: #{diff.join(', ')}") + experimental_remote_branches(remote_name, env: env) + else + branches = [] + + rugged.references.each("refs/remotes/#{remote_name}/*").map do |ref| + name = ref.name.sub(%r{\Arefs/remotes/#{remote_name}/}, '') + + begin + target_commit = Gitlab::Git::Commit.find(self, ref.target.oid) + branches << Gitlab::Git::Branch.new(self, name, ref.target, target_commit) + rescue Rugged::ReferenceError + # Omit invalid branch + end end - end - branches + branches + end end def push_remote_branches(remote_name, branch_names, forced: true, env: {}) @@ -125,20 +112,23 @@ module Gitlab end def list_remote_refs(remote, env:) - ref_list, exit_code, error = nil + @list_remote_refs ||= {} + @list_remote_refs[remote] ||= begin + ref_list, exit_code, error = nil - # List heads and tags, ignoring stuff like `refs/merge-requests` and `refs/pull` - cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --heads --tags #{remote}] + # List heads and tags, ignoring stuff like `refs/merge-requests` and `refs/pull` + cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --heads --tags #{remote}] - Open3.popen3(env, *cmd) do |_stdin, stdout, stderr, wait_thr| - ref_list = stdout.read - error = stderr.read - exit_code = wait_thr.value.exitstatus - end + Open3.popen3(env, *cmd) do |_stdin, stdout, stderr, wait_thr| + ref_list = stdout.read + error = stderr.read + exit_code = wait_thr.value.exitstatus + end - raise RemoteError, error unless exit_code.zero? + raise RemoteError, error unless exit_code.zero? - ref_list.each_line(chomp: true) + ref_list.each_line(chomp: true) + end end end end |