Remove unused aspects of VCS
These appear unused by anything.
Change-Id: I957a0226b052673ffdd4e4a12dd6058e919f4e85
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/374458
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
diff --git a/go/buildskia/continuous_test.go b/go/buildskia/continuous_test.go
index a2d8218..8ae212b 100644
--- a/go/buildskia/continuous_test.go
+++ b/go/buildskia/continuous_test.go
@@ -79,9 +79,6 @@
func (m *mockVcs) Range(begin, end time.Time) []*vcsinfo.IndexCommit { return nil }
func (m *mockVcs) IndexOf(ctx context.Context, hash string) (int, error) { return 0, nil }
func (m *mockVcs) ByIndex(ctx context.Context, N int) (*vcsinfo.LongCommit, error) { return nil, nil }
-func (m *mockVcs) GetFile(ctx context.Context, fileName, commitHash string) (string, error) {
- return "", nil
-}
// Details returns the full commit information for the given hash.
// If includeBranchInfo is true the Branches field of the returned
diff --git a/go/git/gitinfo/gitinfo.go b/go/git/gitinfo/gitinfo.go
index 01b1223..fff2f39 100644
--- a/go/git/gitinfo/gitinfo.go
+++ b/go/git/gitinfo/gitinfo.go
@@ -440,15 +440,6 @@
return strings.Trim(output, "\n"), nil
}
-// GetFile returns the contents of the given file at the given commit.
-func (g *GitInfo) GetFile(ctx context.Context, fileName, commit string) (string, error) {
- output, err := g.dir.Git(ctx, "show", commit+":"+fileName)
- if err != nil {
- return "", err
- }
- return output, nil
-}
-
// InitialCommit returns the hash of the initial commit.
func (g *GitInfo) InitialCommit(ctx context.Context) (string, error) {
output, err := g.dir.Git(ctx, "rev-list", "--max-parents=0", "--first-parent", "HEAD")
diff --git a/go/gitstore/types.go b/go/gitstore/types.go
index 4b123f3..69080cd 100644
--- a/go/gitstore/types.go
+++ b/go/gitstore/types.go
@@ -65,19 +65,6 @@
RangeByTime(ctx context.Context, start, end time.Time, branch string) ([]*vcsinfo.IndexCommit, error)
}
-// GitStoreBased is an interface that tags an object as being based on GitStore and the
-// underlying GitStore instance can be retrieved. e.g.
-//
-// if gsb, ok := someInstance.(GitStoreBased); ok {
-// gitStore := gsb.GetGitStore()
-// ... do something with gitStore
-// }
-//
-type GitStoreBased interface {
- // GetGitStore returns the underlying GitStore instances.
- GetGitStore() GitStore
-}
-
// BranchPointer captures the HEAD of a branch and the index of that commit.
type BranchPointer struct {
Head string
diff --git a/go/vcsinfo/bt_vcs/bt_vcs.go b/go/vcsinfo/bt_vcs/bt_vcs.go
index bb66f53..e78dea0 100644
--- a/go/vcsinfo/bt_vcs/bt_vcs.go
+++ b/go/vcsinfo/bt_vcs/bt_vcs.go
@@ -8,7 +8,6 @@
"sync"
"time"
- "go.skia.org/infra/go/gitiles"
"go.skia.org/infra/go/gitstore"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
@@ -18,7 +17,6 @@
// BigTableVCS implements the vcsinfo.VCS interface based on a BT-backed GitStore.
type BigTableVCS struct {
gitStore gitstore.GitStore
- gitiles *gitiles.Repo
branch string
// This mutex protects detailsCache and indexCommits
@@ -35,13 +33,12 @@
// New returns an instance of vcsinfo.VCS that is backed by the given GitStore and uses the
// gittiles.Repo to retrieve files. Each instance provides an interface to one branch.
// The instance of gitiles.Repo is only used to fetch files.
-func New(ctx context.Context, gitStore gitstore.GitStore, branch string, repo *gitiles.Repo) (*BigTableVCS, error) {
+func New(ctx context.Context, gitStore gitstore.GitStore, branch string) (*BigTableVCS, error) {
if gitStore == nil {
return nil, errors.New("Cannot have nil gitStore")
}
ret := &BigTableVCS{
gitStore: gitStore,
- gitiles: repo,
branch: branch,
detailsCache: map[string]*vcsinfo.LongCommit{},
}
@@ -249,20 +246,6 @@
return b.detailsCache[b.indexCommits[idx].Hash], nil
}
-// GetFile implements the vcsinfo.VCS interface
-func (b *BigTableVCS) GetFile(ctx context.Context, fileName, commitHash string) (string, error) {
- contents, err := b.gitiles.ReadFileAtRef(ctx, fileName, commitHash)
- if err != nil {
- return "", skerr.Wrapf(err, "reading file %s @ %s via gitiles", fileName, commitHash)
- }
- return string(contents), nil
-}
-
-// GetGitStore implements the gitstore.GitStoreBased interface
-func (b *BigTableVCS) GetGitStore() gitstore.GitStore {
- return b.gitStore
-}
-
// timeRange retrieves IndexCommits from the given time range. Assumes that the
// caller holds b.mutex.
func (b *BigTableVCS) timeRange(start time.Time, end time.Time) []*vcsinfo.IndexCommit {
diff --git a/go/vcsinfo/bt_vcs/bt_vcs_test.go b/go/vcsinfo/bt_vcs/bt_vcs_test.go
index 742ac37..6e3ef22 100644
--- a/go/vcsinfo/bt_vcs/bt_vcs_test.go
+++ b/go/vcsinfo/bt_vcs/bt_vcs_test.go
@@ -9,7 +9,6 @@
"github.com/stretchr/testify/require"
"go.skia.org/infra/go/git"
- "go.skia.org/infra/go/gitiles"
"go.skia.org/infra/go/gitstore"
gs_testutils "go.skia.org/infra/go/gitstore/bt_gitstore/testutils"
"go.skia.org/infra/go/gitstore/mocks"
@@ -21,11 +20,6 @@
"golang.org/x/sync/errgroup"
)
-const (
- skiaRepoURL = "https://skia.googlesource.com/skia.git"
- localRepoURL = "https://example.com/local.git"
-)
-
func TestVCSSuite(t *testing.T) {
unittest.LargeTest(t)
vcs, _, cleanup := setupVCSLocalRepo(t, git.DefaultBranch)
@@ -78,7 +72,7 @@
mg.On("Get", testutils.AnyContext, hashes[:1]).Return(lcs[:1], nil).Once()
ctx := context.Background()
- vcs, err := New(ctx, mg, git.DefaultBranch, nil)
+ vcs, err := New(ctx, mg, git.DefaultBranch)
require.NoError(t, err)
// Now, pretend that the other two commits have landed, and run Update
@@ -100,19 +94,6 @@
require.NoError(t, egroup.Wait())
}
-// TestGetFile makes sure that we can use gittiles to fetch an
-// arbitrary file (DEPS) from the Skia repo at a chosen commit.
-func TestGetFile(t *testing.T) {
- unittest.LargeTest(t)
- gtRepo := gitiles.NewRepo(skiaRepoURL, nil)
- hash := "9be246ed747fd1b900013dd0596aed0b1a63a1fa"
- vcs := &BigTableVCS{
- gitiles: gtRepo,
- }
- _, err := vcs.GetFile(context.Background(), "DEPS", hash)
- require.NoError(t, err)
-}
-
// TestDetailsCaching makes sure that multiple calls to Details do
// not result in multiple calls to the underlying gitstore, that is,
// the details per commit hash are cached.
@@ -127,7 +108,7 @@
mg.On("RangeN", testutils.AnyContext, 0, math.MaxInt32, git.DefaultBranch).Return(makeTestIndexCommits(), nil)
mg.On("Get", testutils.AnyContext, []string{firstHash, secondHash, thirdHash}).Return(commits, nil).Once()
- vcs, err := New(context.Background(), mg, git.DefaultBranch, nil)
+ vcs, err := New(context.Background(), mg, git.DefaultBranch)
require.NoError(t, err)
// query details 3 times, and make sure it uses the cache after the
@@ -160,7 +141,7 @@
mg.On("RangeN", testutils.AnyContext, 0, math.MaxInt32, git.DefaultBranch).Return(makeTestIndexCommits(), nil)
mg.On("Get", testutils.AnyContext, []string{firstHash, secondHash, thirdHash}).Return(commits, nil).Once()
- vcs, err := New(context.Background(), mg, git.DefaultBranch, nil)
+ vcs, err := New(context.Background(), mg, git.DefaultBranch)
require.NoError(t, err)
// query details 3 times, and make sure it uses the cache after the
@@ -194,7 +175,7 @@
require.NoError(t, err)
ctx := context.Background()
_, _, btgs := gs_testutils.SetupAndLoadBTGitStore(t, ctx, wd, "file://"+repoDir, true)
- vcs, err := New(ctx, btgs, branch, nil)
+ vcs, err := New(ctx, btgs, branch)
require.NoError(t, err)
return vcs, btgs, func() {
util.RemoveAll(wd)
@@ -274,12 +255,3 @@
},
}
}
-
-func makeTestBranchPointerMap() map[string]*gitstore.BranchPointer {
- return map[string]*gitstore.BranchPointer{
- git.DefaultBranch: {
- Head: git.DefaultBranch,
- Index: 3,
- },
- }
-}
diff --git a/go/vcsinfo/mocks/VCS.go b/go/vcsinfo/mocks/VCS.go
index 5bfd333..2bb9453 100644
--- a/go/vcsinfo/mocks/VCS.go
+++ b/go/vcsinfo/mocks/VCS.go
@@ -101,27 +101,6 @@
return r0
}
-// GetFile provides a mock function with given fields: ctx, fileName, commitHash
-func (_m *VCS) GetFile(ctx context.Context, fileName string, commitHash string) (string, error) {
- ret := _m.Called(ctx, fileName, commitHash)
-
- var r0 string
- if rf, ok := ret.Get(0).(func(context.Context, string, string) string); ok {
- r0 = rf(ctx, fileName, commitHash)
- } else {
- r0 = ret.Get(0).(string)
- }
-
- var r1 error
- if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok {
- r1 = rf(ctx, fileName, commitHash)
- } else {
- r1 = ret.Error(1)
- }
-
- return r0, r1
-}
-
// IndexOf provides a mock function with given fields: ctx, hash
func (_m *VCS) IndexOf(ctx context.Context, hash string) (int, error) {
ret := _m.Called(ctx, hash)
diff --git a/go/vcsinfo/types.go b/go/vcsinfo/types.go
index 5a1c50e..ab3866d 100644
--- a/go/vcsinfo/types.go
+++ b/go/vcsinfo/types.go
@@ -113,7 +113,4 @@
// ByIndex returns a LongCommit describing the commit
// at position N, as ordered in the current branch.
ByIndex(ctx context.Context, N int) (*LongCommit, error)
-
- // GetFile returns the content of the given file at the given commit.
- GetFile(ctx context.Context, fileName, commitHash string) (string, error)
}
diff --git a/golden/cmd/gold_frontend/gold_frontend.go b/golden/cmd/gold_frontend/gold_frontend.go
index 68e988b..8c8cd8e 100644
--- a/golden/cmd/gold_frontend/gold_frontend.go
+++ b/golden/cmd/gold_frontend/gold_frontend.go
@@ -33,7 +33,6 @@
"go.skia.org/infra/go/common"
"go.skia.org/infra/go/firestore"
"go.skia.org/infra/go/gerrit"
- "go.skia.org/infra/go/gitiles"
"go.skia.org/infra/go/gitstore/bt_gitstore"
"go.skia.org/infra/go/httputils"
"go.skia.org/infra/go/login"
@@ -385,10 +384,7 @@
// mustMakeVCS returns a vcsinfo.VCS that wraps the given BigTable-backed GitStore.
func mustMakeVCS(ctx context.Context, fsc *frontendServerConfig, gitStore *bt_gitstore.BigTableGitStore) *bt_vcs.BigTableVCS {
- // TODO(kjlubick): remove gitilesRepo and the GetFile() from vcsinfo (unused and
- // leaky abstraction).
- gitilesRepo := gitiles.NewRepo("", nil)
- vcs, err := bt_vcs.New(ctx, gitStore, fsc.GitRepoBranch, gitilesRepo)
+ vcs, err := bt_vcs.New(ctx, gitStore, fsc.GitRepoBranch)
if err != nil {
sklog.Fatalf("Error creating BT-backed VCS instance: %s", err)
}
diff --git a/golden/cmd/gold_ingestion/gold_ingestion.go b/golden/cmd/gold_ingestion/gold_ingestion.go
index 6813bc8..3b83efd 100644
--- a/golden/cmd/gold_ingestion/gold_ingestion.go
+++ b/golden/cmd/gold_ingestion/gold_ingestion.go
@@ -18,7 +18,6 @@
"go.skia.org/infra/go/auth"
"go.skia.org/infra/go/common"
- "go.skia.org/infra/go/gitiles"
"go.skia.org/infra/go/gitstore/bt_gitstore"
"go.skia.org/infra/go/httputils"
"go.skia.org/infra/go/sklog"
@@ -156,9 +155,8 @@
sklog.Fatalf("could not instantiate gitstore for %s: %s", isc.GitRepoURL, err)
}
- // Set up VCS instance to track master.
- gitilesRepo := gitiles.NewRepo(isc.GitRepoURL, client)
- vcs, err := bt_vcs.New(ctx, gitStore, isc.GitRepoBranch, gitilesRepo)
+ // Set up VCS instance to track primary branch.
+ vcs, err := bt_vcs.New(ctx, gitStore, isc.GitRepoBranch)
if err != nil {
sklog.Fatalf("could not instantiate BT VCS for %s", isc.GitRepoURL)
}
diff --git a/golden/cmd/trace_tool/trace_tool.go b/golden/cmd/trace_tool/trace_tool.go
index 43ceef5..312dd52 100644
--- a/golden/cmd/trace_tool/trace_tool.go
+++ b/golden/cmd/trace_tool/trace_tool.go
@@ -10,7 +10,6 @@
"cloud.google.com/go/bigtable"
"go.skia.org/infra/go/bt"
- "go.skia.org/infra/go/gitiles"
"go.skia.org/infra/go/gitstore/bt_gitstore"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/vcsinfo/bt_vcs"
@@ -44,8 +43,7 @@
sklog.Fatalf("Error instantiating gitstore: %s", err)
}
- gitilesRepo := gitiles.NewRepo("", nil)
- vcs, err := bt_vcs.New(ctx, gitStore, branch, gitilesRepo)
+ vcs, err := bt_vcs.New(ctx, gitStore, branch)
if err != nil {
sklog.Fatalf("Error creating BT-backed VCS instance: %s", err)
}
