diff options
author | James Fargher <proglottis@gmail.com> | 2023-03-20 22:48:25 +0300 |
---|---|---|
committer | James Fargher <proglottis@gmail.com> | 2023-03-20 22:48:25 +0300 |
commit | 902f1f365fd2f0c8e190accdb092d533cc4692a9 (patch) | |
tree | f579f87c2cead724ae765f6b36b2caeb218a9415 | |
parent | 5338468e55ae2bd6eba6ddc6021e160854348f2b (diff) | |
parent | aa269afa0b3f37c13fb41da668c9b5f9e2635104 (diff) |
Merge branch 'cleanup_module-updater' into 'master'
Easier major version upgrades
Closes #4894
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5505
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
-rw-r--r-- | Makefile | 2 | ||||
-rw-r--r-- | internal/praefect/datastore/glsql/doc.go | 2 | ||||
-rw-r--r-- | internal/praefect/datastore/queue_bm_test.go | 4 | ||||
-rw-r--r-- | internal/praefect/datastore/repository_store_bm_test.go | 5 | ||||
-rw-r--r-- | internal/testhelper/leakage.go | 4 | ||||
-rw-r--r-- | internal/testhelper/testcfg/binaries.go | 4 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 13 | ||||
-rw-r--r-- | tools/module-updater/main.go | 60 |
8 files changed, 50 insertions, 44 deletions
@@ -70,7 +70,7 @@ GOLANGCI_LINT_OPTIONS ?= GOLANGCI_LINT_CONFIG ?= ${SOURCE_DIR}/.golangci.yml # Build information -GITALY_PACKAGE := gitlab.com/gitlab-org/gitaly/v15 +GITALY_PACKAGE := $(shell go list -m) GITALY_VERSION := $(shell ${GIT} describe --match v* 2>/dev/null | sed 's/^v//' || cat ${SOURCE_DIR}/VERSION 2>/dev/null || echo unknown) GO_LDFLAGS := -X ${GITALY_PACKAGE}/internal/version.version=${GITALY_VERSION} SERVER_BUILD_TAGS := tracer_static,tracer_static_jaeger,tracer_static_stackdriver,continuous_profiler_stackdriver diff --git a/internal/praefect/datastore/glsql/doc.go b/internal/praefect/datastore/glsql/doc.go index 3e68b4bf2..655995fd6 100644 --- a/internal/praefect/datastore/glsql/doc.go +++ b/internal/praefect/datastore/glsql/doc.go @@ -23,7 +23,7 @@ // go test \ // -v \ // -count=1 \ -// gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore/glsql \ +// ./internal/praefect/datastore/glsql \ // -run=^TestOpenDB$ // // Once it is finished successfully you can be sure other tests would be able to diff --git a/internal/praefect/datastore/queue_bm_test.go b/internal/praefect/datastore/queue_bm_test.go index 7c8bee2fc..de7f60f29 100644 --- a/internal/praefect/datastore/queue_bm_test.go +++ b/internal/praefect/datastore/queue_bm_test.go @@ -11,22 +11,18 @@ import ( ) func BenchmarkPostgresReplicationEventQueue_Acknowledge(b *testing.B) { - // go test -tags=postgres -test.run=~ -test.bench=BenchmarkPostgresReplicationEventQueue_Acknowledge/small -benchtime=1000x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("small", func(b *testing.B) { benchmarkPostgresReplicationEventQueueAcknowledge(b, map[JobState]int{JobStateReady: 10, JobStateInProgress: 10, JobStateFailed: 10}) }) - // go test -tags=postgres -test.run=~ -test.bench=BenchmarkPostgresReplicationEventQueue_Acknowledge/medium -benchtime=100x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("medium", func(b *testing.B) { benchmarkPostgresReplicationEventQueueAcknowledge(b, map[JobState]int{JobStateReady: 1_000, JobStateInProgress: 100, JobStateFailed: 100}) }) - // go test -tags=postgres -test.run=~ -test.bench=BenchmarkPostgresReplicationEventQueue_Acknowledge/big -benchtime=10x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("big", func(b *testing.B) { benchmarkPostgresReplicationEventQueueAcknowledge(b, map[JobState]int{JobStateReady: 100_000, JobStateInProgress: 100, JobStateFailed: 100}) }) - // go test -tags=postgres -test.run=~ -test.bench=BenchmarkPostgresReplicationEventQueue_Acknowledge/huge -benchtime=1x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("huge", func(b *testing.B) { benchmarkPostgresReplicationEventQueueAcknowledge(b, map[JobState]int{JobStateReady: 1_000_000, JobStateInProgress: 100, JobStateFailed: 100}) }) diff --git a/internal/praefect/datastore/repository_store_bm_test.go b/internal/praefect/datastore/repository_store_bm_test.go index 3f982e417..cf595e06c 100644 --- a/internal/praefect/datastore/repository_store_bm_test.go +++ b/internal/praefect/datastore/repository_store_bm_test.go @@ -13,27 +13,22 @@ import ( // The test setup takes a lot of time, so it is better to run each sub-benchmark separately with limit on number of repeats. func BenchmarkPostgresRepositoryStore_GetConsistentStorages(b *testing.B) { - // go test -tags=postgres -test.bench=BenchmarkPostgresRepositoryStore_GetConsistentStorages/extra-small -benchtime=5000x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("extra-small", func(b *testing.B) { benchmarkGetConsistentStorages(b, 3, 1000) }) - // go test -tags=postgres -test.bench=BenchmarkPostgresRepositoryStore_GetConsistentStorages/small -benchtime=1000x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("small", func(b *testing.B) { benchmarkGetConsistentStorages(b, 3, 10_000) }) - // go test -tags=postgres -test.bench=BenchmarkPostgresRepositoryStore_GetConsistentStorages/medium -benchtime=50x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("medium", func(b *testing.B) { benchmarkGetConsistentStorages(b, 3, 100_000) }) - // go test -tags=postgres -test.bench=BenchmarkPostgresRepositoryStore_GetConsistentStorages/large -benchtime=10x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("large", func(b *testing.B) { benchmarkGetConsistentStorages(b, 3, 1_000_000) }) - // go test -tags=postgres -test.bench=BenchmarkPostgresRepositoryStore_GetConsistentStorages/huge -benchtime=1x gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore b.Run("huge", func(b *testing.B) { benchmarkGetConsistentStorages(b, 6, 1_000_000) }) diff --git a/internal/testhelper/leakage.go b/internal/testhelper/leakage.go index 0b78353f7..a944efb73 100644 --- a/internal/testhelper/leakage.go +++ b/internal/testhelper/leakage.go @@ -29,13 +29,13 @@ func mustHaveNoGoroutines() { // eventually, but the pragmatic approach is to just wait until we remove // the Ruby sidecar altogether. goleak.IgnoreTopFunction("google.golang.org/grpc.(*ccBalancerWrapper).watcher"), - goleak.IgnoreTopFunction("gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/rubyserver/balancer.(*builder).monitor"), + goleak.IgnoreTopFunction(PkgPath("internal/gitaly/rubyserver/balancer.(*builder).monitor")), // labkit's logger spawns a Goroutine which cannot be closed when calling // `Initialize()`. goleak.IgnoreTopFunction("gitlab.com/gitlab-org/labkit/log.listenForSignalHangup"), // The backchannel code is somehow stock on closing its connections. I have no clue // why that is, but we should investigate. - goleak.IgnoreTopFunction("gitlab.com/gitlab-org/gitaly/v15/internal/backchannel.clientHandshake.serve.func4"), + goleak.IgnoreTopFunction(PkgPath("internal/backchannel.clientHandshake.serve.func4")), ); err != nil { panic(fmt.Errorf("goroutines running: %w", err)) } diff --git a/internal/testhelper/testcfg/binaries.go b/internal/testhelper/testcfg/binaries.go index 001860586..c9f4a360b 100644 --- a/internal/testhelper/testcfg/binaries.go +++ b/internal/testhelper/testcfg/binaries.go @@ -113,7 +113,7 @@ func BuildBinary(tb testing.TB, targetDir, sourcePath string) string { "build", "-buildvcs=false", "-tags", strings.Join(buildTags, ","), - "-ldflags", fmt.Sprintf("-X gitlab.com/gitlab-org/gitaly/v15/internal/version.version=%s", version.GetVersion()), + "-ldflags", fmt.Sprintf("-X %s/version.version=%s", testhelper.PkgPath("internal"), version.GetVersion()), "-o", sharedBinaryPath, sourcePath, ) @@ -139,5 +139,5 @@ func BuildBinary(tb testing.TB, targetDir, sourcePath string) string { } func gitalyCommandPath(command string) string { - return fmt.Sprintf("gitlab.com/gitlab-org/gitaly/v15/cmd/%s", command) + return fmt.Sprintf("%s/cmd/%s", testhelper.PkgPath(), command) } diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 8e4609dfb..bd0d279b0 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -17,7 +17,9 @@ import ( "net/http" "os" "os/exec" + "path" "path/filepath" + "reflect" "syscall" "testing" "time" @@ -399,3 +401,14 @@ func SkipQuarantinedTest(t *testing.T, issue string, tests ...string) { t.Skipf("This test has been quarantined. Please see %s for more information.", issue) } } + +// pkgPath is used to determine the package path using reflection. +type pkgPath struct{} + +// PkgPath returns the gitaly module package path, including major version +// number. paths will be path joined to the returned package path. +func PkgPath(paths ...string) string { + internalPkgPath := path.Dir(reflect.TypeOf(pkgPath{}).PkgPath()) + rootPkgPath := path.Dir(internalPkgPath) + return path.Join(append([]string{rootPkgPath}, paths...)...) +} diff --git a/tools/module-updater/main.go b/tools/module-updater/main.go index 608980d4c..f2a51afc4 100644 --- a/tools/module-updater/main.go +++ b/tools/module-updater/main.go @@ -161,48 +161,50 @@ func rewriteImports(moduleAbsRootPath, prev, next string) error { } seen[file.Name()] = struct{}{} - astFile, err := parser.ParseFile(fileSet, file.Name(), nil, parser.ParseComments) - if err != nil { + if err := rewriteFileImports(fileSet, file.Name(), prev, next); err != nil { rerr = err return false } - for _, imprt := range astFile.Imports { - oldImport, err := strconv.Unquote(imprt.Path.Value) - if err != nil { - rerr = fmt.Errorf("unquote import: %s :%w", imprt.Path.Value, err) - return false - } + return true + }) - if newImport := strings.Replace(oldImport, prev, next, 1); newImport != oldImport { - imprt.EndPos = imprt.End() - imprt.Path.Value = strconv.Quote(newImport) - } - } + return rerr +} + +func rewriteFileImports(fileSet *token.FileSet, filename, prev, next string) (retErr error) { + astFile, err := parser.ParseFile(fileSet, filename, nil, parser.ParseComments) + if err != nil { + return err + } - f, err := os.Create(file.Name()) + for _, imprt := range astFile.Imports { + oldImport, err := strconv.Unquote(imprt.Path.Value) if err != nil { - rerr = fmt.Errorf("open file %q: %w", file.Name(), err) - return false + return fmt.Errorf("unquote import: %s :%w", imprt.Path.Value, err) } - ferr := format.Node(f, fileSet, astFile) - cerr := f.Close() - - if ferr != nil { - rerr = fmt.Errorf("rewrite file %q: %w", file.Name(), err) - return false + if newImport := strings.Replace(oldImport, prev, next, 1); newImport != oldImport { + imprt.EndPos = imprt.End() + imprt.Path.Value = strconv.Quote(newImport) } + } - if cerr != nil { - rerr = fmt.Errorf("close file %q: %w", file.Name(), err) - return false + f, err := os.Create(filename) + if err != nil { + return fmt.Errorf("open file %q: %w", filename, err) + } + defer func() { + if err := f.Close(); retErr == nil && err != nil { + retErr = fmt.Errorf("close file %q: %w", filename, err) } + }() - return true - }) + if err := format.Node(f, fileSet, astFile); err != nil { + return fmt.Errorf("rewrite file %q: %w", filename, err) + } - return rerr + return nil } func normaliseDesiredImportReplacement(module, from, to string) (string, string, error) { @@ -289,7 +291,7 @@ func rewriteProto(moduleAbsRootPath, prev, next string) error { for _, line := range lines { tokens := bytes.Fields(line) pckg := bytes.Join(tokens, []byte{'~'}) - if !bytes.HasPrefix(pckg, []byte(`option~go_package~=~"`+prev+`/proto/go/gitalypb";`)) { + if !bytes.HasPrefix(pckg, []byte(`option~go_package~=~"`+prev+`/proto/`)) { modified = append(modified, line) continue } |