From d6a48a17325a1f196eaf75b3aafba81954aa088d Mon Sep 17 00:00:00 2001 From: manjari Date: Mon, 2 Nov 2026 19:56:15 +0000 Subject: [PATCH] refactor(baserevision): Add a base revision extension --- Makefile | 1 + .../orchestrator/server/BUILD.bazel | 3 +- .../orchestrator/server/profiles.go | 27 +++-- .../extension/baserevision/BUILD.bazel | 8 ++ .../extension/baserevision/baserevision.go | 28 +++++ .../git}/BUILD.bazel | 11 +- submitqueue/extension/baserevision/git/git.go | 110 ++++++++++++++++++ .../git/git_test.go} | 16 +-- .../extension/baserevision/mock/BUILD.bazel | 9 ++ .../baserevision/mock/baserevision_mock.go | 56 +++++++++ .../extension/conflict/tango/BUILD.bazel | 2 + .../extension/conflict/tango/README.md | 2 +- .../extension/conflict/tango/analyzer.go | 8 +- .../extension/conflict/tango/analyzer_test.go | 15 ++- .../tango/baserevision/baserevision.go | 102 ---------------- 15 files changed, 266 insertions(+), 132 deletions(-) create mode 100644 submitqueue/extension/baserevision/BUILD.bazel create mode 100644 submitqueue/extension/baserevision/baserevision.go rename submitqueue/extension/{conflict/tango/baserevision => baserevision/git}/BUILD.bazel (73%) create mode 100644 submitqueue/extension/baserevision/git/git.go rename submitqueue/extension/{conflict/tango/baserevision/baserevision_test.go => baserevision/git/git_test.go} (94%) create mode 100644 submitqueue/extension/baserevision/mock/BUILD.bazel create mode 100644 submitqueue/extension/baserevision/mock/baserevision_mock.go delete mode 100644 submitqueue/extension/conflict/tango/baserevision/baserevision.go diff --git a/Makefile b/Makefile index f8e6c187f..792edef9b 100644 --- a/Makefile +++ b/Makefile @@ -636,6 +636,7 @@ GO_GENERATE_PACKAGES := \ ./stovepipe/extension/sourcecontrol/... \ ./stovepipe/extension/storage/... \ ./submitqueue/core/changeset/... \ + ./submitqueue/extension/baserevision/... \ ./submitqueue/extension/buildrunner/... \ ./submitqueue/extension/changeprovider/... \ ./submitqueue/extension/conflict/... \ diff --git a/service/submitqueue/orchestrator/server/BUILD.bazel b/service/submitqueue/orchestrator/server/BUILD.bazel index 6e186511c..051ea967e 100644 --- a/service/submitqueue/orchestrator/server/BUILD.bazel +++ b/service/submitqueue/orchestrator/server/BUILD.bazel @@ -43,6 +43,8 @@ go_library( "//service/submitqueue/core:go_default_library", "//submitqueue/core/changeset:go_default_library", "//submitqueue/entity:go_default_library", + "//submitqueue/extension/baserevision:go_default_library", + "//submitqueue/extension/baserevision/git:go_default_library", "//submitqueue/extension/buildrunner:go_default_library", "//submitqueue/extension/buildrunner/buildkite:go_default_library", "//submitqueue/extension/buildrunner/fake:go_default_library", @@ -59,7 +61,6 @@ go_library( "//submitqueue/extension/conflict/none:go_default_library", "//submitqueue/extension/conflict/pathoverlap:go_default_library", "//submitqueue/extension/conflict/tango:go_default_library", - "//submitqueue/extension/conflict/tango/baserevision:go_default_library", "//submitqueue/extension/conflict/tango/mysql:go_default_library", "//submitqueue/extension/conflict/tango/tangoclient:go_default_library", "//submitqueue/extension/queuepause:go_default_library", diff --git a/service/submitqueue/orchestrator/server/profiles.go b/service/submitqueue/orchestrator/server/profiles.go index e2ad5b718..d1414b125 100644 --- a/service/submitqueue/orchestrator/server/profiles.go +++ b/service/submitqueue/orchestrator/server/profiles.go @@ -18,6 +18,7 @@ import ( "database/sql" "fmt" nethttp "net/http" + "sync" orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" @@ -36,6 +37,8 @@ import ( "github.com/uber/submitqueue/platform/http" "github.com/uber/submitqueue/submitqueue/core/changeset" "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/baserevision" + baserevisiongit "github.com/uber/submitqueue/submitqueue/extension/baserevision/git" "github.com/uber/submitqueue/submitqueue/extension/buildrunner" buildkiterunner "github.com/uber/submitqueue/submitqueue/extension/buildrunner/buildkite" buildfake "github.com/uber/submitqueue/submitqueue/extension/buildrunner/fake" @@ -52,7 +55,6 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/conflict/none" "github.com/uber/submitqueue/submitqueue/extension/conflict/pathoverlap" "github.com/uber/submitqueue/submitqueue/extension/conflict/tango" - "github.com/uber/submitqueue/submitqueue/extension/conflict/tango/baserevision" tangomysql "github.com/uber/submitqueue/submitqueue/extension/conflict/tango/mysql" "github.com/uber/submitqueue/submitqueue/extension/conflict/tango/tangoclient" "github.com/uber/submitqueue/submitqueue/extension/speculation/allocator/sticky" @@ -761,24 +763,35 @@ func (b *profileBuilder) newTangoAnalyzerFactory(key tangoAnalyzerKey, where str return nil, fmt.Errorf("%s: no change repository provisioned at %q", where, key.Git.RepoPath) } workspaces := cloneWorkspaces{} - target := baserevision.Target{Repo: key.Git.RepoPath, Remote: key.Git.Remote, Branch: key.Git.Target} + target := baserevisiongit.Target{Repo: key.Git.RepoPath, Remote: key.Git.Remote, Branch: key.Git.Target} return analyzerFunc(func(c conflict.Config) (conflict.Analyzer, error) { - fromGit := baserevision.FromGit(workspaces, map[string]baserevision.Target{c.QueueName: target}) return tango.New(tango.Params{ Signatures: tangomysql.NewSignatureStore(b.tango.DB, b.scope.SubScope("tango"), c.QueueName), Cache: b.tango.Cache, Client: client, - BaseRevision: func(ctx context.Context, queue string) (string, error) { - repo.Lock() - defer repo.Unlock() - return fromGit(ctx, queue) + BaseRevisions: lockedBaseRevisions{ + Resolver: baserevisiongit.New(workspaces, map[string]baserevisiongit.Target{c.QueueName: target}), + lock: repo, }, Changes: b.resolver, }), nil }), nil } +// lockedBaseRevisions holds the clone's lock around each lookup, because the +// change provider works in the same clone. +type lockedBaseRevisions struct { + baserevision.Resolver + lock sync.Locker +} + +func (l lockedBaseRevisions) Resolve(ctx context.Context, queue string) (string, error) { + l.lock.Lock() + defer l.lock.Unlock() + return l.Resolver.Resolve(ctx, queue) +} + // cloneWorkspaces serves each repo, named by its clone directory, as a local // git workspace. type cloneWorkspaces struct{} diff --git a/submitqueue/extension/baserevision/BUILD.bazel b/submitqueue/extension/baserevision/BUILD.bazel new file mode 100644 index 000000000..50a13064e --- /dev/null +++ b/submitqueue/extension/baserevision/BUILD.bazel @@ -0,0 +1,8 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["baserevision.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/baserevision", + visibility = ["//visibility:public"], +) diff --git a/submitqueue/extension/baserevision/baserevision.go b/submitqueue/extension/baserevision/baserevision.go new file mode 100644 index 000000000..a739c6d78 --- /dev/null +++ b/submitqueue/extension/baserevision/baserevision.go @@ -0,0 +1,28 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package baserevision + +//go:generate mockgen -source=baserevision.go -destination=mock/baserevision_mock.go -package=mock + +import "context" + +// Resolver returns the commit a queue's new batches are analyzed against. +type Resolver interface { + // Resolve returns the full commit SHA of the tip of the queue's target + // branch as of the call. An error means the tip could not be determined, + // and a caller must not fall back to an earlier value. Implementations + // must be safe for concurrent use. + Resolve(ctx context.Context, queue string) (string, error) +} diff --git a/submitqueue/extension/conflict/tango/baserevision/BUILD.bazel b/submitqueue/extension/baserevision/git/BUILD.bazel similarity index 73% rename from submitqueue/extension/conflict/tango/baserevision/BUILD.bazel rename to submitqueue/extension/baserevision/git/BUILD.bazel index d0bc9a873..21c8885b9 100644 --- a/submitqueue/extension/conflict/tango/baserevision/BUILD.bazel +++ b/submitqueue/extension/baserevision/git/BUILD.bazel @@ -2,15 +2,18 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["baserevision.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/tango/baserevision", + srcs = ["git.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/baserevision/git", visibility = ["//visibility:public"], - deps = ["//platform/extension/gitworkspace:go_default_library"], + deps = [ + "//platform/extension/gitworkspace:go_default_library", + "//submitqueue/extension/baserevision:go_default_library", + ], ) go_test( name = "go_default_test", - srcs = ["baserevision_test.go"], + srcs = ["git_test.go"], embed = [":go_default_library"], deps = [ "//platform/extension/gitworkspace:go_default_library", diff --git a/submitqueue/extension/baserevision/git/git.go b/submitqueue/extension/baserevision/git/git.go new file mode 100644 index 000000000..3c21bd070 --- /dev/null +++ b/submitqueue/extension/baserevision/git/git.go @@ -0,0 +1,110 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package git resolves the base revision of a queue by asking a git workspace +// for the tip of the queue's target branch. Any gitworkspace.Factory works, so +// where git runs is the deployment's choice. +package git + +import ( + "context" + "fmt" + "regexp" + "strings" + "sync" + + "github.com/uber/submitqueue/platform/extension/gitworkspace" + "github.com/uber/submitqueue/submitqueue/extension/baserevision" +) + +const defaultRemote = "origin" + +// fullCommitSHA matches the SHA-1 and SHA-256 object formats. Tango needs the +// full SHA, so an abbreviated one is rejected. +var fullCommitSHA = regexp.MustCompile(`^([0-9a-f]{40}|[0-9a-f]{64})$`) + +// Target locates the branch whose tip is a queue's base revision. +type Target struct { + // Repo identifies the repository to the workspace factory. + Repo string + + // Remote is the git remote the branch is fetched from. It defaults to + // origin. + Remote string + + // Branch is the target branch. + Branch string +} + +type resolver struct { + factory gitworkspace.Factory + targets map[string]Target + locks map[string]*sync.Mutex +} + +// New returns a Resolver that fetches the target branch of the queue and +// returns its tip as a full commit SHA. A queue without a target, or any git +// failure, is an error. Calls that share a Repo are serialized, because +// concurrent fetches into one clone can fail on a ref lock and overwrite each +// other's FETCH_HEAD. +func New(factory gitworkspace.Factory, targets map[string]Target) baserevision.Resolver { + locks := make(map[string]*sync.Mutex, len(targets)) + for _, target := range targets { + locks[target.Repo] = &sync.Mutex{} + } + return resolver{factory: factory, targets: targets, locks: locks} +} + +func (r resolver) Resolve(ctx context.Context, queue string) (string, error) { + target, ok := r.targets[queue] + if !ok { + return "", fmt.Errorf("no target branch configured for queue %s", queue) + } + remote := target.Remote + if remote == "" { + remote = defaultRemote + } + + lock := r.locks[target.Repo] + lock.Lock() + defer lock.Unlock() + + workspace, err := r.factory.For(ctx, target.Repo) + if err != nil { + return "", fmt.Errorf("failed to open a workspace for repo %s: %w", target.Repo, err) + } + defer workspace.Close() + + outputs, err := workspace.Exec([]gitworkspace.Command{ + {Alias: "fetch", Bin: "git", Args: []string{"fetch", remote, target.Branch}}, + {Alias: "tip", Bin: "git", Args: []string{"rev-parse", "FETCH_HEAD^{commit}"}}, + }) + if err != nil { + return "", fmt.Errorf("failed to run git in repo %s: %w", target.Repo, err) + } + for _, out := range outputs { + if out.ExitCode != 0 { + return "", fmt.Errorf("git %s failed with exit code %d: %s", out.Alias, out.ExitCode, strings.TrimSpace(out.Stderr)) + } + } + if len(outputs) != 2 { + return "", fmt.Errorf("expected 2 git outputs, got %d", len(outputs)) + } + + sha := strings.TrimSpace(outputs[1].Stdout) + if !fullCommitSHA.MatchString(sha) { + return "", fmt.Errorf("git returned %q, not a full commit SHA", sha) + } + return sha, nil +} diff --git a/submitqueue/extension/conflict/tango/baserevision/baserevision_test.go b/submitqueue/extension/baserevision/git/git_test.go similarity index 94% rename from submitqueue/extension/conflict/tango/baserevision/baserevision_test.go rename to submitqueue/extension/baserevision/git/git_test.go index 9333198f3..cfb2be3ea 100644 --- a/submitqueue/extension/conflict/tango/baserevision/baserevision_test.go +++ b/submitqueue/extension/baserevision/git/git_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package baserevision +package git import ( "context" @@ -38,7 +38,7 @@ const ( var errTestFailure = errors.New("dependency down") -func TestFromGit(t *testing.T) { +func TestResolver_Resolve(t *testing.T) { tests := []struct { name string queue string @@ -142,7 +142,7 @@ func TestFromGit(t *testing.T) { } } - got, err := FromGit(factory, map[string]Target{testQueue: tt.target})(context.Background(), tt.queue) + got, err := New(factory, map[string]Target{testQueue: tt.target}).Resolve(context.Background(), tt.queue) if tt.wantErr { require.Error(t, err) @@ -154,7 +154,7 @@ func TestFromGit(t *testing.T) { } } -func TestFromGit_RepoLocks(t *testing.T) { +func TestResolver_Resolve_RepoLocks(t *testing.T) { validOutputs := []gitworkspace.Output{{Alias: "fetch"}, {Alias: "tip", Stdout: testSHA}} t.Run("calls on one repo never overlap", func(t *testing.T) { @@ -172,7 +172,7 @@ func TestFromGit_RepoLocks(t *testing.T) { inFlight.Add(-1) return validOutputs, nil }).AnyTimes() - baseRevision := FromGit(factory, map[string]Target{ + resolver := New(factory, map[string]Target{ "main-queue": {Repo: testRepo, Branch: "main"}, "release-queue": {Repo: testRepo, Branch: "release"}, }) @@ -186,7 +186,7 @@ func TestFromGit_RepoLocks(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - _, err := baseRevision(context.Background(), queue) + _, err := resolver.Resolve(context.Background(), queue) assert.NoError(t, err) }() } @@ -209,7 +209,7 @@ func TestFromGit_RepoLocks(t *testing.T) { }) workspace.EXPECT().Close().Return(nil) } - baseRevision := FromGit(factory, map[string]Target{ + resolver := New(factory, map[string]Target{ "queue-x": {Repo: "repo-x", Branch: "main"}, "queue-y": {Repo: "repo-y", Branch: "main"}, }) @@ -219,7 +219,7 @@ func TestFromGit_RepoLocks(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - _, err := baseRevision(context.Background(), queue) + _, err := resolver.Resolve(context.Background(), queue) assert.NoError(t, err) }() } diff --git a/submitqueue/extension/baserevision/mock/BUILD.bazel b/submitqueue/extension/baserevision/mock/BUILD.bazel new file mode 100644 index 000000000..428178d50 --- /dev/null +++ b/submitqueue/extension/baserevision/mock/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["baserevision_mock.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/baserevision/mock", + visibility = ["//visibility:public"], + deps = ["@org_uber_go_mock//gomock:go_default_library"], +) diff --git a/submitqueue/extension/baserevision/mock/baserevision_mock.go b/submitqueue/extension/baserevision/mock/baserevision_mock.go new file mode 100644 index 000000000..6500b26ca --- /dev/null +++ b/submitqueue/extension/baserevision/mock/baserevision_mock.go @@ -0,0 +1,56 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: baserevision.go +// +// Generated by this command: +// +// mockgen -source=baserevision.go -destination=mock/baserevision_mock.go -package=mock +// + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockResolver is a mock of Resolver interface. +type MockResolver struct { + ctrl *gomock.Controller + recorder *MockResolverMockRecorder + isgomock struct{} +} + +// MockResolverMockRecorder is the mock recorder for MockResolver. +type MockResolverMockRecorder struct { + mock *MockResolver +} + +// NewMockResolver creates a new mock instance. +func NewMockResolver(ctrl *gomock.Controller) *MockResolver { + mock := &MockResolver{ctrl: ctrl} + mock.recorder = &MockResolverMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockResolver) EXPECT() *MockResolverMockRecorder { + return m.recorder +} + +// Resolve mocks base method. +func (m *MockResolver) Resolve(ctx context.Context, queue string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Resolve", ctx, queue) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Resolve indicates an expected call of Resolve. +func (mr *MockResolverMockRecorder) Resolve(ctx, queue any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Resolve", reflect.TypeOf((*MockResolver)(nil).Resolve), ctx, queue) +} diff --git a/submitqueue/extension/conflict/tango/BUILD.bazel b/submitqueue/extension/conflict/tango/BUILD.bazel index 7b5f51152..9c8e229a1 100644 --- a/submitqueue/extension/conflict/tango/BUILD.bazel +++ b/submitqueue/extension/conflict/tango/BUILD.bazel @@ -20,6 +20,7 @@ go_library( "//platform/base/change:go_default_library", "//submitqueue/core/changeset:go_default_library", "//submitqueue/entity:go_default_library", + "//submitqueue/extension/baserevision:go_default_library", "//submitqueue/extension/conflict:go_default_library", "//submitqueue/extension/storage:go_default_library", "@org_uber_go_mock//gomock:go_default_library", @@ -40,6 +41,7 @@ go_test( "//platform/base/change:go_default_library", "//submitqueue/core/changeset/mock:go_default_library", "//submitqueue/entity:go_default_library", + "//submitqueue/extension/baserevision/mock:go_default_library", "//submitqueue/extension/conflict:go_default_library", "//submitqueue/extension/storage:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", diff --git a/submitqueue/extension/conflict/tango/README.md b/submitqueue/extension/conflict/tango/README.md index 3ff2c1652..00ccac76c 100644 --- a/submitqueue/extension/conflict/tango/README.md +++ b/submitqueue/extension/conflict/tango/README.md @@ -20,7 +20,7 @@ Each signature is computed against trunk at its own analysis time, without the o ## Injected pieces -The analyzer owns no connection. A deployment supplies the Tango client adapter, which collects the response and resolves each target's label, and a function that returns the commit SHA of the target branch tip a new batch is analyzed against. `baserevision.FromGit` builds that function over any `gitworkspace.Factory`. The changes of a batch come from the changeset resolver, the signature store is bound to one queue, and one signature cache can be shared by every queue's analyzer. The MySQL implementations of the signature and dictionary stores are in `mysql/`. +The analyzer owns no connection. A deployment supplies the Tango client adapter, which collects the response and resolves each target's label, and a `baserevision.Resolver` that returns the commit SHA of the target branch tip a new batch is analyzed against. The `baserevision` extension holds the contract and a git implementation over any `gitworkspace.Factory`. The changes of a batch come from the changeset resolver, the signature store is bound to one queue, and one signature cache can be shared by every queue's analyzer. The MySQL implementations of the signature and dictionary stores are in `mysql/`. ## Reference orchestrator diff --git a/submitqueue/extension/conflict/tango/analyzer.go b/submitqueue/extension/conflict/tango/analyzer.go index e7ddffece..3a7959429 100644 --- a/submitqueue/extension/conflict/tango/analyzer.go +++ b/submitqueue/extension/conflict/tango/analyzer.go @@ -22,6 +22,7 @@ import ( "github.com/uber/submitqueue/submitqueue/core/changeset" "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/baserevision" "github.com/uber/submitqueue/submitqueue/extension/conflict" "github.com/uber/submitqueue/submitqueue/extension/storage" ) @@ -43,9 +44,8 @@ type Params struct { // Client returns the targets a set of changes affects. Client ChangedTargetsClient - // BaseRevision returns the full commit SHA of the target branch tip that a - // new batch of the named queue is analyzed against. - BaseRevision func(ctx context.Context, queue string) (string, error) + // BaseRevisions returns the commit a new batch is analyzed against. + BaseRevisions baserevision.Resolver // Changes resolves a batch into the changes it contains. Changes changeset.Resolver @@ -114,7 +114,7 @@ func (a *analyzer) signNewBatch(ctx context.Context, batch entity.Batch) (Target if err != nil { return TargetSignature{}, fmt.Errorf("failed to resolve the changes: %w", err) } - baseRevision, err := a.BaseRevision(ctx, batch.Queue) + baseRevision, err := a.BaseRevisions.Resolve(ctx, batch.Queue) if err != nil { return TargetSignature{}, fmt.Errorf("failed to resolve the base revision of queue %s: %w", batch.Queue, err) } diff --git a/submitqueue/extension/conflict/tango/analyzer_test.go b/submitqueue/extension/conflict/tango/analyzer_test.go index 93ddb3209..f4ec34ec7 100644 --- a/submitqueue/extension/conflict/tango/analyzer_test.go +++ b/submitqueue/extension/conflict/tango/analyzer_test.go @@ -26,6 +26,7 @@ import ( "github.com/uber/submitqueue/platform/base/change" changesetmock "github.com/uber/submitqueue/submitqueue/core/changeset/mock" "github.com/uber/submitqueue/submitqueue/entity" + baserevisionmock "github.com/uber/submitqueue/submitqueue/extension/baserevision/mock" "github.com/uber/submitqueue/submitqueue/extension/conflict" "github.com/uber/submitqueue/submitqueue/extension/storage" ) @@ -49,17 +50,21 @@ type analyzerHarness struct { func newAnalyzerHarness(t *testing.T) *analyzerHarness { t.Helper() ctrl := gomock.NewController(t) + baseRevisions := baserevisionmock.NewMockResolver(ctrl) h := &analyzerHarness{ signatures: NewMockSignatureStore(ctrl), client: NewMockChangedTargetsClient(ctrl), changes: changesetmock.NewMockResolver(ctrl), } + baseRevisions.EXPECT().Resolve(gomock.Any(), analyzerTestQueue).DoAndReturn( + func(context.Context, string) (string, error) { return analyzerTestBaseRevision, h.baseErr }, + ).AnyTimes() h.analyzer = New(Params{ - Signatures: h.signatures, - Cache: NewSignatureCache(1 << 20), - Client: h.client, - BaseRevision: func(context.Context, string) (string, error) { return analyzerTestBaseRevision, h.baseErr }, - Changes: h.changes, + Signatures: h.signatures, + Cache: NewSignatureCache(1 << 20), + Client: h.client, + BaseRevisions: baseRevisions, + Changes: h.changes, }) return h } diff --git a/submitqueue/extension/conflict/tango/baserevision/baserevision.go b/submitqueue/extension/conflict/tango/baserevision/baserevision.go deleted file mode 100644 index 2b56be8e9..000000000 --- a/submitqueue/extension/conflict/tango/baserevision/baserevision.go +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) 2026 Uber Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package baserevision resolves the commit a new batch is analyzed against by -// asking a git workspace for the tip of the queue's target branch. Any -// gitworkspace.Factory works, so where git runs is the deployment's choice. -package baserevision - -import ( - "context" - "fmt" - "regexp" - "strings" - "sync" - - "github.com/uber/submitqueue/platform/extension/gitworkspace" -) - -const defaultRemote = "origin" - -// fullCommitSHA matches the SHA-1 and SHA-256 object formats. Tango needs the -// full SHA, so an abbreviated one is rejected. -var fullCommitSHA = regexp.MustCompile(`^([0-9a-f]{40}|[0-9a-f]{64})$`) - -// Target locates the branch whose tip is a queue's base revision. -type Target struct { - // Repo identifies the repository to the workspace factory. - Repo string - - // Remote is the git remote the branch is fetched from. It defaults to - // origin. - Remote string - - // Branch is the target branch. - Branch string -} - -// FromGit returns a function for tango.Params.BaseRevision that fetches the -// target branch of the queue and returns its tip as a full commit SHA, as of -// the call. A queue without a target, or any git failure, is an error. Calls -// that share a Repo are serialized, because concurrent fetches into one clone -// can fail on a ref lock and overwrite each other's FETCH_HEAD. -func FromGit(factory gitworkspace.Factory, targets map[string]Target) func(ctx context.Context, queue string) (string, error) { - locks := make(map[string]*sync.Mutex, len(targets)) - for _, target := range targets { - locks[target.Repo] = &sync.Mutex{} - } - - return func(ctx context.Context, queue string) (string, error) { - target, ok := targets[queue] - if !ok { - return "", fmt.Errorf("no target branch configured for queue %s", queue) - } - remote := target.Remote - if remote == "" { - remote = defaultRemote - } - - lock := locks[target.Repo] - lock.Lock() - defer lock.Unlock() - - workspace, err := factory.For(ctx, target.Repo) - if err != nil { - return "", fmt.Errorf("failed to open a workspace for repo %s: %w", target.Repo, err) - } - defer workspace.Close() - - outputs, err := workspace.Exec([]gitworkspace.Command{ - {Alias: "fetch", Bin: "git", Args: []string{"fetch", remote, target.Branch}}, - {Alias: "tip", Bin: "git", Args: []string{"rev-parse", "FETCH_HEAD^{commit}"}}, - }) - if err != nil { - return "", fmt.Errorf("failed to run git in repo %s: %w", target.Repo, err) - } - for _, out := range outputs { - if out.ExitCode != 0 { - return "", fmt.Errorf("git %s failed with exit code %d: %s", out.Alias, out.ExitCode, strings.TrimSpace(out.Stderr)) - } - } - if len(outputs) != 2 { - return "", fmt.Errorf("expected 2 git outputs, got %d", len(outputs)) - } - - sha := strings.TrimSpace(outputs[1].Stdout) - if !fullCommitSHA.MatchString(sha) { - return "", fmt.Errorf("git returned %q, not a full commit SHA", sha) - } - return sha, nil - } -}