Merge branch 'pks-git-housekeeping-sha256' into 'master'

housekeeping: Enable testing with SHA256 object format

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5191

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>

10 files changed, 67 insertions, 48 deletions

diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go
index acf310535..5499d3a38 100644
--- a/internal/git/gittest/repo.go
+++ b/internal/git/gittest/repo.go
@@ -185,14 +185,12 @@ func CreateRepository(tb testing.TB, ctx context.Context, cfg config.Cfg, config
 			Exec(tb, cfg, "clone", "--no-hardlinks", "--dissociate", "--bare", testRepositoryPath(tb, opts.Seed), repoPath)
 			Exec(tb, cfg, "-C", repoPath, "remote", "remove", "origin")
 		} else {
-			args := []string{"init", "--bare"}
-			args = append(args, initRepoExtraArgs...)
-			args = append(args, repoPath)
-			if opts.ObjectFormat != "" {
-				args = append(args, "--object-format", opts.ObjectFormat)
+			objectFormat := opts.ObjectFormat
+			if objectFormat == "" {
+				objectFormat = DefaultObjectHash.Format
 			}
 
-			Exec(tb, cfg, args...)
+			Exec(tb, cfg, "init", "--bare", "--object-format="+objectFormat, repoPath)
 		}
 
 		tb.Cleanup(func() { require.NoError(tb, os.RemoveAll(repoPath)) })
diff --git a/internal/git/gittest/sha1.go b/internal/git/gittest/sha1.go
index 4a5d8de33..b3407bd83 100644
--- a/internal/git/gittest/sha1.go
+++ b/internal/git/gittest/sha1.go
@@ -4,9 +4,5 @@ package gittest
 
 import "gitlab.com/gitlab-org/gitaly/v15/internal/git"
 
-var (
-	// DefaultObjectHash is the default hash used for running tests.
-	DefaultObjectHash = git.ObjectHashSHA1
-
-	initRepoExtraArgs = []string{}
-)
+// DefaultObjectHash is the default hash used for running tests.
+var DefaultObjectHash = git.ObjectHashSHA1
diff --git a/internal/git/gittest/sha256.go b/internal/git/gittest/sha256.go
index d571657a9..1ca46e89e 100644
--- a/internal/git/gittest/sha256.go
+++ b/internal/git/gittest/sha256.go
@@ -6,9 +6,5 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 )
 
-var (
-	// DefaultObjectHash is the default hash used for running tests.
-	DefaultObjectHash = git.ObjectHashSHA256
-
-	initRepoExtraArgs = []string{"--object-format=sha256"}
-)
+// DefaultObjectHash is the default hash used for running tests.
+var DefaultObjectHash = git.ObjectHashSHA256
diff --git a/internal/git/housekeeping/clean_stale_data_test.go b/internal/git/housekeeping/clean_stale_data_test.go
index abe931396..2a8283bc5 100644
--- a/internal/git/housekeeping/clean_stale_data_test.go
+++ b/internal/git/housekeeping/clean_stale_data_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package housekeeping
 
 import (
@@ -278,9 +276,13 @@ func TestRepositoryManager_CleanStaleData(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
-			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 			ctx := testhelper.Context(t)
+			cfg := testcfg.Build(t)
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+			})
+			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 			// We need to fix permissions so we don't fail to
 			// remove the temporary directory after the test.
@@ -380,7 +382,12 @@ func TestRepositoryManager_CleanStaleData_references(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			t.Parallel()
 
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+			ctx := testhelper.Context(t)
+			cfg := testcfg.Build(t)
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+			})
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 			for _, ref := range tc.refs {
@@ -391,7 +398,6 @@ func TestRepositoryManager_CleanStaleData_references(t *testing.T) {
 				filetime := time.Now().Add(-ref.age)
 				require.NoError(t, os.Chtimes(path, filetime, filetime))
 			}
-			ctx := testhelper.Context(t)
 
 			mgr := NewManager(cfg.Prometheus, nil)
 
@@ -508,9 +514,13 @@ func TestRepositoryManager_CleanStaleData_emptyRefDirs(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
-			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 			ctx := testhelper.Context(t)
+			cfg := testcfg.Build(t)
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+			})
+			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 			for _, e := range tc.entries {
 				e.create(t, repoPath)
@@ -635,8 +645,11 @@ func TestRepositoryManager_CleanStaleData_withSpecificFile(t *testing.T) {
 			t.Parallel()
 
 			ctx := testhelper.Context(t)
+			cfg := testcfg.Build(t)
 
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+			})
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 			mgr := NewManager(cfg.Prometheus, nil)
 
@@ -697,9 +710,13 @@ func TestRepositoryManager_CleanStaleData_withSpecificFile(t *testing.T) {
 
 func TestRepositoryManager_CleanStaleData_serverInfo(t *testing.T) {
 	t.Parallel()
+
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+	})
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 	entries := []entry{
@@ -811,7 +828,12 @@ func TestRepositoryManager_CleanStaleData_referenceLocks(t *testing.T) {
 
 		t.Run(tc.desc, func(t *testing.T) {
 			t.Parallel()
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+
+			cfg := testcfg.Build(t)
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+			})
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 			for _, e := range tc.entries {
@@ -930,9 +952,14 @@ func TestIsStaleTemporaryObject(t *testing.T) {
 
 func TestRepositoryManager_CleanStaleData_missingRepo(t *testing.T) {
 	t.Parallel()
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
-	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+	})
+	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 	require.NoError(t, os.RemoveAll(repoPath))
 
@@ -1023,6 +1050,10 @@ func TestRepositoryManager_CleanStaleData_unsetConfigurationTransactional(t *tes
 
 	expectedConfig := "core.repositoryformatversion\ncore.filemode\ncore.bare\n"
 
+	if gittest.DefaultObjectHash.Format == "sha256" {
+		expectedConfig = expectedConfig + "extensions.objectformat\n"
+	}
+
 	if runtime.GOOS == "darwin" {
 		expectedConfig = expectedConfig + "core.ignorecase\ncore.precomposeunicode\n"
 	}
diff --git a/internal/git/housekeeping/objects_test.go b/internal/git/housekeeping/objects_test.go
index 825b9d6e3..f30148a0c 100644
--- a/internal/git/housekeeping/objects_test.go
+++ b/internal/git/housekeeping/objects_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package housekeeping
 
 import (
diff --git a/internal/git/housekeeping/optimization_strategy_test.go b/internal/git/housekeeping/optimization_strategy_test.go
index 34bfa0d14..33ae06f2d 100644
--- a/internal/git/housekeeping/optimization_strategy_test.go
+++ b/internal/git/housekeeping/optimization_strategy_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package housekeeping
 
 import (
@@ -173,7 +171,7 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 				info: stats.RepositoryInfo{
 					LooseObjects: stats.LooseObjectsInfo{
 						Count: 2,
-						Size:  hashDependentObjectSize(142, 156),
+						Size:  hashDependentObjectSize(142, 158),
 					},
 					References: stats.ReferencesInfo{
 						LooseReferencesCount: 1,
@@ -204,7 +202,7 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 				info: stats.RepositoryInfo{
 					LooseObjects: stats.LooseObjectsInfo{
 						Count: 2,
-						Size:  hashDependentObjectSize(142, 156),
+						Size:  hashDependentObjectSize(142, 158),
 					},
 					References: stats.ReferencesInfo{
 						LooseReferencesCount: 1,
@@ -235,7 +233,7 @@ func TestNewHeuristicalOptimizationStrategy_variousParameters(t *testing.T) {
 				info: stats.RepositoryInfo{
 					LooseObjects: stats.LooseObjectsInfo{
 						Count: 2,
-						Size:  hashDependentObjectSize(142, 156),
+						Size:  hashDependentObjectSize(142, 158),
 					},
 					References: stats.ReferencesInfo{
 						LooseReferencesCount: 1,
diff --git a/internal/git/housekeeping/optimize_repository_ext_test.go b/internal/git/housekeeping/optimize_repository_ext_test.go
index 37c859609..1203fa3e2 100644
--- a/internal/git/housekeeping/optimize_repository_ext_test.go
+++ b/internal/git/housekeeping/optimize_repository_ext_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package housekeeping_test
 
 import (
diff --git a/internal/git/housekeeping/optimize_repository_test.go b/internal/git/housekeeping/optimize_repository_test.go
index 065dc063e..565c2e267 100644
--- a/internal/git/housekeeping/optimize_repository_test.go
+++ b/internal/git/housekeeping/optimize_repository_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package housekeeping
 
 import (
diff --git a/internal/git/housekeeping/worktrees_test.go b/internal/git/housekeeping/worktrees_test.go
index 95c2cf1be..3c928bec0 100644
--- a/internal/git/housekeeping/worktrees_test.go
+++ b/internal/git/housekeeping/worktrees_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package housekeeping
 
 import (
@@ -20,7 +18,12 @@ func TestCleanupDisconnectedWorktrees_doesNothingWithoutWorktrees(t *testing.T)
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+	cfg := testcfg.Build(t)
+
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+	})
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"))
 	worktreePath := filepath.Join(testhelper.TempDir(t), "worktree")
 
 	failingGitCmdFactory := gittest.NewInterceptingCommandFactory(t, ctx, cfg, func(git.ExecutionEnvironment) string {
@@ -46,8 +49,13 @@ func TestCleanupDisconnectedWorktrees_doesNothingWithoutWorktrees(t *testing.T)
 func TestRemoveWorktree(t *testing.T) {
 	t.Parallel()
 
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+	})
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"))
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 	existingWorktreePath := filepath.Join(repoPath, worktreePrefix, "existing")
diff --git a/internal/git/localrepo/remote_extra_test.go b/internal/git/localrepo/remote_extra_test.go
index cd0fd2357..805ae2575 100644
--- a/internal/git/localrepo/remote_extra_test.go
+++ b/internal/git/localrepo/remote_extra_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package localrepo_test
 
 import (