[perf] Factor out the git/provider.Provider interface.
This is an initial step in adding Gitiles support, as it factors
out the only touchpoints with git, makes that into an interface (provider.Provider),
and then moves all the git specific code into an implementation
of that interface (providers/cli).
A follow-on CL will implement providers.Provider for Gitiles.
This also adds a provider selection to the config file format
and updates all the existing configs to use 'git'.
Finally this adds an optional StartCommit, as opposed to always
starting from the initial commit to the repo, which would be
prohibitive in a huge repo like Chrome.
Bug: b/271555267
Change-Id: Ie8bb28656684532cf91b39e933d98890435b9b0c
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/651947
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
diff --git a/cmd/presubmit/presubmit.go b/cmd/presubmit/presubmit.go
index 23ebaed..ec3ce6b 100644
--- a/cmd/presubmit/presubmit.go
+++ b/cmd/presubmit/presubmit.go
@@ -517,6 +517,10 @@
// This is the one place where we are allowed to shell out to git; all
// others should go through here.
regexp.MustCompile(`go/git/git_common/.*\.go`),
+ // Just using the word "git" as a config value.
+ regexp.MustCompile(`perf/go/config/.*\.go`),
+ // Just using the word "git" as a directory name.
+ regexp.MustCompile(`perf/go/git/providers/git_checkout/.*\.go`),
},
},
}
diff --git a/perf/configs/angle.json b/perf/configs/angle.json
index 052fb91..3c8f55e 100644
--- a/perf/configs/angle.json
+++ b/perf/configs/angle.json
@@ -25,6 +25,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://chromium.googlesource.com/angle/angle",
"dir": "/tmp/angle",
"debounce_commit_url": false
diff --git a/perf/configs/cdb-android-x.json b/perf/configs/cdb-android-x.json
index 42d5cee..7f10cd0 100644
--- a/perf/configs/cdb-android-x.json
+++ b/perf/configs/cdb-android-x.json
@@ -32,6 +32,7 @@
},
"git_repo_config": {
"git_auth_type": "gerrit",
+ "provider": "git",
"url": "https://skia.googlesource.com/perf-buildid/android-master",
"dir": "/tmp/androidx",
"debounce_commit_url": true
diff --git a/perf/configs/cdb-ct-prod.json b/perf/configs/cdb-ct-prod.json
index 984dc1e..c2feee7 100644
--- a/perf/configs/cdb-ct-prod.json
+++ b/perf/configs/cdb-ct-prod.json
@@ -25,6 +25,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://skia.googlesource.com/perf-ct",
"dir": "/tmp/ct",
"debounce_commit_url": false
diff --git a/perf/configs/cdb-nano.json b/perf/configs/cdb-nano.json
index b9ecc45..5771b1f 100644
--- a/perf/configs/cdb-nano.json
+++ b/perf/configs/cdb-nano.json
@@ -32,6 +32,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://skia.googlesource.com/skia",
"dir": "/tmp/skiaperf",
"debounce_commit_url": false,
diff --git a/perf/configs/comp-ui.json b/perf/configs/comp-ui.json
index 2ffb8c0..31c8223 100644
--- a/perf/configs/comp-ui.json
+++ b/perf/configs/comp-ui.json
@@ -26,6 +26,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"git_auth_type": "gerrit",
"url": "https://skia.googlesource.com/perf-compui",
"dir": "/tmp/compui-perf",
diff --git a/perf/configs/demo.json b/perf/configs/demo.json
index 29bf13d..6c68831 100644
--- a/perf/configs/demo.json
+++ b/perf/configs/demo.json
@@ -20,6 +20,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://github.com/skia-dev/perf-demo-repo.git",
"dir": "/tmp/perf-demo",
"debounce_commit_url": false
diff --git a/perf/configs/flutter-engine2.json b/perf/configs/flutter-engine2.json
index 0b3c872..c402da8 100644
--- a/perf/configs/flutter-engine2.json
+++ b/perf/configs/flutter-engine2.json
@@ -26,6 +26,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://github.com/flutter/engine",
"dir": "/tmp/flutter-engine",
"debounce_commit_url": false,
diff --git a/perf/configs/flutter-flutter2.json b/perf/configs/flutter-flutter2.json
index a4cafb3..9922e74 100644
--- a/perf/configs/flutter-flutter2.json
+++ b/perf/configs/flutter-flutter2.json
@@ -26,6 +26,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://github.com/flutter/flutter",
"dir": "/tmp/flutter-flutter",
"debounce_commit_url": false,
diff --git a/perf/configs/v8.json b/perf/configs/v8.json
index f466228..4dcdb52 100644
--- a/perf/configs/v8.json
+++ b/perf/configs/v8.json
@@ -26,6 +26,7 @@
"file_ingestion_pubsub_topic_name": ""
},
"git_repo_config": {
+ "provider": "git",
"url": "https://chromium.googlesource.com/v8/v8.git",
"dir": "/tmp/v8",
"debounce_commit_url": false
diff --git a/perf/go/bug/BUILD.bazel b/perf/go/bug/BUILD.bazel
index 1e74128..212bbf6 100644
--- a/perf/go/bug/BUILD.bazel
+++ b/perf/go/bug/BUILD.bazel
@@ -8,7 +8,7 @@
visibility = ["//visibility:public"],
deps = [
"//go/sklog",
- "//perf/go/git",
+ "//perf/go/git/provider",
"@in_gopkg_olivere_elastic_v5//uritemplates",
],
)
@@ -18,7 +18,7 @@
srcs = ["bug_test.go"],
embed = [":bug"],
deps = [
- "//perf/go/git",
+ "//perf/go/git/provider",
"@com_github_stretchr_testify//assert",
],
)
diff --git a/perf/go/bug/bug.go b/perf/go/bug/bug.go
index 030b78f..f487451 100644
--- a/perf/go/bug/bug.go
+++ b/perf/go/bug/bug.go
@@ -3,12 +3,12 @@
import (
"go.skia.org/infra/go/sklog"
- perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"gopkg.in/olivere/elastic.v5/uritemplates"
)
// Expand the uriTemplate given a link to the regressing cluster, the commit, and the user's message about the regression.
-func Expand(uriTemplate string, clusterLink string, c perfgit.Commit, message string) string {
+func Expand(uriTemplate string, clusterLink string, c provider.Commit, message string) string {
expansion := map[string]string{
"cluster_url": clusterLink,
"commit_url": c.URL,
@@ -23,7 +23,7 @@
// ExampleExpand expands the given uriTemplate with example data.
func ExampleExpand(uriTemplate string) string {
- c := perfgit.Commit{
+ c := provider.Commit{
URL: "https://skia.googlesource.com/skia/+show/d261e1075a93677442fdf7fe72aba7e583863664",
}
clusterLink := "https://perf.skia.org/t/?begin=1498332791&end=1498528391&subset=flagged"
diff --git a/perf/go/bug/bug_test.go b/perf/go/bug/bug_test.go
index 10aed4b..e8ad6a7 100644
--- a/perf/go/bug/bug_test.go
+++ b/perf/go/bug/bug_test.go
@@ -4,12 +4,12 @@
"testing"
"github.com/stretchr/testify/assert"
- perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
)
func TestExpand(t *testing.T) {
- c := perfgit.Commit{
+ c := provider.Commit{
URL: "https://skia.googlesource.com/skia/+show/d261e1075a93677442fdf7fe72aba7e583863664",
}
clusterLink := "https://perf.skia.org/t/?begin=1498332791&end=1498528391&subset=flagged"
diff --git a/perf/go/builders/builders_test.go b/perf/go/builders/builders_test.go
index 53486f4..b76e827 100644
--- a/perf/go/builders/builders_test.go
+++ b/perf/go/builders/builders_test.go
@@ -172,7 +172,7 @@
}
func TestNewPerfGitFromConfig_CockroachDB_Success(t *testing.T) {
- ctx, _, _, hashes, instanceConfig := gittest.NewForTest(t)
+ ctx, _, _, hashes, _, instanceConfig := gittest.NewForTest(t)
instanceConfig.DataStoreConfig.DataStoreType = config.CockroachDBDataStoreType
diff --git a/perf/go/config/config.go b/perf/go/config/config.go
index c1578d2..18fa3a1 100644
--- a/perf/go/config/config.go
+++ b/perf/go/config/config.go
@@ -209,12 +209,39 @@
GitAuthGerrit GitAuthType = "gerrit"
)
+// GitProvider is the method used to interrogate git repos.
+type GitProvider string
+
+const (
+ // GitProviderCLI uses a local copy of git to checkout the repo.
+ GitProviderCLI GitProvider = "git"
+
+ // GitProviderGitiles uses the Gitiles API.
+ GitProviderGitiles GitProvider = "gitiles"
+)
+
+// AllGitProviders is a slice of all valid GitProviders.
+var AllGitProviders []GitProvider = []GitProvider{
+ GitProviderCLI,
+ GitProviderGitiles,
+}
+
// GitRepoConfig is the config for the git repo.
type GitRepoConfig struct {
// GitAuthType is the type of authentication the repo requires. Defaults to
// GitAuthNone.
GitAuthType GitAuthType `json:"git_auth_type,omitempty"`
+ // Provider is the method used to interrogate git repos.
+ Provider GitProvider `json:"provider"`
+
+ // StartCommit is the commit in the repo where we start tracking commits,
+ // i.e. StartCommit will have a Commit Number of 0. If not supplied then
+ // default to the first commit in the repo. This is used to avoid having to
+ // ingest all the commits in a huge repo where we don't care about the
+ // majority of the history, e.g. Chrome.
+ StartCommit string `json:"start_commit,omitempty"`
+
// URL that the Git repo is fetched from.
URL string `json:"url"`
diff --git a/perf/go/config/instanceConfigSchema.json b/perf/go/config/instanceConfigSchema.json
index 2db9926..3f7d533 100644
--- a/perf/go/config/instanceConfigSchema.json
+++ b/perf/go/config/instanceConfigSchema.json
@@ -73,6 +73,7 @@
},
"GitRepoConfig": {
"required": [
+ "provider",
"url",
"dir"
],
@@ -80,6 +81,12 @@
"git_auth_type": {
"type": "string"
},
+ "provider": {
+ "type": "string"
+ },
+ "start_commit": {
+ "type": "string"
+ },
"url": {
"type": "string"
},
diff --git a/perf/go/dataframe/dataframe_test.go b/perf/go/dataframe/dataframe_test.go
index c46678e..73d9eb8 100644
--- a/perf/go/dataframe/dataframe_test.go
+++ b/perf/go/dataframe/dataframe_test.go
@@ -134,7 +134,7 @@
}
func TestFromTimeRange_Success(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -154,7 +154,7 @@
}
func TestFromTimeRange_EmptySlicesIfNothingInTimeRange(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
diff --git a/perf/go/dfbuilder/dfbuilder_test.go b/perf/go/dfbuilder/dfbuilder_test.go
index 5c3fb76..83e4c20 100644
--- a/perf/go/dfbuilder/dfbuilder_test.go
+++ b/perf/go/dfbuilder/dfbuilder_test.go
@@ -65,7 +65,7 @@
func TestBuildNew(t *testing.T) {
ctx := context.Background()
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -195,7 +195,7 @@
}
func TestFromIndexRange_Success(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -219,7 +219,7 @@
}
func TestFromIndexRange_EmptySliceOnBadCommitNumber(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -231,7 +231,7 @@
}
func TestPreflightQuery_EmptyQuery_ReturnsError(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -258,7 +258,7 @@
}
func TestPreflightQuery_NonEmptyQuery_Success(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -301,7 +301,7 @@
}
func TestPreflightQuery_TilesContainDifferentNumberOfMatches_ReturnedParamSetReflectsBothTiles(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -350,7 +350,7 @@
}
func TestNumMatches_EmptyQuery_ReturnsError(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -367,7 +367,7 @@
}
func TestNumMatches_NonEmptyQuery_Success(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -396,7 +396,7 @@
}
func TestNumMatches_TilesContainDifferentNumberOfMatches_TheLargerOfTheTwoCountsIsReturned(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
diff --git a/perf/go/dfiter/dfiter_test.go b/perf/go/dfiter/dfiter_test.go
index 996c159..4492485 100644
--- a/perf/go/dfiter/dfiter_test.go
+++ b/perf/go/dfiter/dfiter_test.go
@@ -70,7 +70,7 @@
}, "gs://foo.json", time.Now()) // Time is irrelevent.
assert.NoError(t, err)
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
instanceConfig.DataStoreConfig.TileSize = testTileSize
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
diff --git a/perf/go/dryrun/BUILD.bazel b/perf/go/dryrun/BUILD.bazel
index cdaf00c..7aa4647 100644
--- a/perf/go/dryrun/BUILD.bazel
+++ b/perf/go/dryrun/BUILD.bazel
@@ -11,6 +11,7 @@
"//go/sklog",
"//perf/go/dataframe",
"//perf/go/git",
+ "//perf/go/git/provider",
"//perf/go/progress",
"//perf/go/regression",
"//perf/go/shortcut",
diff --git a/perf/go/dryrun/dryrun.go b/perf/go/dryrun/dryrun.go
index 5ee51f3..8ef1b9e 100644
--- a/perf/go/dryrun/dryrun.go
+++ b/perf/go/dryrun/dryrun.go
@@ -13,6 +13,7 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/perf/go/dataframe"
perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/progress"
"go.skia.org/infra/perf/go/regression"
"go.skia.org/infra/perf/go/shortcut"
@@ -21,7 +22,7 @@
// RegressionAtCommit is a Regression found for a specific commit.
type RegressionAtCommit struct {
- CID perfgit.Commit `json:"cid"`
+ CID provider.Commit `json:"cid"`
Regression *regression.Regression `json:"regression"`
}
diff --git a/perf/go/frontend/BUILD.bazel b/perf/go/frontend/BUILD.bazel
index d128fe5..a7d7bbe 100644
--- a/perf/go/frontend/BUILD.bazel
+++ b/perf/go/frontend/BUILD.bazel
@@ -31,6 +31,7 @@
"//perf/go/dfbuilder",
"//perf/go/dryrun",
"//perf/go/git",
+ "//perf/go/git/provider",
"//perf/go/notify",
"//perf/go/progress",
"//perf/go/psrefresh",
diff --git a/perf/go/frontend/frontend.go b/perf/go/frontend/frontend.go
index fbec235..599e08e 100644
--- a/perf/go/frontend/frontend.go
+++ b/perf/go/frontend/frontend.go
@@ -48,6 +48,7 @@
"go.skia.org/infra/perf/go/dfbuilder"
"go.skia.org/infra/perf/go/dryrun"
perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/notify"
"go.skia.org/infra/perf/go/progress"
"go.skia.org/infra/perf/go/psrefresh"
@@ -521,7 +522,7 @@
}
// Filter if we have a restricted set of branches.
- ret := []perfgit.Commit{}
+ ret := []provider.Commit{}
if len(config.Config.IngestionConfig.Branches) != 0 {
for _, details := range resp {
for _, branch := range config.Config.IngestionConfig.Branches {
@@ -647,7 +648,7 @@
// CIDHandlerResponse is the form of the response from the /_/cid/ endpoint.
type CIDHandlerResponse struct {
// CommitSlice describes all the commits requested.
- CommitSlice []perfgit.Commit `json:"commitSlice"`
+ CommitSlice []provider.Commit `json:"commitSlice"`
// LogEntry is the full git log entry for the first commit in the
// CommitSlice.
@@ -999,7 +1000,7 @@
//
// The Columns have the same order as RegressionRangeResponse.Header.
type RegressionRow struct {
- Commit perfgit.Commit `json:"cid"`
+ Commit provider.Commit `json:"cid"`
Columns []*regression.Regression `json:"columns"`
}
@@ -1086,7 +1087,7 @@
}
// Get a list of commits for the range.
- var commits []perfgit.Commit
+ var commits []provider.Commit
if rr.Subset == SubsetAll {
commits, err = f.perfGit.CommitSliceFromTimeRange(r.Context(), time.Unix(rr.Begin, 0), time.Unix(rr.End, 0))
if err != nil {
@@ -1113,7 +1114,7 @@
// Reverse the order of the cids, so the latest
// commit shows up first in the UI display.
- revCids := make([]perfgit.Commit, len(commits), len(commits))
+ revCids := make([]provider.Commit, len(commits), len(commits))
for i, c := range commits {
revCids[len(commits)-1-i] = c
}
diff --git a/perf/go/git/BUILD.bazel b/perf/go/git/BUILD.bazel
index 5bc0e7c..f3eb9ec 100644
--- a/perf/go/git/BUILD.bazel
+++ b/perf/go/git/BUILD.bazel
@@ -7,21 +7,18 @@
importpath = "go.skia.org/infra/perf/go/git",
visibility = ["//visibility:public"],
deps = [
- "//go/auth",
- "//go/git/git_common",
- "//go/gitauth",
"//go/gitiles",
- "//go/human",
"//go/metrics2",
"//go/skerr",
"//go/sklog",
"//perf/go/config",
+ "//perf/go/git/provider",
+ "//perf/go/git/providers",
"//perf/go/types",
"@com_github_hashicorp_golang_lru//:golang-lru",
"@com_github_jackc_pgx_v4//:pgx",
"@com_github_jackc_pgx_v4//pgxpool",
"@io_opencensus_go//trace",
- "@org_golang_x_oauth2//google",
],
)
@@ -43,6 +40,7 @@
"//go/git/testutils",
"//perf/go/config",
"//perf/go/git/gittest",
+ "//perf/go/git/provider",
"//perf/go/types",
"@com_github_stretchr_testify//assert",
"@com_github_stretchr_testify//require",
diff --git a/perf/go/git/git.go b/perf/go/git/git.go
index 2f9a3d1..b414293 100644
--- a/perf/go/git/git.go
+++ b/perf/go/git/git.go
@@ -6,33 +6,22 @@
package git
import (
- "bufio"
- "bytes"
"context"
"fmt"
- "io"
- "os"
- "os/exec"
- "path/filepath"
- "strconv"
- "strings"
"time"
lru "github.com/hashicorp/golang-lru"
"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v4/pgxpool"
"go.opencensus.io/trace"
- "go.skia.org/infra/go/auth"
- "go.skia.org/infra/go/git/git_common"
- "go.skia.org/infra/go/gitauth"
"go.skia.org/infra/go/gitiles"
- "go.skia.org/infra/go/human"
"go.skia.org/infra/go/metrics2"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/perf/go/config"
+ "go.skia.org/infra/perf/go/git/provider"
+ "go.skia.org/infra/perf/go/git/providers"
"go.skia.org/infra/perf/go/types"
- "golang.org/x/oauth2/google"
)
// For rough numbers a Commit Author is 50 , Subject 80 , URL 200, and GitHash 32 bytes. So
@@ -40,23 +29,6 @@
// 25,000 entries.
const commitCacheSize = 25_000
-// Commit represents a single commit stored in the database.
-//
-// JSON annotations make it serialize like the legacy cid.CommitDetail.
-type Commit struct {
- CommitNumber types.CommitNumber `json:"offset"`
- GitHash string `json:"hash"`
- Timestamp int64 `json:"ts"` // Unix timestamp, seconds from the epoch.
- Author string `json:"author"`
- Subject string `json:"message"`
- URL string `json:"url"`
-}
-
-// Display returns a display string that describes the commit.
-func (c Commit) Display(now time.Time) string {
- return fmt.Sprintf("%s - %s - %s", c.GitHash[:7], human.Duration(now.Sub(time.Unix(c.Timestamp, 0))), c.Subject)
-}
-
// statement is an SQL statement identifier.
type statement int
@@ -75,7 +47,7 @@
var (
// BadCommit is returned on errors from functions that return Commits.
- BadCommit = Commit{
+ BadCommit = provider.Commit{
CommitNumber: types.BadCommitNumber,
}
)
@@ -175,8 +147,7 @@
//
// Please see perf/sql/migrations for the database schema used.
type Git struct {
- // gitFullPath is the path of the git executable.
- gitFullPath string
+ gp provider.Provider
instanceConfig *config.InstanceConfig
@@ -202,47 +173,17 @@
// The instance created does not poll by default, callers need to call
// StartBackgroundPolling().
func New(ctx context.Context, local bool, db *pgxpool.Pool, instanceConfig *config.InstanceConfig) (*Git, error) {
- // Do git authentication if required.
- if instanceConfig.GitRepoConfig.GitAuthType == config.GitAuthGerrit {
- sklog.Info("Authenticating to Gerrit.")
- ts, err := google.DefaultTokenSource(ctx, auth.ScopeGerrit)
- if err != nil {
- return nil, skerr.Wrapf(err, "Failed to get tokensource perfgit.Git for config %v", *instanceConfig)
- }
- if _, err := gitauth.New(ts, "/tmp/git-cookie", true, ""); err != nil {
- return nil, skerr.Wrapf(err, "Failed to gitauth perfgit.Git for config %v", *instanceConfig)
- }
- }
-
- // Find the path to the git executable, which might be relative to working dir.
- gitFullPath, _, _, err := git_common.FindGit(ctx)
- if err != nil {
- return nil, skerr.Wrapf(err, "Failed to find git.")
- }
-
- // Force the path to be absolute.
- gitFullPath, err = filepath.Abs(gitFullPath)
- if err != nil {
- return nil, skerr.Wrapf(err, "Failed to get absolute path to git.")
- }
-
- // Clone the git repo if necessary.
- sklog.Infof("Cloning repo.")
- if _, err := os.Stat(instanceConfig.GitRepoConfig.Dir); os.IsNotExist(err) {
- cmd := exec.CommandContext(ctx, gitFullPath, "clone", instanceConfig.GitRepoConfig.URL, instanceConfig.GitRepoConfig.Dir)
- if err := cmd.Run(); err != nil {
- exerr := err.(*exec.ExitError)
- return nil, skerr.Wrapf(err, "Failed to clone repo: %s - %s", err, exerr.Stderr)
- }
- }
-
cache, err := lru.New(commitCacheSize)
if err != nil {
return nil, skerr.Wrap(err)
}
+ gp, err := providers.New(ctx, instanceConfig)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
ret := &Git{
- gitFullPath: gitFullPath,
+ gp: gp,
db: db,
cache: cache,
instanceConfig: instanceConfig,
@@ -269,10 +210,9 @@
func (g *Git) StartBackgroundPolling(ctx context.Context, duration time.Duration) {
go func() {
liveness := metrics2.NewLiveness("perf_git_udpate_polling_livenes")
+ ctx := context.Background()
for range time.Tick(duration) {
- timeoutCtx, cancel := context.WithTimeout(ctx, duration)
- defer cancel()
- if err := g.Update(timeoutCtx); err != nil {
+ if err := g.Update(ctx); err != nil {
sklog.Errorf("Failed to update git repo: %s", err)
} else {
liveness.Reset()
@@ -281,80 +221,6 @@
}()
}
-type parseGitRevLogStreamProcessSingleCommit func(commit Commit) error
-
-// parseGitRevLogStream parses the input stream for input of the form:
-//
-// commit 6079a7810530025d9877916895dd14eb8bb454c0
-// Joe Gregorio <joe@bitworking.org>
-// Change #9
-// 1584837783
-// commit 977e0ef44bec17659faf8c5d4025c5a068354817
-// Joe Gregorio <joe@bitworking.org>
-// Change #8
-// 1584837783
-//
-// And calls the parseGitRevLogStreamProcessSingleCommit function with each
-// entry it finds. The passed in Commit has all valid fields except
-// CommitNumber, which is set to types.BadCommitNumber.
-func parseGitRevLogStream(r io.ReadCloser, f parseGitRevLogStreamProcessSingleCommit) error {
- scanner := bufio.NewScanner(r)
- lineNumber := 0
- for scanner.Scan() {
- line := scanner.Text()
- if !strings.HasPrefix(line, "commit ") {
- return skerr.Fmt("Invalid format, expected commit at line %d: %q", lineNumber, line)
- }
- lineNumber++
- gitHash := strings.Split(line, " ")[1]
-
- if !scanner.Scan() {
- return skerr.Fmt("Ran out of input, expecting an author line: %d", lineNumber)
- }
- lineNumber++
- author := scanner.Text()
-
- if !scanner.Scan() {
- return skerr.Fmt("Ran out of input, expecting a subject line: %d", lineNumber)
- }
- lineNumber++
- subject := scanner.Text()
-
- if !scanner.Scan() {
- return skerr.Fmt("Ran out of input, expecting a timestamp line: %d", lineNumber)
- }
- lineNumber++
- timestampString := scanner.Text()
- ts, err := strconv.ParseInt(timestampString, 10, 64)
- if err != nil {
- return skerr.Fmt("Failed to parse timestamp %q at line %d", timestampString, lineNumber)
- }
- if err := f(Commit{
- CommitNumber: types.BadCommitNumber,
- GitHash: gitHash,
- Timestamp: ts,
- Author: author,
- Subject: subject}); err != nil {
- return skerr.Wrap(err)
- }
- }
- return skerr.Wrap(scanner.Err())
-}
-
-// pull does a git pull on the git repo.
-func pull(ctx context.Context, gitFullPath, dir string) error {
- ctx, span := trace.StartSpan(ctx, "perfgit.pull")
- defer span.End()
-
- cmd := exec.CommandContext(ctx, gitFullPath, "pull")
- cmd.Dir = dir
- if err := cmd.Run(); err != nil {
- exerr := err.(*exec.ExitError)
- return skerr.Wrapf(err, "Failed to pull repo %q with git %q: %s", dir, gitFullPath, exerr.Stderr)
- }
- return nil
-}
-
// Update does a git pull and then finds all the new commits
// added to the repo since our last Update.
//
@@ -383,38 +249,24 @@
sklog.Infof("perfgit: Update called.")
g.updateCalled.Inc(1)
- if err := pull(ctx, g.gitFullPath, g.instanceConfig.GitRepoConfig.Dir); err != nil {
+ if err := g.gp.Update(ctx); err != nil {
return skerr.Wrap(err)
}
- var cmd *exec.Cmd
mostRecentGitHash, mostRecentCommitNumber, err := g.getMostRecentCommit(ctx)
nextCommitNumber := mostRecentCommitNumber + 1
if err != nil {
// If the Commits table is empty then start populating it from the very
// first commit to the repo.
if err == pgx.ErrNoRows {
- cmd = exec.CommandContext(ctx, g.gitFullPath, "rev-list", "HEAD", `--pretty=%aN <%aE>%n%s%n%ct`, "--reverse")
+ mostRecentGitHash = ""
nextCommitNumber = types.CommitNumber(0)
} else {
return skerr.Wrapf(err, "Failed looking up most recect commit.")
}
- } else {
- // Add all the commits from the repo since the last time we looked.
- cmd = exec.CommandContext(ctx, g.gitFullPath, "rev-list", "HEAD", "^"+mostRecentGitHash, `--pretty=%aN <%aE>%n%s%n%ct`, "--reverse")
- }
- sklog.Infof("perfgit: Starting update with nextCommitNumber: %d", nextCommitNumber)
- cmd.Dir = g.instanceConfig.GitRepoConfig.Dir
-
- stdout, err := cmd.StdoutPipe()
- if err != nil {
- return skerr.Wrap(err)
- }
- if err := cmd.Start(); err != nil {
- return skerr.Wrap(err)
}
total := 0
- err = parseGitRevLogStream(stdout, func(p Commit) error {
+ return g.gp.CommitsFromMostRecentGitHashToHead(ctx, mostRecentGitHash, func(p provider.Commit) error {
// Add p to the database starting at nextCommitNumber.
_, err := g.db.Exec(ctx, statements[insert], nextCommitNumber, p.GitHash, p.Timestamp, p.Author, p.Subject)
if err != nil {
@@ -426,19 +278,8 @@
sklog.Infof("Added %d commits this update cycle.", total)
}
return nil
- })
- if err != nil {
- // Once we've successfully called cmd.Start() we must always call
- // cmd.Wait() to close stdout.
- _ = cmd.Wait()
- return skerr.Wrap(err)
- }
- if err := cmd.Wait(); err != nil {
- exerr := err.(*exec.ExitError)
- return skerr.Wrapf(err, "Failed to pull repo: %s", exerr.Stderr)
- }
- return nil
+ })
}
// getMostRecentCommit as seen in the database.
@@ -469,7 +310,7 @@
}
// urlFromParts creates the URL to link to a specific commit in a repo.
-func urlFromParts(instanceConfig *config.InstanceConfig, commit Commit) string {
+func urlFromParts(instanceConfig *config.InstanceConfig, commit provider.Commit) string {
if instanceConfig.GitRepoConfig.DebouceCommitURL {
return commit.Subject
}
@@ -482,15 +323,15 @@
}
// CommitFromCommitNumber returns all the stored details for a given CommitNumber.
-func (g *Git) CommitFromCommitNumber(ctx context.Context, commitNumber types.CommitNumber) (Commit, error) {
+func (g *Git) CommitFromCommitNumber(ctx context.Context, commitNumber types.CommitNumber) (provider.Commit, error) {
ctx, span := trace.StartSpan(ctx, "perfgit.CommitFromCommitNumber")
defer span.End()
g.commitFromCommitNumberCalled.Inc(1)
if iCommit, ok := g.cache.Get(commitNumber); ok {
- return iCommit.(Commit), nil
+ return iCommit.(provider.Commit), nil
}
- var ret Commit
+ var ret provider.Commit
if err := g.db.QueryRow(ctx, statements[getDetails], commitNumber).Scan(&ret.GitHash, &ret.Timestamp, &ret.Author, &ret.Subject); err != nil {
return ret, skerr.Wrapf(err, "Failed to get details for CommitNumber: %d", commitNumber)
}
@@ -502,12 +343,12 @@
}
// CommitSliceFromCommitNumberSlice returns all the stored details for a given slice of CommitNumbers.
-func (g *Git) CommitSliceFromCommitNumberSlice(ctx context.Context, commitNumberSlice []types.CommitNumber) ([]Commit, error) {
+func (g *Git) CommitSliceFromCommitNumberSlice(ctx context.Context, commitNumberSlice []types.CommitNumber) ([]provider.Commit, error) {
ctx, span := trace.StartSpan(ctx, "perfgit.CommitSliceFromCommitNumberSlice")
defer span.End()
g.commitSliceFromCommitNumberSlice.Inc(1)
- ret := make([]Commit, len(commitNumberSlice))
+ ret := make([]provider.Commit, len(commitNumberSlice))
for i, commitNumber := range commitNumberSlice {
details, err := g.CommitFromCommitNumber(ctx, commitNumber)
if err != nil {
@@ -543,7 +384,7 @@
// CommitSliceFromTimeRange returns a slice of Commits that fall in the range
// [begin, end), i.e inclusive of begin and exclusive of end.
-func (g *Git) CommitSliceFromTimeRange(ctx context.Context, begin, end time.Time) ([]Commit, error) {
+func (g *Git) CommitSliceFromTimeRange(ctx context.Context, begin, end time.Time) ([]provider.Commit, error) {
ctx, span := trace.StartSpan(ctx, "perfgit.CommitSliceFromTimeRange")
defer span.End()
@@ -553,9 +394,9 @@
return nil, skerr.Wrapf(err, "Failed to query for commit slice in range %s-%s", begin, end)
}
defer rows.Close()
- ret := []Commit{}
+ ret := []provider.Commit{}
for rows.Next() {
- var c Commit
+ var c provider.Commit
if err := rows.Scan(&c.CommitNumber, &c.GitHash, &c.Timestamp, &c.Author, &c.Subject); err != nil {
return nil, skerr.Wrapf(err, "Failed to read row in range %s-%s", begin, end)
}
@@ -566,7 +407,7 @@
// CommitSliceFromCommitNumberRange returns a slice of Commits that fall in the range
// [begin, end], i.e inclusive of both begin and end.
-func (g *Git) CommitSliceFromCommitNumberRange(ctx context.Context, begin, end types.CommitNumber) ([]Commit, error) {
+func (g *Git) CommitSliceFromCommitNumberRange(ctx context.Context, begin, end types.CommitNumber) ([]provider.Commit, error) {
ctx, span := trace.StartSpan(ctx, "perfgit.CommitSliceFromCommitNumberRange")
defer span.End()
@@ -576,9 +417,9 @@
return nil, skerr.Wrapf(err, "Failed to query for commit slice in range %v-%v", begin, end)
}
defer rows.Close()
- ret := []Commit{}
+ ret := []provider.Commit{}
for rows.Next() {
- var c Commit
+ var c provider.Commit
if err := rows.Scan(&c.CommitNumber, &c.GitHash, &c.Timestamp, &c.Author, &c.Subject); err != nil {
return nil, skerr.Wrapf(err, "Failed to read row in range %v-%v", begin, end)
}
@@ -608,43 +449,29 @@
defer span.End()
g.commitNumbersWhenFileChangesInCommitNumberRangeCalled.Inc(1)
- var revisionRange string
+ // Default to beginHash being the empty string, which means start at the
+ // beginning of the repo's history.
+ var beginHash string
+ // Covert the commit numbers to hashes.
+ if begin != types.BadCommitNumber && begin-1 != types.BadCommitNumber {
+ var err error
+ beginHash, err = g.GitHashFromCommitNumber(ctx, begin-1)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ }
+
endHash, err := g.GitHashFromCommitNumber(ctx, end)
if err != nil {
return nil, skerr.Wrap(err)
}
- if begin == types.CommitNumber(0) {
- // git log revision range queries of the form hash1..hash2 are exclusive
- // of hash1, so we need to always back up begin one commit, except in
- // the case where the commit number is 0, then we change the revision
- // range.
- revisionRange = endHash
- } else {
- // Covert the commit numbers to hashes.
- beginHash, err := g.GitHashFromCommitNumber(ctx, begin-1)
- if err != nil {
- return nil, skerr.Wrap(err)
- }
- revisionRange = beginHash + ".." + endHash
- }
- // Build the git log command to run.
- cmd := exec.CommandContext(ctx, g.gitFullPath, "log", revisionRange, "--reverse", "--format=format:%H", "--", filename)
- cmd.Dir = g.instanceConfig.GitRepoConfig.Dir
-
- stdout, err := cmd.StdoutPipe()
+ hashes, err := g.gp.GitHashesInRangeForFile(ctx, beginHash, endHash, filename)
if err != nil {
return nil, skerr.Wrap(err)
}
- if err := cmd.Start(); err != nil {
- return nil, skerr.Wrap(err)
- }
-
- // Read the git log output.
- scanner := bufio.NewScanner(stdout)
- ret := []types.CommitNumber{}
- for scanner.Scan() {
- githash := scanner.Text()
+ var ret []types.CommitNumber
+ for _, githash := range hashes {
commitNumber, err := g.CommitNumberFromGitHash(ctx, githash)
if err != nil {
return nil, skerr.Wrapf(err, "git log returned invalid git hash: %q", githash)
@@ -652,18 +479,6 @@
ret = append(ret, commitNumber)
}
- if scanner.Err() != nil {
- // Once we've successfully called cmd.Start() we must always call
- // cmd.Wait() to close stdout.
- _ = cmd.Wait()
- return nil, skerr.Wrap(err)
- }
-
- if err := cmd.Wait(); err != nil {
- exerr := err.(*exec.ExitError)
- return nil, skerr.Wrapf(err, "Failed to get logs: %s", exerr.Stderr)
- }
-
return ret, nil
}
@@ -673,18 +488,5 @@
if err != nil {
return "", skerr.Wrap(err)
}
-
- // Build the git log command to run.
- cmd := exec.CommandContext(ctx, g.gitFullPath, "show", "-s", hash)
- cmd.Dir = g.instanceConfig.GitRepoConfig.Dir
- var out bytes.Buffer
- cmd.Stdout = &out
- var stderr bytes.Buffer
- cmd.Stderr = &stderr
-
- if err := cmd.Run(); err != nil {
- return "", skerr.Wrapf(err, "Failed running %q: stdout: %q stderr: %q", cmd.String(), out.String(), stderr.String())
- }
-
- return out.String(), nil
+ return g.gp.LogEntry(ctx, hash)
}
diff --git a/perf/go/git/git_test.go b/perf/go/git/git_test.go
index fd4cac6..7cbc0c7 100644
--- a/perf/go/git/git_test.go
+++ b/perf/go/git/git_test.go
@@ -4,8 +4,6 @@
import (
"context"
- "fmt"
- "io/ioutil"
"strings"
"testing"
"time"
@@ -15,13 +13,14 @@
"go.skia.org/infra/go/git/testutils"
"go.skia.org/infra/perf/go/config"
"go.skia.org/infra/perf/go/git/gittest"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/types"
)
func TestCockroachDB(t *testing.T) {
for name, subTest := range subTests {
t.Run(name, func(t *testing.T) {
- ctx, db, gb, hashes, instanceConfig := gittest.NewForTest(t)
+ ctx, db, gb, hashes, _, instanceConfig := gittest.NewForTest(t)
g, err := New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -99,7 +98,7 @@
// The prefix of the URL will change, so just confirm it has the right suffix.
require.True(t, strings.HasSuffix(commit.URL, commit.GitHash))
- assert.Equal(t, Commit{
+ assert.Equal(t, provider.Commit{
Timestamp: gittest.StartTime.Add(time.Minute).Unix(),
GitHash: hashes[1],
Author: "test <test@google.com>",
@@ -127,7 +126,7 @@
require.True(t, strings.HasSuffix(commits[0].URL, commits[0].GitHash))
require.True(t, strings.HasSuffix(commits[1].URL, commits[1].GitHash))
- assert.Equal(t, Commit{
+ assert.Equal(t, provider.Commit{
Timestamp: gittest.StartTime.Add(time.Minute).Unix(),
GitHash: hashes[1],
Author: "test <test@google.com>",
@@ -135,7 +134,7 @@
URL: commits[0].URL,
CommitNumber: commitNumbers[0],
}, commits[0])
- assert.Equal(t, Commit{
+ assert.Equal(t, provider.Commit{
Timestamp: gittest.StartTime.Add(3 * time.Minute).Unix(),
GitHash: hashes[3],
Author: "test <test@google.com>",
@@ -291,125 +290,6 @@
require.Error(t, err)
}
-func TestParseGitRevLogStream_Success(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0
-Joe Gregorio <joe@bitworking.org>
-Change #9
-1584837783`)
-
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Equal(t, Commit{
- CommitNumber: types.BadCommitNumber,
- GitHash: "6079a7810530025d9877916895dd14eb8bb454c0",
- Timestamp: 1584837783,
- Author: "Joe Gregorio <joe@bitworking.org>",
- Subject: "Change #9"}, p)
- return nil
- })
- assert.NoError(t, err)
-}
-
-func TestParseGitRevLogStream_ErrPropagatesWhenCallbackReturnsError(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0
-Joe Gregorio <joe@bitworking.org>
-Change #9
-1584837783`)
-
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- return fmt.Errorf("This is an error.")
- })
- assert.Contains(t, err.Error(), "This is an error.")
-}
-
-func TestParseGitRevLogStream_SuccessForTwoCommits(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0
-Joe Gregorio <joe@bitworking.org>
-Change #9
-1584837783
-commit 977e0ef44bec17659faf8c5d4025c5a068354817
-Joe Gregorio <joe@bitworking.org>
-Change #8
-1584837780`)
- count := 0
- hashes := []string{"6079a7810530025d9877916895dd14eb8bb454c0", "977e0ef44bec17659faf8c5d4025c5a068354817"}
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Equal(t, "Joe Gregorio <joe@bitworking.org>", p.Author)
- assert.Equal(t, hashes[count], p.GitHash)
- count++
- return nil
- })
- assert.Equal(t, 2, count)
- assert.NoError(t, err)
-}
-
-func TestParseGitRevLogStream_EmptyFile_Success(t *testing.T) {
- r := strings.NewReader("")
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Fail(t, "Should never get here.")
- return nil
- })
- assert.NoError(t, err)
-}
-
-func TestParseGitRevLogStream_ErrMissingTimestamp(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0
-Joe Gregorio <joe@bitworking.org>
-Change #9`)
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Fail(t, "Should never get here.")
- return nil
- })
- assert.Contains(t, err.Error(), "expecting a timestamp")
-}
-
-func TestParseGitRevLogStream_ErrFailedToParseTimestamp(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0
-Joe Gregorio <joe@bitworking.org>
-Change #9
-ooops 1584837780`)
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Fail(t, "Should never get here.")
- return nil
- })
- assert.Contains(t, err.Error(), "Failed to parse timestamp")
-}
-
-func TestParseGitRevLogStream_ErrMissingSubject(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0
-Joe Gregorio <joe@bitworking.org>`)
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Fail(t, "Should never get here.")
- return nil
- })
- assert.Contains(t, err.Error(), "expecting a subject")
-}
-
-func TestParseGitRevLogStream_ErrMissingAuthor(t *testing.T) {
- r := strings.NewReader(
- `commit 6079a7810530025d9877916895dd14eb8bb454c0`)
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Fail(t, "Should never get here.")
- return nil
- })
- assert.Contains(t, err.Error(), "expecting an author")
-}
-
-func TestParseGitRevLogStream_ErrMalformedCommitLine(t *testing.T) {
- r := strings.NewReader(
- `something_not_commit 6079a7810530025d9877916895dd14eb8bb454c0`)
- err := parseGitRevLogStream(ioutil.NopCloser(r), func(p Commit) error {
- assert.Fail(t, "Should never get here.")
- return nil
- })
- assert.Contains(t, err.Error(), "expected commit at")
-}
-
func TestURLFromParts_DebounceCommitURL_Success(t *testing.T) {
const debounceURL = "https://some.other.url.example.org"
@@ -419,7 +299,7 @@
DebouceCommitURL: true,
},
}
- commit := Commit{
+ commit := provider.Commit{
GitHash: "6079a7810530025d9877916895dd14eb8bb454c0",
Subject: debounceURL,
}
@@ -434,7 +314,7 @@
CommitURL: "%s/commit/%s",
},
}
- commit := Commit{
+ commit := provider.Commit{
GitHash: "6079a7810530025d9877916895dd14eb8bb454c0",
}
assert.Equal(t, "https://github.com/google/skia/commit/6079a7810530025d9877916895dd14eb8bb454c0", urlFromParts(instanceConfig, commit))
@@ -447,7 +327,7 @@
URL: "https://skia.googlesource.com/skia",
},
}
- commit := Commit{
+ commit := provider.Commit{
GitHash: "6079a7810530025d9877916895dd14eb8bb454c0",
}
assert.Equal(t, "https://skia.googlesource.com/skia/+show/6079a7810530025d9877916895dd14eb8bb454c0", urlFromParts(instanceConfig, commit))
@@ -455,7 +335,7 @@
func TestCommit_Display(t *testing.T) {
- c := Commit{
+ c := provider.Commit{
CommitNumber: 10223,
GitHash: "d261e1075a93677442fdf7fe72aba7e583863664",
Timestamp: 1498176000,
diff --git a/perf/go/git/gittest/BUILD.bazel b/perf/go/git/gittest/BUILD.bazel
index 5317ef9..ec882a9 100644
--- a/perf/go/git/gittest/BUILD.bazel
+++ b/perf/go/git/gittest/BUILD.bazel
@@ -9,6 +9,8 @@
"//bazel/external/cipd/git",
"//go/git/testutils",
"//perf/go/config",
+ "//perf/go/git/provider",
+ "//perf/go/git/providers/git_checkout",
"//perf/go/sql/sqltest",
"@com_github_jackc_pgx_v4//pgxpool",
"@com_github_stretchr_testify//assert",
diff --git a/perf/go/git/gittest/gittest.go b/perf/go/git/gittest/gittest.go
index 0526718..9cac4fe 100644
--- a/perf/go/git/gittest/gittest.go
+++ b/perf/go/git/gittest/gittest.go
@@ -15,6 +15,8 @@
cipd_git "go.skia.org/infra/bazel/external/cipd/git"
"go.skia.org/infra/go/git/testutils"
"go.skia.org/infra/perf/go/config"
+ "go.skia.org/infra/perf/go/git/provider"
+ "go.skia.org/infra/perf/go/git/providers/git_checkout"
"go.skia.org/infra/perf/go/sql/sqltest"
)
@@ -34,7 +36,7 @@
// The repo is populated with 8 commits, one minute apart, starting at StartTime.
//
// The hashes for each commit are going to be random and so are returned also.
-func NewForTest(t *testing.T) (context.Context, *pgxpool.Pool, *testutils.GitBuilder, []string, *config.InstanceConfig) {
+func NewForTest(t *testing.T) (context.Context, *pgxpool.Pool, *testutils.GitBuilder, []string, provider.Provider, *config.InstanceConfig) {
ctx := cipd_git.UseGitFinder(context.Background())
ctx, cancel := context.WithCancel(ctx)
@@ -71,5 +73,7 @@
Dir: filepath.Join(tmpDir, "checkout"),
},
}
- return ctx, db, gb, hashes, instanceConfig
+ gp, err := git_checkout.New(ctx, instanceConfig)
+ require.NoError(t, err)
+ return ctx, db, gb, hashes, gp, instanceConfig
}
diff --git a/perf/go/git/provider/BUILD.bazel b/perf/go/git/provider/BUILD.bazel
new file mode 100644
index 0000000..232306f
--- /dev/null
+++ b/perf/go/git/provider/BUILD.bazel
@@ -0,0 +1,12 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "provider",
+ srcs = ["provider.go"],
+ importpath = "go.skia.org/infra/perf/go/git/provider",
+ visibility = ["//visibility:public"],
+ deps = [
+ "//go/human",
+ "//perf/go/types",
+ ],
+)
diff --git a/perf/go/git/provider/provider.go b/perf/go/git/provider/provider.go
new file mode 100644
index 0000000..1b17db1
--- /dev/null
+++ b/perf/go/git/provider/provider.go
@@ -0,0 +1,57 @@
+// Package provider contains types and interfaces for interacting with Git
+// repos.
+package provider
+
+import (
+ "context"
+ "fmt"
+ "time"
+
+ "go.skia.org/infra/go/human"
+ "go.skia.org/infra/perf/go/types"
+)
+
+// Commit represents a single commit stored in the database.
+//
+// JSON annotations make it serialize like the legacy cid.CommitDetail.
+type Commit struct {
+ CommitNumber types.CommitNumber `json:"offset"`
+ GitHash string `json:"hash"`
+ Timestamp int64 `json:"ts"` // Unix timestamp, seconds from the epoch.
+ Author string `json:"author"`
+ Subject string `json:"message"`
+ URL string `json:"url"`
+}
+
+// Display returns a display string that describes the commit.
+func (c Commit) Display(now time.Time) string {
+ return fmt.Sprintf("%s - %s - %s", c.GitHash[:7], human.Duration(now.Sub(time.Unix(c.Timestamp, 0))), c.Subject)
+}
+
+// CommitProcessor is a callback function that will be called with a Commit.
+// Used in GitProvider.
+type CommitProcessor func(c Commit) error
+
+// Provider in abstraction of how we get information about a repo. This could
+// be implemented by either Git or the Gitiles API.
+type Provider interface {
+ // CommitsFromMostRecentGitHashToHead will call the `cb` func with every
+ // Commit, starting from the oldest and going to the newest. If
+ // mostRecentGitHash is the empty string then the commits will start with
+ // the very first commit to the repo (on HEAD, aka using --first-parent
+ // semantics).
+ CommitsFromMostRecentGitHashToHead(ctx context.Context, mostRecentGitHash string, cb CommitProcessor) error
+
+ // GitHashesInRangeForFile returns all the git hashes when the given file
+ // has changed between [begin, end], i.e. the given range is exclusive of
+ // the begin commit and inclusive of the end commit. If 'begin' is the empty
+ // string then the scan should go back to the initial commit of the repo.
+ GitHashesInRangeForFile(ctx context.Context, begin, end, filename string) ([]string, error)
+
+ // LogEntry returns the full log entry of a commit (minus the diff) as a string.
+ LogEntry(ctx context.Context, gitHash string) (string, error)
+
+ // Update does any necessary work, like a `git pull`, to ensure that the
+ // GitProvider has the most recent commits available.
+ Update(ctx context.Context) error
+}
diff --git a/perf/go/git/providers/BUILD.bazel b/perf/go/git/providers/BUILD.bazel
new file mode 100644
index 0000000..ff1ea91
--- /dev/null
+++ b/perf/go/git/providers/BUILD.bazel
@@ -0,0 +1,15 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "providers",
+ srcs = ["builder.go"],
+ importpath = "go.skia.org/infra/perf/go/git/providers",
+ visibility = ["//visibility:public"],
+ deps = [
+ "//go/skerr",
+ "//go/util",
+ "//perf/go/config",
+ "//perf/go/git/provider",
+ "//perf/go/git/providers/git_checkout",
+ ],
+)
diff --git a/perf/go/git/providers/builder.go b/perf/go/git/providers/builder.go
new file mode 100644
index 0000000..1f42f30
--- /dev/null
+++ b/perf/go/git/providers/builder.go
@@ -0,0 +1,20 @@
+// Package providers builds different kinds of provider.Provider.
+package providers
+
+import (
+ "context"
+
+ "go.skia.org/infra/go/skerr"
+ "go.skia.org/infra/go/util"
+ "go.skia.org/infra/perf/go/config"
+ "go.skia.org/infra/perf/go/git/provider"
+ "go.skia.org/infra/perf/go/git/providers/git_checkout"
+)
+
+// New builds a Provider based on the instance config.
+func New(ctx context.Context, instanceConfig *config.InstanceConfig) (provider.Provider, error) {
+ if util.In(string(instanceConfig.GitRepoConfig.Provider), []string{"", string(config.GitProviderCLI)}) {
+ return git_checkout.New(ctx, instanceConfig)
+ }
+ return nil, skerr.Fmt("invalid type of Provider selected: %q expected one of %q", instanceConfig.GitRepoConfig.Provider, config.AllGitProviders)
+}
diff --git a/perf/go/git/providers/git_checkout/BUILD.bazel b/perf/go/git/providers/git_checkout/BUILD.bazel
new file mode 100644
index 0000000..7225356
--- /dev/null
+++ b/perf/go/git/providers/git_checkout/BUILD.bazel
@@ -0,0 +1,36 @@
+load("//bazel/go:go_test.bzl", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "git_checkout",
+ srcs = ["git_checkout.go"],
+ importpath = "go.skia.org/infra/perf/go/git/providers/git_checkout",
+ visibility = ["//visibility:public"],
+ deps = [
+ "//go/auth",
+ "//go/git/git_common",
+ "//go/gitauth",
+ "//go/skerr",
+ "//go/sklog",
+ "//perf/go/config",
+ "//perf/go/git/provider",
+ "//perf/go/types",
+ "@io_opencensus_go//trace",
+ "@org_golang_x_oauth2//google",
+ ],
+)
+
+go_test(
+ name = "git_checkout_test",
+ srcs = ["git_checkout_test.go"],
+ embed = [":git_checkout"],
+ deps = [
+ "//bazel/external/cipd/git",
+ "//go/git/testutils",
+ "//perf/go/config",
+ "//perf/go/git/provider",
+ "//perf/go/types",
+ "@com_github_stretchr_testify//assert",
+ "@com_github_stretchr_testify//require",
+ ],
+)
diff --git a/perf/go/git/providers/git_checkout/git_checkout.go b/perf/go/git/providers/git_checkout/git_checkout.go
new file mode 100644
index 0000000..1205e56
--- /dev/null
+++ b/perf/go/git/providers/git_checkout/git_checkout.go
@@ -0,0 +1,260 @@
+// Package git_checkout implements provider.Provider by shelling out to run git commands.
+package git_checkout
+
+import (
+ "bufio"
+ "bytes"
+ "context"
+ "io"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "strconv"
+ "strings"
+
+ "go.opencensus.io/trace"
+ "go.skia.org/infra/go/auth"
+ "go.skia.org/infra/go/git/git_common"
+ "go.skia.org/infra/go/gitauth"
+ "go.skia.org/infra/go/skerr"
+ "go.skia.org/infra/go/sklog"
+ "go.skia.org/infra/perf/go/config"
+ "go.skia.org/infra/perf/go/git/provider"
+ "go.skia.org/infra/perf/go/types"
+ "golang.org/x/oauth2/google"
+)
+
+// Impl implements provider.Provider.
+type Impl struct {
+ // gitFullPath is the path of the git executable.
+ gitFullPath string
+
+ // repoFullPath if the full path of the checked out Git repo.
+ repoFullPath string
+
+ // startCommit is the commit in the repo where we start tracking commits. If
+ // not supplied then we start with the first commit in the repo as reachable
+ // from HEAD.
+ startCommit string
+}
+
+// New returns a new instance of Impl, which implements provider.Provider.
+func New(ctx context.Context, instanceConfig *config.InstanceConfig) (*Impl, error) {
+
+ // Do git authentication if required.
+ if instanceConfig.GitRepoConfig.GitAuthType == config.GitAuthGerrit {
+ sklog.Info("Authenticating to Gerrit.")
+ ts, err := google.DefaultTokenSource(ctx, auth.ScopeGerrit)
+ if err != nil {
+ return nil, skerr.Wrapf(err, "Failed to get tokensource perfgit.Git for config %v", *instanceConfig)
+ }
+ if _, err := gitauth.New(ts, "/tmp/git-cookie", true, ""); err != nil {
+ return nil, skerr.Wrapf(err, "Failed to gitauth perfgit.Git for config %v", *instanceConfig)
+ }
+ }
+
+ // Find the path to the git executable, which might be relative to working dir.
+ gitFullPath, _, _, err := git_common.FindGit(ctx)
+ if err != nil {
+ return nil, skerr.Wrapf(err, "Failed to find git.")
+ }
+
+ // Force the path to be absolute.
+ gitFullPath, err = filepath.Abs(gitFullPath)
+ if err != nil {
+ return nil, skerr.Wrapf(err, "Failed to get absolute path to git.")
+ }
+
+ // Clone the git repo if necessary.
+ sklog.Infof("Cloning repo.")
+ if _, err := os.Stat(instanceConfig.GitRepoConfig.Dir); os.IsNotExist(err) {
+ cmd := exec.CommandContext(ctx, gitFullPath, "clone", instanceConfig.GitRepoConfig.URL, instanceConfig.GitRepoConfig.Dir)
+ if err := cmd.Run(); err != nil {
+ exerr := err.(*exec.ExitError)
+ return nil, skerr.Wrapf(err, "Failed to clone repo: %s - %s", err, exerr.Stderr)
+ }
+ }
+
+ return &Impl{
+ gitFullPath: gitFullPath,
+ repoFullPath: instanceConfig.GitRepoConfig.Dir,
+ startCommit: instanceConfig.GitRepoConfig.StartCommit,
+ }, nil
+}
+
+// CommitsFromMostRecentGitHashToHead implements provider.Provider.
+func (i Impl) CommitsFromMostRecentGitHashToHead(ctx context.Context, mostRecentGitHash string, cb provider.CommitProcessor) error {
+ var cmd *exec.Cmd
+ if mostRecentGitHash == "" {
+ mostRecentGitHash = i.startCommit
+ }
+ if mostRecentGitHash == "" {
+ cmd = exec.CommandContext(ctx, i.gitFullPath, "rev-list", "HEAD", `--pretty=%aN <%aE>%n%s%n%ct`, "--reverse")
+ } else {
+ // Add all the commits from the repo since the last time we looked.
+ cmd = exec.CommandContext(ctx, i.gitFullPath, "rev-list", "HEAD", "^"+mostRecentGitHash, `--pretty=%aN <%aE>%n%s%n%ct`, "--reverse")
+ }
+
+ cmd.Dir = i.repoFullPath
+ stdout, err := cmd.StdoutPipe()
+ if err != nil {
+ return skerr.Wrap(err)
+ }
+ if err := cmd.Start(); err != nil {
+ return skerr.Wrap(err)
+ }
+
+ err = parseGitRevLogStream(stdout, func(p provider.Commit) error {
+ return cb(p)
+ })
+ if err != nil {
+ // Once we've successfully called cmd.Start() we must always call
+ // cmd.Wait() to close stdout.
+ _ = cmd.Wait()
+ return skerr.Wrap(err)
+ }
+
+ return nil
+}
+
+// GitHashesInRangeForFile implements provider.Provider.
+func (i Impl) GitHashesInRangeForFile(ctx context.Context, begin, end, filename string) ([]string, error) {
+ var revisionRange string
+ if begin == "" {
+ begin = i.startCommit
+ }
+ if begin == "" {
+ // git log revision range queries of the form hash1..hash2 are exclusive
+ // of hash1, so we need to always back up begin one commit, except in
+ // the case where the commit number is 0, then we change the revision
+ // range.
+ revisionRange = end
+ } else {
+ revisionRange = begin + ".." + end
+ }
+
+ // Build the git log command to run.
+ cmd := exec.CommandContext(ctx, i.gitFullPath, "log", revisionRange, "--reverse", "--format=format:%H", "--", filename)
+ cmd.Dir = i.repoFullPath
+
+ stdout, err := cmd.StdoutPipe()
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ if err := cmd.Start(); err != nil {
+ return nil, skerr.Wrap(err)
+ }
+
+ // Read the git log output.
+ scanner := bufio.NewScanner(stdout)
+ ret := []string{}
+ for scanner.Scan() {
+ ret = append(ret, scanner.Text())
+ }
+
+ if scanner.Err() != nil {
+ // Once we've successfully called cmd.Start() we must always call
+ // cmd.Wait() to close stdout.
+ _ = cmd.Wait()
+ return nil, skerr.Wrap(err)
+ }
+
+ if err := cmd.Wait(); err != nil {
+ exerr := err.(*exec.ExitError)
+ return nil, skerr.Wrapf(err, "Failed to get logs: %s", exerr.Stderr)
+ }
+ return ret, nil
+}
+
+// LogEntry implements provider.Provider.
+func (i Impl) LogEntry(ctx context.Context, hash string) (string, error) {
+ // Build the git log command to run.
+ cmd := exec.CommandContext(ctx, i.gitFullPath, "show", "-s", hash)
+ cmd.Dir = i.repoFullPath
+ var out bytes.Buffer
+ cmd.Stdout = &out
+ var stderr bytes.Buffer
+ cmd.Stderr = &stderr
+
+ if err := cmd.Run(); err != nil {
+ return "", skerr.Wrapf(err, "Failed running %q: stdout: %q stderr: %q", cmd.String(), out.String(), stderr.String())
+ }
+
+ return out.String(), nil
+}
+
+type parseGitRevLogStreamProcessSingleCommit func(commit provider.Commit) error
+
+// parseGitRevLogStream parses the input stream for input of the form:
+//
+// commit 6079a7810530025d9877916895dd14eb8bb454c0
+// Joe Gregorio <joe@bitworking.org>
+// Change #9
+// 1584837783
+// commit 977e0ef44bec17659faf8c5d4025c5a068354817
+// Joe Gregorio <joe@bitworking.org>
+// Change #8
+// 1584837783
+//
+// And calls the parseGitRevLogStreamProcessSingleCommit function with each
+// entry it finds. The passed in Commit has all valid fields except
+// CommitNumber, which is set to types.BadCommitNumber.
+func parseGitRevLogStream(r io.ReadCloser, f parseGitRevLogStreamProcessSingleCommit) error {
+ scanner := bufio.NewScanner(r)
+ lineNumber := 0
+ for scanner.Scan() {
+ line := scanner.Text()
+ if !strings.HasPrefix(line, "commit ") {
+ return skerr.Fmt("Invalid format, expected commit at line %d: %q", lineNumber, line)
+ }
+ lineNumber++
+ gitHash := strings.Split(line, " ")[1]
+
+ if !scanner.Scan() {
+ return skerr.Fmt("Ran out of input, expecting an author line: %d", lineNumber)
+ }
+ lineNumber++
+ author := scanner.Text()
+
+ if !scanner.Scan() {
+ return skerr.Fmt("Ran out of input, expecting a subject line: %d", lineNumber)
+ }
+ lineNumber++
+ subject := scanner.Text()
+
+ if !scanner.Scan() {
+ return skerr.Fmt("Ran out of input, expecting a timestamp line: %d", lineNumber)
+ }
+ lineNumber++
+ timestampString := scanner.Text()
+ ts, err := strconv.ParseInt(timestampString, 10, 64)
+ if err != nil {
+ return skerr.Fmt("Failed to parse timestamp %q at line %d", timestampString, lineNumber)
+ }
+ if err := f(provider.Commit{
+ CommitNumber: types.BadCommitNumber,
+ GitHash: gitHash,
+ Timestamp: ts,
+ Author: author,
+ Subject: subject}); err != nil {
+ return skerr.Wrap(err)
+ }
+ }
+ return skerr.Wrap(scanner.Err())
+}
+
+// Update implements provider.Provider.
+func (i Impl) Update(ctx context.Context) error {
+ ctx, span := trace.StartSpan(ctx, "perfgit.pull")
+ defer span.End()
+
+ cmd := exec.CommandContext(ctx, i.gitFullPath, "pull")
+ cmd.Dir = i.repoFullPath
+ if err := cmd.Run(); err != nil {
+ exerr := err.(*exec.ExitError)
+ return skerr.Wrapf(err, "Failed to pull repo %q with git %q: %s", i.repoFullPath, i.gitFullPath, exerr.Stderr)
+ }
+ return nil
+}
+
+var _ provider.Provider = Impl{}
diff --git a/perf/go/git/providers/git_checkout/git_checkout_test.go b/perf/go/git/providers/git_checkout/git_checkout_test.go
new file mode 100644
index 0000000..41a902a
--- /dev/null
+++ b/perf/go/git/providers/git_checkout/git_checkout_test.go
@@ -0,0 +1,311 @@
+package git_checkout
+
+import (
+ "context"
+ "fmt"
+ "io/ioutil"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ cipd_git "go.skia.org/infra/bazel/external/cipd/git"
+ "go.skia.org/infra/go/git/testutils"
+ "go.skia.org/infra/perf/go/config"
+ "go.skia.org/infra/perf/go/git/provider"
+ "go.skia.org/infra/perf/go/types"
+)
+
+var (
+ // StartTime is the time of the first commit.
+ StartTime = time.Unix(1680000000, 0)
+)
+
+// NewForTest returns all the necessary variables needed to test against infra/go/git.
+//
+// The repo is populated with 8 commits, one minute apart, starting at StartTime.
+//
+// The hashes for each commit are going to be random and so are returned also.
+func NewForTest(t *testing.T) (context.Context, *testutils.GitBuilder, []string, *config.InstanceConfig) {
+ ctx := cipd_git.UseGitFinder(context.Background())
+ ctx, cancel := context.WithCancel(ctx)
+
+ // Create a git repo for testing purposes.
+ gb := testutils.GitInit(t, ctx)
+ hashes := []string{}
+ hashes = append(hashes, gb.CommitGenAt(ctx, "foo.txt", StartTime))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "foo.txt", StartTime.Add(time.Minute)))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "foo.txt", StartTime.Add(2*time.Minute)))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "bar.txt", StartTime.Add(3*time.Minute)))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "foo.txt", StartTime.Add(4*time.Minute)))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "foo.txt", StartTime.Add(5*time.Minute)))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "bar.txt", StartTime.Add(6*time.Minute)))
+ hashes = append(hashes, gb.CommitGenAt(ctx, "foo.txt", StartTime.Add(7*time.Minute)))
+
+ // Get tmp dir to use for repo checkout.
+ tmpDir, err := ioutil.TempDir("", "git")
+ require.NoError(t, err)
+
+ // Create the cleanup function.
+ t.Cleanup(func() {
+ cancel()
+ err = os.RemoveAll(tmpDir)
+ assert.NoError(t, err)
+ gb.Cleanup()
+ })
+
+ instanceConfig := &config.InstanceConfig{
+ GitRepoConfig: config.GitRepoConfig{
+ URL: gb.Dir(),
+ Dir: filepath.Join(tmpDir, "checkout"),
+ },
+ }
+ return ctx, gb, hashes, instanceConfig
+}
+
+func TestParseGitRevLogStream_Success(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0
+Joe Gregorio <joe@bitworking.org>
+Change #9
+1584837783`)
+
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Equal(t, provider.Commit{
+ CommitNumber: types.BadCommitNumber,
+ GitHash: "6079a7810530025d9877916895dd14eb8bb454c0",
+ Timestamp: 1584837783,
+ Author: "Joe Gregorio <joe@bitworking.org>",
+ Subject: "Change #9"}, p)
+ return nil
+ })
+ assert.NoError(t, err)
+}
+
+func TestParseGitRevLogStream_ErrPropagatesWhenCallbackReturnsError(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0
+Joe Gregorio <joe@bitworking.org>
+Change #9
+1584837783`)
+
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ return fmt.Errorf("This is an error.")
+ })
+ assert.Contains(t, err.Error(), "This is an error.")
+}
+
+func TestParseGitRevLogStream_SuccessForTwoCommits(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0
+Joe Gregorio <joe@bitworking.org>
+Change #9
+1584837783
+commit 977e0ef44bec17659faf8c5d4025c5a068354817
+Joe Gregorio <joe@bitworking.org>
+Change #8
+1584837780`)
+ count := 0
+ hashes := []string{"6079a7810530025d9877916895dd14eb8bb454c0", "977e0ef44bec17659faf8c5d4025c5a068354817"}
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Equal(t, "Joe Gregorio <joe@bitworking.org>", p.Author)
+ assert.Equal(t, hashes[count], p.GitHash)
+ count++
+ return nil
+ })
+ assert.Equal(t, 2, count)
+ assert.NoError(t, err)
+}
+
+func TestParseGitRevLogStream_EmptyFile_Success(t *testing.T) {
+ r := strings.NewReader("")
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Fail(t, "Should never get here.")
+ return nil
+ })
+ assert.NoError(t, err)
+}
+
+func TestParseGitRevLogStream_ErrMissingTimestamp(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0
+Joe Gregorio <joe@bitworking.org>
+Change #9`)
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Fail(t, "Should never get here.")
+ return nil
+ })
+ assert.Contains(t, err.Error(), "expecting a timestamp")
+}
+
+func TestParseGitRevLogStream_ErrFailedToParseTimestamp(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0
+Joe Gregorio <joe@bitworking.org>
+Change #9
+ooops 1584837780`)
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Fail(t, "Should never get here.")
+ return nil
+ })
+ assert.Contains(t, err.Error(), "Failed to parse timestamp")
+}
+
+func TestParseGitRevLogStream_ErrMissingSubject(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0
+Joe Gregorio <joe@bitworking.org>`)
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Fail(t, "Should never get here.")
+ return nil
+ })
+ assert.Contains(t, err.Error(), "expecting a subject")
+}
+
+func TestParseGitRevLogStream_ErrMissingAuthor(t *testing.T) {
+ r := strings.NewReader(
+ `commit 6079a7810530025d9877916895dd14eb8bb454c0`)
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Fail(t, "Should never get here.")
+ return nil
+ })
+ assert.Contains(t, err.Error(), "expecting an author")
+}
+
+func TestParseGitRevLogStream_ErrMalformedCommitLine(t *testing.T) {
+ r := strings.NewReader(
+ `something_not_commit 6079a7810530025d9877916895dd14eb8bb454c0`)
+ err := parseGitRevLogStream(ioutil.NopCloser(r), func(p provider.Commit) error {
+ assert.Fail(t, "Should never get here.")
+ return nil
+ })
+ assert.Contains(t, err.Error(), "expected commit at")
+}
+
+func TestLogEntry_Success(t *testing.T) {
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ got, err := g.LogEntry(ctx, hashes[1])
+ require.NoError(t, err)
+ expected := `commit 881dfc43620250859549bb7e0301b6910d9b8e70
+Author: test <test@google.com>
+Date: Tue Mar 28 10:41:00 2023 +0000
+
+ 501233450539197794
+`
+ require.Equal(t, expected, got)
+}
+
+func TestLogEntry_BadCommitId_ReturnsError(t *testing.T) {
+ ctx, _, _, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ _, err = g.LogEntry(ctx, "this-is-not-a-known-git-hash")
+ require.Error(t, err)
+}
+
+func TestUpdate_SuccessAndNewCommitAppears(t *testing.T) {
+ ctx, gb, _, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ _, err = g.LogEntry(ctx, "this-is-not-a-known-git-hash")
+ require.Error(t, err)
+
+ newHash := gb.CommitGenAt(ctx, "foo.txt", StartTime.Add(4*time.Minute))
+
+ err = g.Update(ctx)
+ require.NoError(t, err)
+ _, err = g.LogEntry(ctx, newHash)
+ require.NoError(t, err)
+}
+
+func TestGitHashesInRangeForFile_FileIsChangedAtBeginHash_BeginHashIsExcludedFromResponse(t *testing.T) {
+ // The 'bar.txt' file is only changed commit 3 and 6.
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ // GitHashesInRangeForFile is exclusive of 'begin', so it should not be in
+ // the results.
+ changedAt, err := g.GitHashesInRangeForFile(ctx, hashes[3], hashes[7], "bar.txt")
+ require.NoError(t, err)
+ require.Equal(t, []string{hashes[6]}, changedAt)
+}
+
+func TestGitHashesInRangeForFile_BeginHashIsEmpty_SearchGoesToBeginningOfRepoHistory(t *testing.T) {
+ // The 'bar.txt' file is only changed commit 3 and 6.
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ changedAt, err := g.GitHashesInRangeForFile(ctx, "", hashes[7], "bar.txt")
+ require.NoError(t, err)
+ require.Equal(t, []string{hashes[3], hashes[6]}, changedAt)
+}
+
+func TestGitHashesInRangeForFile_BeginHashIsEmptyButStartCommitIsSet_SearchGoesToBeginningOfRepoHistory(t *testing.T) {
+ // The 'bar.txt' file is only changed commit 3 and 6.
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ // We change the StartCommit to 3, so we should only see the change at 6.
+ instanceConfig.GitRepoConfig.StartCommit = hashes[3]
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ changedAt, err := g.GitHashesInRangeForFile(ctx, "", hashes[7], "bar.txt")
+ require.NoError(t, err)
+ require.Equal(t, []string{hashes[6]}, changedAt)
+}
+
+func TestCommitsFromMostRecentGitHashToHead_ProvideEmptyGitHash_ReceiveAllHashesInRepo(t *testing.T) {
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ err = g.CommitsFromMostRecentGitHashToHead(ctx, "", func(c provider.Commit) error {
+ require.Equal(t, hashes[0], c.GitHash)
+ hashes = hashes[1:]
+ return nil
+ })
+ require.NoError(t, err)
+}
+
+func TestCommitsFromMostRecentGitHashToHead_ProvideEmptyGitHashButStartCommitIsSet_ReceiveAllHashesInRepoStartingFromStartCommit(t *testing.T) {
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ instanceConfig.GitRepoConfig.StartCommit = hashes[2]
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ // StartCommit is set to 2, so we should get all commits after that.
+ expected := hashes[3:]
+ err = g.CommitsFromMostRecentGitHashToHead(ctx, "", func(c provider.Commit) error {
+ require.Equal(t, expected[0], c.GitHash)
+ expected = expected[1:]
+ return nil
+ })
+ require.NoError(t, err)
+}
+
+func TestCommitsFromMostRecentGitHashToHead_ProvideNonEmptyGitHash_ReceiveAllNewerHashesInRepo(t *testing.T) {
+ ctx, _, hashes, instanceConfig := NewForTest(t)
+ g, err := New(ctx, instanceConfig)
+ require.NoError(t, err)
+
+ fmt.Println(hashes)
+ // Note we use 3 here, because we pass hashes[2] below, so
+ // CommitsFromMostRecentGitHashToHead will return all commits newer than
+ // hashes[2] exclusive.
+ expected := hashes[3:]
+ err = g.CommitsFromMostRecentGitHashToHead(ctx, hashes[2], func(c provider.Commit) error {
+ require.Equal(t, expected[0], c.GitHash)
+ expected = expected[1:]
+ return nil
+ })
+ require.NoError(t, err)
+}
diff --git a/perf/go/git/providers/gitiles/BUILD.bazel b/perf/go/git/providers/gitiles/BUILD.bazel
new file mode 100644
index 0000000..7c53206
--- /dev/null
+++ b/perf/go/git/providers/gitiles/BUILD.bazel
@@ -0,0 +1,8 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "gitiles",
+ srcs = ["gitiles.go"],
+ importpath = "go.skia.org/infra/perf/go/git/providers/gitiles",
+ visibility = ["//visibility:public"],
+)
diff --git a/perf/go/git/providers/gitiles/gitiles.go b/perf/go/git/providers/gitiles/gitiles.go
new file mode 100644
index 0000000..23cfc59
--- /dev/null
+++ b/perf/go/git/providers/gitiles/gitiles.go
@@ -0,0 +1,4 @@
+// Package gitiles imlements provider.Provider using the Gitiles API.
+package gitiles
+
+// TODO(jcgregorio) Add implementation.
diff --git a/perf/go/notify/BUILD.bazel b/perf/go/notify/BUILD.bazel
index 6100d61..2c72d2c 100644
--- a/perf/go/notify/BUILD.bazel
+++ b/perf/go/notify/BUILD.bazel
@@ -12,7 +12,7 @@
"//go/sklog",
"//perf/go/alerts",
"//perf/go/clustering2",
- "//perf/go/git",
+ "//perf/go/git/provider",
"//perf/go/stepfit",
],
)
diff --git a/perf/go/notify/notify.go b/perf/go/notify/notify.go
index d068ae1..eb727bb 100644
--- a/perf/go/notify/notify.go
+++ b/perf/go/notify/notify.go
@@ -13,7 +13,7 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/perf/go/alerts"
"go.skia.org/infra/perf/go/clustering2"
- perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/stepfit"
)
@@ -101,12 +101,12 @@
// context is used in expanding the emailTemplate.
type templateContext struct {
URL string
- Commit perfgit.Commit
+ Commit provider.Commit
Alert *alerts.Alert
Cluster *clustering2.ClusterSummary
}
-func (n *Notifier) formatEmail(c perfgit.Commit, alert *alerts.Alert, cl *clustering2.ClusterSummary) (string, error) {
+func (n *Notifier) formatEmail(c provider.Commit, alert *alerts.Alert, cl *clustering2.ClusterSummary) (string, error) {
templateContext := &templateContext{
URL: n.url,
Commit: c,
@@ -132,7 +132,7 @@
}
// Send a notification for the given cluster found at the given commit. Where to send it is defined in the alerts.Config.
-func (n *Notifier) Send(ctx context.Context, c perfgit.Commit, alert *alerts.Alert, cl *clustering2.ClusterSummary) error {
+func (n *Notifier) Send(ctx context.Context, c provider.Commit, alert *alerts.Alert, cl *clustering2.ClusterSummary) error {
if alert.Alert == "" {
return fmt.Errorf("No notification sent. No email address set for alert #%s", alert.IDAsString)
}
@@ -150,7 +150,7 @@
// ExampleSend sends an example for dummy data for the given alerts.Config.
func (n *Notifier) ExampleSend(ctx context.Context, alert *alerts.Alert) error {
- c := perfgit.Commit{
+ c := provider.Commit{
Subject: "Re-enable opList dependency tracking",
URL: "https://skia.googlesource.com/skia/+show/d261e1075a93677442fdf7fe72aba7e583863664",
GitHash: "d261e1075a93677442fdf7fe72aba7e583863664",
diff --git a/perf/go/regression/BUILD.bazel b/perf/go/regression/BUILD.bazel
index 20699c3..4fc1990 100644
--- a/perf/go/regression/BUILD.bazel
+++ b/perf/go/regression/BUILD.bazel
@@ -23,6 +23,7 @@
"//perf/go/dataframe",
"//perf/go/dfiter",
"//perf/go/git",
+ "//perf/go/git/provider",
"//perf/go/progress",
"//perf/go/shortcut",
"//perf/go/stepfit",
diff --git a/perf/go/regression/continuous/BUILD.bazel b/perf/go/regression/continuous/BUILD.bazel
index 3879f2b..ee69b56 100644
--- a/perf/go/regression/continuous/BUILD.bazel
+++ b/perf/go/regression/continuous/BUILD.bazel
@@ -17,6 +17,7 @@
"//perf/go/config",
"//perf/go/dataframe",
"//perf/go/git",
+ "//perf/go/git/provider",
"//perf/go/ingestevents",
"//perf/go/notify",
"//perf/go/regression",
diff --git a/perf/go/regression/continuous/continuous.go b/perf/go/regression/continuous/continuous.go
index cdfedc5..69efafa 100644
--- a/perf/go/regression/continuous/continuous.go
+++ b/perf/go/regression/continuous/continuous.go
@@ -21,6 +21,7 @@
"go.skia.org/infra/perf/go/config"
"go.skia.org/infra/perf/go/dataframe"
perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/ingestevents"
"go.skia.org/infra/perf/go/notify"
"go.skia.org/infra/perf/go/regression"
@@ -44,9 +45,9 @@
// Current state of looking for regressions, i.e. the current commit and alert
// being worked on.
type Current struct {
- Commit perfgit.Commit `json:"commit"`
- Alert *alerts.Alert `json:"alert"`
- Message string `json:"message"`
+ Commit provider.Commit `json:"commit"`
+ Alert *alerts.Alert `json:"alert"`
+ Message string `json:"message"`
}
// ConfigProvider is a function that's called to return a slice of
diff --git a/perf/go/regression/fromsummary.go b/perf/go/regression/fromsummary.go
index 71586a1..942f3f2 100644
--- a/perf/go/regression/fromsummary.go
+++ b/perf/go/regression/fromsummary.go
@@ -7,12 +7,13 @@
"go.skia.org/infra/perf/go/alerts"
perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/stepfit"
)
// RegressionFromClusterResponse returns the commit for the regression along with
// the *Regression.
-func RegressionFromClusterResponse(ctx context.Context, resp *RegressionDetectionResponse, cfg *alerts.Alert, perfGit *perfgit.Git) (perfgit.Commit, *Regression, error) {
+func RegressionFromClusterResponse(ctx context.Context, resp *RegressionDetectionResponse, cfg *alerts.Alert, perfGit *perfgit.Git) (provider.Commit, *Regression, error) {
ret := &Regression{}
headerLength := len(resp.Frame.DataFrame.Header)
midPoint := headerLength / 2
diff --git a/perf/go/trybot/results/dfloader/dfloader_test.go b/perf/go/trybot/results/dfloader/dfloader_test.go
index 89ba4ec..6b8265f 100644
--- a/perf/go/trybot/results/dfloader/dfloader_test.go
+++ b/perf/go/trybot/results/dfloader/dfloader_test.go
@@ -28,7 +28,7 @@
const e = vec32.MissingDataSentinel
func setupForTest(t *testing.T) (context.Context, *perfgit.Git, []string) {
- ctx, db, _, hashes, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, hashes, _, instanceConfig := gittest.NewForTest(t)
instanceConfig.DataStoreConfig.TileSize = testTileSize
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
diff --git a/perf/go/ts/BUILD.bazel b/perf/go/ts/BUILD.bazel
index fe3d473..83a7a65 100644
--- a/perf/go/ts/BUILD.bazel
+++ b/perf/go/ts/BUILD.bazel
@@ -13,7 +13,7 @@
"//perf/go/clustering2",
"//perf/go/dryrun",
"//perf/go/frontend",
- "//perf/go/git",
+ "//perf/go/git/provider",
"//perf/go/ingest/format",
"//perf/go/pivot",
"//perf/go/progress",
diff --git a/perf/go/ts/main.go b/perf/go/ts/main.go
index 486a266..05d68ed 100644
--- a/perf/go/ts/main.go
+++ b/perf/go/ts/main.go
@@ -16,7 +16,7 @@
"go.skia.org/infra/perf/go/clustering2"
"go.skia.org/infra/perf/go/dryrun"
"go.skia.org/infra/perf/go/frontend"
- perfgit "go.skia.org/infra/perf/go/git"
+ "go.skia.org/infra/perf/go/git/provider"
"go.skia.org/infra/perf/go/ingest/format"
"go.skia.org/infra/perf/go/pivot"
"go.skia.org/infra/perf/go/progress"
@@ -77,7 +77,7 @@
frontend.TriageResponse{},
frontend.TryBugRequest{},
frontend.TryBugResponse{},
- perfgit.Commit{},
+ provider.Commit{},
regression.FullSummary{},
regression.RegressionDetectionRequest{},
regression.RegressionDetectionResponse{},
diff --git a/perf/go/ui/frame/frame_test.go b/perf/go/ui/frame/frame_test.go
index 9cd7d7a..10003a1 100644
--- a/perf/go/ui/frame/frame_test.go
+++ b/perf/go/ui/frame/frame_test.go
@@ -36,7 +36,7 @@
)
func TestGetSkps_Success(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -56,7 +56,7 @@
}
func TestGetSkps_SuccessIfFileChangeMarkerNotSet(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
@@ -76,7 +76,7 @@
}
func TestGetSkps_ErrOnBadCommitNumber(t *testing.T) {
- ctx, db, _, _, instanceConfig := gittest.NewForTest(t)
+ ctx, db, _, _, _, instanceConfig := gittest.NewForTest(t)
g, err := perfgit.New(ctx, true, db, instanceConfig)
require.NoError(t, err)
