Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 175667d

Browse files
eseligereseliger
authored
gitserver: Add OctopusMergeBase RPC method (#63842)
This method can be used to find a common ancestor for many commit SHAs, to be used by code intel for finding visible uploads. Closes SRC-485 Test plan: Added unit tests for the several layers (client, grpc, gitcli).
1 parent 36d785d commit 175667d

File tree

21 files changed

+1645
-402
lines changed

21 files changed

+1645
-402
lines changed

‎cmd/gitserver/internal/git/gitcli/exec.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ var (
2727
"ls-files": {"--with-tree", "-z"},
2828
"for-each-ref": {"--format", "--points-at", "--contains", "--sort", "-creatordate", "-refname", "-HEAD"},
2929
"tag": {"--list", "--sort", "-creatordate", "--format", "--points-at"},
30-
"merge-base": {"--"},
30+
"merge-base": {"--octopus", "--"},
3131
"show-ref": {"--heads"},
3232
"shortlog": {"--summary", "--numbered", "--email", "--no-merges", "--after", "--before"},
3333
"cat-file": {"-p", "-t"},

‎cmd/gitserver/internal/git/gitcli/mergebase.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/sourcegraph/sourcegraph/internal/api"
99
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
10+
"github.com/sourcegraph/sourcegraph/internal/lazyregexp"
1011
"github.com/sourcegraph/sourcegraph/lib/errors"
1112
)
1213

@@ -47,3 +48,52 @@ func (g *gitCLIBackend) MergeBase(ctx context.Context, baseRevspec, headRevspec
4748

4849
return api.CommitID(bytes.TrimSpace(stdout)), nil
4950
}
51+
52+
func (g *gitCLIBackend) MergeBaseOctopus(ctx context.Context, revspecs ...string) (api.CommitID, error) {
53+
if len(revspecs) < 2 {
54+
return "", errors.New("at least two revspecs must be given")
55+
}
56+
57+
args := make([]string, 0, len(revspecs)+3)
58+
args = append(args, "merge-base", "--octopus", "--")
59+
args = append(args, revspecs...)
60+
61+
out, err := g.NewCommand(
62+
ctx,
63+
WithArguments(args...),
64+
)
65+
if err != nil {
66+
return "", err
67+
}
68+
69+
defer out.Close()
70+
71+
stdout, err := io.ReadAll(out)
72+
if err != nil {
73+
// Exit code 1 and empty output most likely means that no common merge-base was found.
74+
var e *commandFailedError
75+
if errors.As(err, &e) {
76+
if e.ExitStatus == 1 {
77+
if len(e.Stderr) == 0 {
78+
return "", nil
79+
}
80+
} else if e.ExitStatus == 128 && bytes.Contains(e.Stderr, []byte("fatal: Not a valid object name")) {
81+
p := octopusNotAValidObjectRegexp.FindSubmatch(e.Stderr)
82+
var spec string
83+
if len(p) > 0 {
84+
spec = string(p[1])
85+
}
86+
return "", &gitdomain.RevisionNotFoundError{
87+
Repo: g.repoName,
88+
Spec: spec,
89+
}
90+
}
91+
}
92+
93+
return "", err
94+
}
95+
96+
return api.CommitID(bytes.TrimSpace(stdout)), nil
97+
}
98+
99+
var octopusNotAValidObjectRegexp = lazyregexp.New(`fatal: Not a valid object name ([^\s]+)`)

‎cmd/gitserver/internal/git/gitcli/mergebase_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,99 @@ func TestGitCLIBackend_MergeBase(t *testing.T) {
7474
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
7575
})
7676
}
77+
78+
func TestGitCLIBackend_MergeBaseOctopus(t *testing.T) {
79+
ctx := context.Background()
80+
81+
t.Run("resolves", func(t *testing.T) {
82+
// Prepare repo state:
83+
// Structure:
84+
// 1 - 5 master
85+
// \ - 2 b2
86+
// \ - 3 - 4 b3
87+
// Expected merge base of {master, b2, b3}: 1 (aka testbase)
88+
backend := BackendWithRepoCommands(t,
89+
"echo line1 > f",
90+
"git add f",
91+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
92+
"git tag testbase",
93+
94+
"git checkout -b b2",
95+
"echo line2 >> f",
96+
"git add f",
97+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
98+
"git checkout master",
99+
100+
"git checkout -b b3",
101+
"echo line3 >> f",
102+
"git add f",
103+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
104+
"echo line4 >> f",
105+
"git add f",
106+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
107+
108+
"git checkout master",
109+
"echo line2 > f",
110+
"git add f",
111+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
112+
)
113+
114+
wantSHA, err := backend.ResolveRevision(ctx, "testbase")
115+
require.NoError(t, err)
116+
require.NotEmpty(t, wantSHA)
117+
118+
base, err := backend.MergeBaseOctopus(ctx, "master", "b2", "b3")
119+
require.NoError(t, err)
120+
require.Equal(t, wantSHA, base)
121+
})
122+
t.Run("orphan branches", func(t *testing.T) {
123+
// Prepare repo state:
124+
backend := BackendWithRepoCommands(t,
125+
"echo line1 > f",
126+
"git add f",
127+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
128+
"git checkout --orphan b2",
129+
"echo line2 >> f",
130+
"git add f",
131+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
132+
"git checkout master",
133+
"git checkout --orphan b3",
134+
"echo line3 >> f",
135+
"git add f",
136+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
137+
"git checkout master",
138+
)
139+
140+
base, err := backend.MergeBaseOctopus(ctx, "master", "b2", "b3")
141+
require.NoError(t, err)
142+
require.Equal(t, api.CommitID(""), base)
143+
})
144+
t.Run("not found revspec", func(t *testing.T) {
145+
// Prepare repo state:
146+
backend := BackendWithRepoCommands(t,
147+
"echo line1 > f",
148+
"git add f",
149+
"git commit -m foo --author='Foo Author <foo@sourcegraph.com>'",
150+
)
151+
152+
// Last revspec not found
153+
_, err := backend.MergeBaseOctopus(ctx, "master", "notfound")
154+
require.Error(t, err)
155+
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
156+
require.Equal(t, "notfound", err.(*gitdomain.RevisionNotFoundError).Spec)
157+
158+
// First revspec not found
159+
_, err = backend.MergeBaseOctopus(ctx, "notfound", "master")
160+
require.Error(t, err)
161+
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
162+
require.Equal(t, "notfound", err.(*gitdomain.RevisionNotFoundError).Spec)
163+
})
164+
t.Run("less than two revspecs", func(t *testing.T) {
165+
// Prepare repo state:
166+
backend := BackendWithRepoCommands(t)
167+
168+
_, err := backend.MergeBaseOctopus(ctx, "master")
169+
require.Error(t, err)
170+
require.ErrorContains(t, err, "at least two revspecs must be given")
171+
})
172+
}

‎cmd/gitserver/internal/git/iface.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,25 @@ type GitBackend interface {
162162
// This value can be used to determine if a repository changed since the last
163163
// time the hash has been computed.
164164
RefHash(ctx context.Context) ([]byte, error)
165+
166+
// MergeBaseOctopus returns the octopus merge base commit sha for the specified
167+
// revspecs.
168+
// If no common merge base exists, an empty string is returned.
169+
// See the following diagrams from git-merge-base docs on what octopus merge bases
170+
// are:
171+
// Given three commits A, B, and C, MergeBaseOctopus(A, B, C) will compute the
172+
// best common ancestor of all commits.
173+
// For example, with this topology:
174+
// o---o---o---o---C
175+
// /
176+
// / o---o---o---B
177+
// / /
178+
// ---2---1---o---o---o---A
179+
// The result of MergeBaseOctopus(A, B, C) is 2, because 2 is the
180+
// best common ancestor of all commits.
181+
//
182+
// If one of the given revspecs does not exist, a RevisionNotFoundError is returned.
183+
MergeBaseOctopus(ctx context.Context, revspecs ...string) (api.CommitID, error)
165184
}
166185

167186
// CommitLogOrder is the order of the commits returned by CommitLog.

‎cmd/gitserver/internal/git/mock.go‎

Lines changed: 131 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cmd/gitserver/internal/git/observability.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,16 @@ func (b *observableBackend) MergeBase(ctx context.Context, baseRevspec, headRevs
112112
return b.backend.MergeBase(ctx, baseRevspec, headRevspec)
113113
}
114114

115+
func (b *observableBackend) MergeBaseOctopus(ctx context.Context, revspecs ...string) (_ api.CommitID, err error) {
116+
ctx, _, endObservation := b.operations.mergeBaseOctopus.With(ctx, &err, observation.Args{})
117+
defer endObservation(1, observation.Args{})
118+
119+
concurrentOps.WithLabelValues("MergeBaseOctopus").Inc()
120+
defer concurrentOps.WithLabelValues("MergeBaseOctopus").Dec()
121+
122+
return b.backend.MergeBaseOctopus(ctx, revspecs...)
123+
}
124+
115125
func (b *observableBackend) Blame(ctx context.Context, commit api.CommitID, path string, opt BlameOptions) (_ BlameHunkReader, err error) {
116126
ctx, errCollector, endObservation := b.operations.blame.WithErrors(ctx, &err, observation.Args{})
117127
ctx, cancel := context.WithCancel(ctx)
@@ -536,6 +546,7 @@ type operations struct {
536546
latestCommitTimestamp *observation.Operation
537547
refHash *observation.Operation
538548
commitLog *observation.Operation
549+
mergeBaseOctopus *observation.Operation
539550
}
540551

541552
func newOperations(observationCtx *observation.Context) *operations {
@@ -588,6 +599,7 @@ func newOperations(observationCtx *observation.Context) *operations {
588599
latestCommitTimestamp: op("latest-commit-timestamp"),
589600
refHash: op("ref-hash"),
590601
commitLog: op("commit-log"),
602+
mergeBaseOctopus: op("merge-base-octopus"),
591603
}
592604
}
593605

0 commit comments

Comments
 (0)