diff options
author | Will Chandler <wchandler@gitlab.com> | 2022-08-02 20:33:51 +0300 |
---|---|---|
committer | Will Chandler <wchandler@gitlab.com> | 2022-08-02 20:33:51 +0300 |
commit | 072f01c9a020b2b49c222a338349555b2e807384 (patch) | |
tree | c7e57be9e169839a0f8f32ef26120ea3d6b71f67 | |
parent | 0a03f045e065b9e7a157322fbe486e1f02fd8617 (diff) | |
parent | 4937373fe8cbd23140fdfcc48a351656066727b5 (diff) |
Merge branch 'pks-git-localrepo-sha256' into 'master'
localrepo: Support testing with SHA256
Closes #4354
See merge request gitlab-org/gitaly!4769
24 files changed, 500 insertions, 494 deletions
diff --git a/internal/git/catfile/testhelper_test.go b/internal/git/catfile/testhelper_test.go index 4aeb140bb..e222a52c0 100644 --- a/internal/git/catfile/testhelper_test.go +++ b/internal/git/catfile/testhelper_test.go @@ -53,6 +53,10 @@ func (e *repoExecutor) GitVersion(ctx context.Context) (git.Version, error) { return git.Version{}, nil } +func (e *repoExecutor) ObjectHash(ctx context.Context) (git.ObjectHash, error) { + return gittest.DefaultObjectHash, nil +} + func setupObjectReader(t *testing.T, ctx context.Context) (config.Cfg, ObjectReader, *gitalypb.Repository) { t.Helper() diff --git a/internal/git/gittest/commit.go b/internal/git/gittest/commit.go index 3ffc1035a..fe0b0a914 100644 --- a/internal/git/gittest/commit.go +++ b/internal/git/gittest/commit.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" + "google.golang.org/protobuf/types/known/timestamppb" ) var ( @@ -29,6 +30,14 @@ var ( DefaultCommitterSignature = fmt.Sprintf( "%s <%s> %d %s", DefaultCommitterName, DefaultCommitterMail, DefaultCommitTime.Unix(), DefaultCommitTime.Format("-0700"), ) + // DefaultCommitAuthor is the Protobuf message representation of the default committer and + // author used to create commits. + DefaultCommitAuthor = &gitalypb.CommitAuthor{ + Name: []byte(DefaultCommitterName), + Email: []byte(DefaultCommitterMail), + Date: timestamppb.New(DefaultCommitTime), + Timezone: []byte("+0100"), + } ) type writeCommitConfig struct { diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go index 201f459b1..fb9529b1e 100644 --- a/internal/git/gittest/repo.go +++ b/internal/git/gittest/repo.go @@ -155,6 +155,10 @@ func CreateRepository(ctx context.Context, t testing.TB, cfg config.Cfg, configs }) require.NoError(t, err) } else { + if ObjectHashIsSHA256() { + require.FailNow(t, "CreateRepository does not yet support creating SHA256 repositories") + } + _, err := client.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{ Repository: repository, }) diff --git a/internal/git/gittest/repository_suite.go b/internal/git/gittest/repository_suite.go index 676e17564..505279598 100644 --- a/internal/git/gittest/repository_suite.go +++ b/internal/git/gittest/repository_suite.go @@ -13,7 +13,7 @@ import ( // GetRepositoryFunc is used to get a clean test repository for the different implementations of the // Repository interface in the common test suite TestRepository. -type GetRepositoryFunc func(ctx context.Context, t testing.TB, seeded bool) (git.Repository, string) +type GetRepositoryFunc func(ctx context.Context, t testing.TB) (git.Repository, string) // TestRepository tests an implementation of Repository. func TestRepository(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) { @@ -43,6 +43,12 @@ func TestRepository(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFun func testRepositoryResolveRevision(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) { ctx := testhelper.Context(t) + repo, repoPath := getRepository(ctx, t) + + firstParentCommitID := WriteCommit(t, cfg, repoPath, WithMessage("first parent")) + secondParentCommitID := WriteCommit(t, cfg, repoPath, WithMessage("second parent")) + masterCommitID := WriteCommit(t, cfg, repoPath, WithBranch("master"), WithParents(firstParentCommitID, secondParentCommitID)) + for _, tc := range []struct { desc string revision string @@ -51,22 +57,22 @@ func testRepositoryResolveRevision(t *testing.T, cfg config.Cfg, getRepository G { desc: "unqualified master branch", revision: "master", - expected: "1e292f8fedd741b75372e19097c76d327140c312", + expected: masterCommitID, }, { desc: "fully qualified master branch", revision: "refs/heads/master", - expected: "1e292f8fedd741b75372e19097c76d327140c312", + expected: masterCommitID, }, { desc: "typed commit", revision: "refs/heads/master^{commit}", - expected: "1e292f8fedd741b75372e19097c76d327140c312", + expected: masterCommitID, }, { desc: "extended SHA notation", revision: "refs/heads/master^2", - expected: "c1c67abbaf91f624347bb3ae96eabe3a1b742478", + expected: secondParentCommitID, }, { desc: "nonexistent branch", @@ -78,7 +84,6 @@ func testRepositoryResolveRevision(t *testing.T, cfg config.Cfg, getRepository G }, } { t.Run(tc.desc, func(t *testing.T) { - repo, _ := getRepository(ctx, t, true) oid, err := repo.ResolveRevision(ctx, git.Revision(tc.revision)) if tc.expected == "" { require.Equal(t, err, git.ErrReferenceNotFound) @@ -94,7 +99,7 @@ func testRepositoryResolveRevision(t *testing.T, cfg config.Cfg, getRepository G func testRepositoryHasBranches(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) { ctx := testhelper.Context(t) - repo, repoPath := getRepository(ctx, t, false) + repo, repoPath := getRepository(ctx, t) emptyCommit := text.ChompBytes(Exec(t, cfg, "-C", repoPath, "commit-tree", DefaultObjectHash.EmptyTreeOID.String())) @@ -112,7 +117,6 @@ func testRepositoryHasBranches(t *testing.T, cfg config.Cfg, getRepository GetRe } func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) { - const testOID = "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863" ctx := testhelper.Context(t) for _, tc := range []struct { @@ -123,7 +127,7 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository { desc: "default ref", repo: func(t *testing.T) git.Repository { - repo, repoPath := getRepository(ctx, t, false) + repo, repoPath := getRepository(ctx, t) oid := WriteCommit(t, cfg, repoPath, WithBranch("apple")) WriteCommit(t, cfg, repoPath, WithParents(oid), WithBranch("main")) return repo @@ -133,7 +137,7 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository { desc: "legacy default ref", repo: func(t *testing.T) git.Repository { - repo, repoPath := getRepository(ctx, t, false) + repo, repoPath := getRepository(ctx, t) oid := WriteCommit(t, cfg, repoPath, WithBranch("apple")) WriteCommit(t, cfg, repoPath, WithParents(oid), WithBranch("master")) return repo @@ -143,14 +147,14 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository { desc: "no branches", repo: func(t *testing.T) git.Repository { - repo, _ := getRepository(ctx, t, false) + repo, _ := getRepository(ctx, t) return repo }, }, { desc: "one branch", repo: func(t *testing.T) git.Repository { - repo, repoPath := getRepository(ctx, t, false) + repo, repoPath := getRepository(ctx, t) WriteCommit(t, cfg, repoPath, WithBranch("apple")) return repo }, @@ -159,7 +163,7 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository { desc: "no default branches", repo: func(t *testing.T) git.Repository { - repo, repoPath := getRepository(ctx, t, false) + repo, repoPath := getRepository(ctx, t) oid := WriteCommit(t, cfg, repoPath, WithBranch("apple")) WriteCommit(t, cfg, repoPath, WithParents(oid), WithBranch("banana")) return repo @@ -167,19 +171,13 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository expectedName: git.NewReferenceNameFromBranchName("apple"), }, { - desc: "test repo default", - repo: func(t *testing.T) git.Repository { - repo, _ := getRepository(ctx, t, true) - return repo - }, - expectedName: git.LegacyDefaultRef, - }, - { desc: "test repo HEAD set", repo: func(t *testing.T) git.Repository { - repo, repoPath := getRepository(ctx, t, true) - Exec(t, cfg, "-C", repoPath, "update-ref", "refs/heads/feature", testOID) + repo, repoPath := getRepository(ctx, t) + + WriteCommit(t, cfg, repoPath, WithBranch("feature")) Exec(t, cfg, "-C", repoPath, "symbolic-ref", "HEAD", "refs/heads/feature") + return repo }, expectedName: git.NewReferenceNameFromBranchName("feature"), diff --git a/internal/git/localrepo/config_test.go b/internal/git/localrepo/config_test.go index a29d8f4eb..3f17c22c5 100644 --- a/internal/git/localrepo/config_test.go +++ b/internal/git/localrepo/config_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo import ( @@ -97,10 +95,15 @@ func TestRepo_SetConfig(t *testing.T) { require.Equal(t, tc.expectedErr, repo.SetConfig(ctx, tc.key, tc.value, &transaction.MockManager{})) standardEntries := []string{ - "core.repositoryformatversion=0", "core.filemode=true", "core.bare=true", } + if gittest.ObjectHashIsSHA256() { + standardEntries = append(standardEntries, "core.repositoryformatversion=1") + standardEntries = append(standardEntries, "extensions.objectformat=sha256") + } else { + standardEntries = append(standardEntries, "core.repositoryformatversion=0") + } if runtime.GOOS == "darwin" { standardEntries = append(standardEntries, @@ -110,7 +113,7 @@ func TestRepo_SetConfig(t *testing.T) { } output := gittest.Exec(t, cfg, "-C", repoPath, "config", "--list", "--local") - require.Equal(t, + require.ElementsMatch(t, append(standardEntries, tc.expectedEntries...), strings.Split(text.ChompBytes(output), "\n"), ) @@ -147,15 +150,11 @@ func TestRepo_UnsetMatchingConfig(t *testing.T) { "core.filemode", "core.bare", } - if runtime.GOOS == "darwin" { - standardKeys = []string{ - "core.repositoryformatversion", - "core.filemode", - "core.bare", - "core.ignorecase", - "core.precomposeunicode", - } + standardKeys = append(standardKeys, "core.ignorecase", "core.precomposeunicode") + } + if gittest.ObjectHashIsSHA256() { + standardKeys = append(standardKeys, "extensions.objectformat") } for _, tc := range []struct { @@ -179,7 +178,7 @@ func TestRepo_UnsetMatchingConfig(t *testing.T) { "foo.qux": "value2", }, regex: "foo.bar", - expectedKeys: append(standardKeys, "foo.qux"), + expectedKeys: append([]string{"foo.qux"}, standardKeys...), }, { desc: "multiple matches", @@ -197,7 +196,7 @@ func TestRepo_UnsetMatchingConfig(t *testing.T) { "foo.qux": "value2", }, regex: "matchme", - expectedKeys: append(standardKeys, "foo.qux"), + expectedKeys: append([]string{"foo.qux"}, standardKeys...), }, { desc: "anchored", @@ -206,7 +205,7 @@ func TestRepo_UnsetMatchingConfig(t *testing.T) { "matchme.foo": "value2", }, regex: "^matchme", - expectedKeys: append(standardKeys, "foo.matchme"), + expectedKeys: append([]string{"foo.matchme"}, standardKeys...), }, { desc: "no matches", @@ -246,7 +245,7 @@ func TestRepo_UnsetMatchingConfig(t *testing.T) { require.Equal(t, tc.expectedErr, repo.UnsetMatchingConfig(ctx, tc.regex, &transaction.MockManager{})) output := gittest.Exec(t, cfg, "-C", repoPath, "config", "--list", "--name-only", "--local") - require.Equal(t, tc.expectedKeys, strings.Split(text.ChompBytes(output), "\n")) + require.ElementsMatch(t, tc.expectedKeys, strings.Split(text.ChompBytes(output), "\n")) }) } diff --git a/internal/git/localrepo/objects.go b/internal/git/localrepo/objects.go index 4616df7a5..43198b0e9 100644 --- a/internal/git/localrepo/objects.go +++ b/internal/git/localrepo/objects.go @@ -47,7 +47,12 @@ func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader) return "", errorWithStderr(err, stderr.Bytes()) } - oid, err := git.ObjectHashSHA1.FromHex(text.ChompBytes(stdout.Bytes())) + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return "", fmt.Errorf("detecting object hash: %w", err) + } + + oid, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes())) if err != nil { return "", err } @@ -159,7 +164,12 @@ func (repo *Repo) WriteTag( return "", MktagError{tagName: tagName, stderr: stderr.String()} } - tagID, err := git.ObjectHashSHA1.FromHex(text.ChompBytes(stdout.Bytes())) + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return "", fmt.Errorf("detecting object hash: %w", err) + } + + tagID, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes())) if err != nil { return "", fmt.Errorf("could not parse tag ID: %w", err) } diff --git a/internal/git/localrepo/objects_test.go b/internal/git/localrepo/objects_test.go index c5c5b1245..a4cd9b95c 100644 --- a/internal/git/localrepo/objects_test.go +++ b/internal/git/localrepo/objects_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo import ( @@ -18,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" - "google.golang.org/protobuf/types/known/timestamppb" ) type ReaderFunc func([]byte) (int, error) @@ -28,7 +25,7 @@ func (fn ReaderFunc) Read(b []byte) (int, error) { return fn(b) } func TestRepo_WriteBlob(t *testing.T) { ctx := testhelper.Context(t) - _, repo, repoPath := setupRepo(t, withEmptyRepo()) + _, repo, repoPath := setupRepo(t) for _, tc := range []struct { desc string @@ -103,7 +100,7 @@ func TestFormatTag(t *testing.T) { // internal/gitaly/service/operations/tags_test.go { desc: "basic signature", - objectID: git.ObjectHashSHA1.ZeroOID, + objectID: gittest.DefaultObjectHash.ZeroOID, objectType: "commit", tagName: []byte("my-tag"), author: &gitalypb.User{ @@ -114,7 +111,7 @@ func TestFormatTag(t *testing.T) { }, { desc: "basic signature", - objectID: git.ObjectHashSHA1.ZeroOID, + objectID: gittest.DefaultObjectHash.ZeroOID, objectType: "commit", tagName: []byte("my-tag\ninjection"), tagBody: []byte(""), @@ -126,7 +123,7 @@ func TestFormatTag(t *testing.T) { }, { desc: "signature with fixed time", - objectID: git.ObjectHashSHA1.ZeroOID, + objectID: gittest.DefaultObjectHash.ZeroOID, objectType: "commit", tagName: []byte("my-tag"), tagBody: []byte(""), @@ -157,6 +154,8 @@ func TestRepo_WriteTag(t *testing.T) { cfg, repo, repoPath := setupRepo(t) + commitID := gittest.WriteCommit(t, cfg, repoPath) + for _, tc := range []struct { desc string objectID git.ObjectID @@ -171,7 +170,7 @@ func TestRepo_WriteTag(t *testing.T) { // internal/gitaly/service/operations/tags_test.go { desc: "basic signature", - objectID: "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd", + objectID: commitID, objectType: "commit", tagName: []byte("my-tag"), tagBody: []byte(""), @@ -182,7 +181,7 @@ func TestRepo_WriteTag(t *testing.T) { }, { desc: "signature with time", - objectID: "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd", + objectID: commitID, objectType: "commit", tagName: []byte("tag-with-timestamp"), tagBody: []byte(""), @@ -191,15 +190,15 @@ func TestRepo_WriteTag(t *testing.T) { Email: []byte("root@localhost"), }, authorDate: time.Unix(12345, 0).UTC(), - expectedTag: `object c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd + expectedTag: fmt.Sprintf(`object %s type commit tag tag-with-timestamp tagger root <root@localhost> 12345 +0000 -`, +`, commitID), }, { desc: "signature with time and timezone", - objectID: "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd", + objectID: commitID, objectType: "commit", tagName: []byte("tag-with-timezone"), tagBody: []byte(""), @@ -208,11 +207,11 @@ tagger root <root@localhost> 12345 +0000 Email: []byte("root@localhost"), }, authorDate: time.Unix(12345, 0).In(time.FixedZone("myzone", -60*60)), - expectedTag: `object c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd + expectedTag: fmt.Sprintf(`object %s type commit tag tag-with-timezone tagger root <root@localhost> 12345 -0100 -`, +`, commitID), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -233,7 +232,8 @@ tagger root <root@localhost> 12345 -0100 func TestRepo_ReadObject(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + blobID := gittest.WriteBlob(t, cfg, repoPath, []byte("content")) for _, tc := range []struct { desc string @@ -243,14 +243,13 @@ func TestRepo_ReadObject(t *testing.T) { }{ { desc: "invalid object", - oid: git.ObjectHashSHA1.ZeroOID, - error: InvalidObjectError(git.ObjectHashSHA1.ZeroOID.String()), + oid: gittest.DefaultObjectHash.ZeroOID, + error: InvalidObjectError(gittest.DefaultObjectHash.ZeroOID.String()), }, { - desc: "valid object", - // README in gitlab-test - oid: "3742e48c1108ced3bf45ac633b34b65ac3f2af04", - content: "Sample repo for testing gitlab features\n", + desc: "valid object", + oid: blobID, + content: "content", }, } { t.Run(tc.desc, func(t *testing.T) { @@ -262,9 +261,48 @@ func TestRepo_ReadObject(t *testing.T) { } func TestRepo_ReadCommit(t *testing.T) { + if gittest.ObjectHashIsSHA256() { + t.Skip("this test is hash-agnostic, but depends on the `git/catfile` package that has not yet been converted") + } + ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + + firstParentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("first parent")) + secondParentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("second parent")) + + treeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{ + {Path: "file", Mode: "100644", Content: "content"}, + }) + commitWithoutTrailers := gittest.WriteCommit(t, cfg, repoPath, + gittest.WithParents(firstParentID, secondParentID), + gittest.WithTree(treeID), + gittest.WithMessage("subject\n\nbody\n"), + gittest.WithBranch("main"), + ) + commitWithTrailers := gittest.WriteCommit(t, cfg, repoPath, + gittest.WithParents(commitWithoutTrailers), + gittest.WithTree(treeID), + gittest.WithMessage("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"), + ) + + // We can't use git-commit-tree(1) directly, but have to manually write signed commits. + signedCommit := text.ChompBytes(gittest.ExecOpts(t, cfg, gittest.ExecConfig{ + Stdin: strings.NewReader(fmt.Sprintf( + `tree %s +parent %s +author %[3]s +committer %[3]s +gpgsig -----BEGIN PGP SIGNATURE----- +some faked pgp-signature + -----END PGP SIGNATURE----- + +signed commit subject + +signed commit body +`, treeID, firstParentID, gittest.DefaultCommitterSignature)), + }, "-C", repoPath, "hash-object", "-t", "commit", "-w", "--stdin")) for _, tc := range []struct { desc string @@ -275,118 +313,92 @@ func TestRepo_ReadCommit(t *testing.T) { }{ { desc: "invalid commit", - revision: git.ObjectHashSHA1.ZeroOID.Revision(), + revision: gittest.DefaultObjectHash.ZeroOID.Revision(), expectedErr: ErrObjectNotFound, }, { desc: "invalid commit with trailers", - revision: git.ObjectHashSHA1.ZeroOID.Revision(), + revision: gittest.DefaultObjectHash.ZeroOID.Revision(), expectedErr: ErrObjectNotFound, opts: []ReadCommitOpt{WithTrailers()}, }, { desc: "valid commit", - revision: "refs/heads/master", + revision: "refs/heads/main", expectedCommit: &gitalypb.GitCommit{ - Id: "1e292f8fedd741b75372e19097c76d327140c312", - TreeId: "07f8147e8e73aab6c935c296e8cdc5194dee729b", + Id: commitWithoutTrailers.String(), + TreeId: treeID.String(), ParentIds: []string{ - "7975be0116940bf2ad4321f79d02a55c5f7779aa", - "c1c67abbaf91f624347bb3ae96eabe3a1b742478", - }, - Subject: []byte("Merge branch 'cherry-pick-ce369011' into 'master'"), - Body: []byte("Merge branch 'cherry-pick-ce369011' into 'master'\n\nAdd file with a _flattable_ path\n\nSee merge request gitlab-org/gitlab-test!35"), - BodySize: 128, - Author: &gitalypb.CommitAuthor{ - Name: []byte("Drew Blessing"), - Email: []byte("drew@blessing.io"), - Date: ×tamppb.Timestamp{ - Seconds: 1540830087, - }, - Timezone: []byte("+0000"), - }, - Committer: &gitalypb.CommitAuthor{ - Name: []byte("Drew Blessing"), - Email: []byte("drew@blessing.io"), - Date: ×tamppb.Timestamp{ - Seconds: 1540830087, - }, - Timezone: []byte("+0000"), + firstParentID.String(), + secondParentID.String(), }, + Subject: []byte("subject"), + Body: []byte("subject\n\nbody\n"), + BodySize: 14, + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, }, }, { desc: "trailers do not get parsed without WithTrailers()", - revision: "5937ac0a7beb003549fc5fd26fc247adbce4a52e", + revision: commitWithTrailers.Revision(), expectedCommit: &gitalypb.GitCommit{ - Id: "5937ac0a7beb003549fc5fd26fc247adbce4a52e", - TreeId: "a6973545d42361b28bfba5ced3b75dba5848b955", + Id: commitWithTrailers.String(), + TreeId: treeID.String(), ParentIds: []string{ - "570e7b2abdd848b95f2f578043fc23bd6f6fd24d", - }, - Subject: []byte("Add submodule from gitlab.com"), - Body: []byte("Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"), - BodySize: 98, - Author: &gitalypb.CommitAuthor{ - Name: []byte("Dmitriy Zaporozhets"), - Email: []byte("dmitriy.zaporozhets@gmail.com"), - Date: ×tamppb.Timestamp{ - Seconds: 1393491698, - }, - Timezone: []byte("+0200"), + commitWithoutTrailers.String(), }, - Committer: &gitalypb.CommitAuthor{ - Name: []byte("Dmitriy Zaporozhets"), - Email: []byte("dmitriy.zaporozhets@gmail.com"), - Date: ×tamppb.Timestamp{ - Seconds: 1393491698, - }, - Timezone: []byte("+0200"), - }, - SignatureType: gitalypb.SignatureType_PGP, + Subject: []byte("with trailers"), + Body: []byte("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"), + BodySize: 71, + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, }, }, { desc: "trailers get parsed with WithTrailers()", - revision: "5937ac0a7beb003549fc5fd26fc247adbce4a52e", + revision: commitWithTrailers.Revision(), opts: []ReadCommitOpt{WithTrailers()}, expectedCommit: &gitalypb.GitCommit{ - Id: "5937ac0a7beb003549fc5fd26fc247adbce4a52e", - TreeId: "a6973545d42361b28bfba5ced3b75dba5848b955", + Id: commitWithTrailers.String(), + TreeId: treeID.String(), ParentIds: []string{ - "570e7b2abdd848b95f2f578043fc23bd6f6fd24d", - }, - Subject: []byte("Add submodule from gitlab.com"), - Body: []byte("Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"), - BodySize: 98, - Author: &gitalypb.CommitAuthor{ - Name: []byte("Dmitriy Zaporozhets"), - Email: []byte("dmitriy.zaporozhets@gmail.com"), - Date: ×tamppb.Timestamp{ - Seconds: 1393491698, - }, - Timezone: []byte("+0200"), + commitWithoutTrailers.String(), }, - Committer: &gitalypb.CommitAuthor{ - Name: []byte("Dmitriy Zaporozhets"), - Email: []byte("dmitriy.zaporozhets@gmail.com"), - Date: ×tamppb.Timestamp{ - Seconds: 1393491698, - }, - Timezone: []byte("+0200"), - }, - SignatureType: gitalypb.SignatureType_PGP, + Subject: []byte("with trailers"), + Body: []byte("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"), + BodySize: 71, + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, Trailers: []*gitalypb.CommitTrailer{ { Key: []byte("Signed-off-by"), - Value: []byte("Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>"), + Value: []byte("John Doe <john.doe@example.com>"), }, }, }, }, { + desc: "with PGP signature", + revision: git.Revision(signedCommit), + opts: []ReadCommitOpt{}, + expectedCommit: &gitalypb.GitCommit{ + Id: signedCommit, + TreeId: treeID.String(), + ParentIds: []string{ + firstParentID.String(), + }, + Subject: []byte("signed commit subject"), + Body: []byte("signed commit subject\n\nsigned commit body\n"), + BodySize: 42, + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, + SignatureType: gitalypb.SignatureType_PGP, + }, + }, + { desc: "not a commit", - revision: "refs/heads/master^{tree}", + revision: "refs/heads/main^{tree}", expectedErr: ErrObjectNotFound, }, } { @@ -401,7 +413,10 @@ func TestRepo_ReadCommit(t *testing.T) { func TestRepo_IsAncestor(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + + parentCommitID := gittest.WriteCommit(t, cfg, repoPath) + childCommitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(parentCommitID)) for _, tc := range []struct { desc string @@ -412,42 +427,42 @@ func TestRepo_IsAncestor(t *testing.T) { }{ { desc: "parent is ancestor", - parent: "HEAD~1", - child: "HEAD", + parent: parentCommitID.Revision(), + child: childCommitID.Revision(), isAncestor: true, }, { desc: "parent is not ancestor", - parent: "HEAD", - child: "HEAD~1", + parent: childCommitID.Revision(), + child: parentCommitID.Revision(), isAncestor: false, }, { desc: "parent is not valid commit", - parent: git.ObjectHashSHA1.ZeroOID.Revision(), - child: "HEAD", + parent: gittest.DefaultObjectHash.ZeroOID.Revision(), + child: childCommitID.Revision(), errorMatcher: func(t testing.TB, err error) { - require.Equal(t, InvalidCommitError(git.ObjectHashSHA1.ZeroOID), err) + require.Equal(t, InvalidCommitError(gittest.DefaultObjectHash.ZeroOID), err) }, }, { desc: "child is not valid commit", - parent: "HEAD", - child: git.ObjectHashSHA1.ZeroOID.Revision(), + parent: childCommitID.Revision(), + child: gittest.DefaultObjectHash.ZeroOID.Revision(), errorMatcher: func(t testing.TB, err error) { - require.Equal(t, InvalidCommitError(git.ObjectHashSHA1.ZeroOID), err) + require.Equal(t, InvalidCommitError(gittest.DefaultObjectHash.ZeroOID), err) }, }, { desc: "child points to a tree", - parent: "HEAD", - child: "HEAD^{tree}", + parent: childCommitID.Revision(), + child: childCommitID.Revision() + "^{tree}", errorMatcher: func(t testing.TB, actualErr error) { - treeOID, err := repo.ResolveRevision(ctx, "HEAD^{tree}") + treeOID, err := repo.ResolveRevision(ctx, childCommitID.Revision()+"^{tree}") require.NoError(t, err) require.EqualError(t, actualErr, fmt.Sprintf( - `determine ancestry: exit status 128, stderr: "error: object %s is a tree, not a commit\nfatal: Not a valid commit name HEAD^{tree}\n"`, - treeOID, + `determine ancestry: exit status 128, stderr: "error: object %s is a tree, not a commit\nfatal: Not a valid commit name %s^{tree}\n"`, + treeOID, childCommitID, )) }, }, diff --git a/internal/git/localrepo/paths_test.go b/internal/git/localrepo/paths_test.go index cd33ec032..e785bd01f 100644 --- a/internal/git/localrepo/paths_test.go +++ b/internal/git/localrepo/paths_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo_test import ( @@ -8,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/quarantine" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" @@ -21,8 +20,10 @@ import ( ) func TestRepo_Path(t *testing.T) { + cfg := testcfg.Build(t) + t.Run("valid repository", func(t *testing.T) { - cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := localrepo.NewTestRepo(t, cfg, repoProto) path, err := repo.Path() @@ -31,7 +32,7 @@ func TestRepo_Path(t *testing.T) { }) t.Run("deleted repository", func(t *testing.T) { - cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := localrepo.NewTestRepo(t, cfg, repoProto) require.NoError(t, os.RemoveAll(repoPath)) @@ -41,7 +42,7 @@ func TestRepo_Path(t *testing.T) { }) t.Run("non-git repository", func(t *testing.T) { - cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := localrepo.NewTestRepo(t, cfg, repoProto) // Recreate the repository as a simple empty directory to simulate @@ -55,7 +56,8 @@ func TestRepo_Path(t *testing.T) { } func TestRepo_ObjectDirectoryPath(t *testing.T) { - cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) + cfg := testcfg.Build(t) + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) locator := config.NewLocator(cfg) ctx := testhelper.Context(t) diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go index 2ba1d988d..f76975e28 100644 --- a/internal/git/localrepo/refs.go +++ b/internal/git/localrepo/refs.go @@ -56,8 +56,13 @@ func (repo *Repo) ResolveRevision(ctx context.Context, revision git.Revision) (g return "", err } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return "", fmt.Errorf("detecting object hash: %w", err) + } + hex := strings.TrimSpace(stdout.String()) - oid, err := git.ObjectHashSHA1.FromHex(hex) + oid, err := objectHash.FromHex(hex) if err != nil { return "", fmt.Errorf("unsupported object hash %q: %w", hex, err) } diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go index 1b7ed148f..c385a8a50 100644 --- a/internal/git/localrepo/refs_test.go +++ b/internal/git/localrepo/refs_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo import ( @@ -9,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -29,15 +28,11 @@ import ( "google.golang.org/grpc/peer" ) -const ( - masterOID = git.ObjectID("1e292f8fedd741b75372e19097c76d327140c312") - nonexistentOID = git.ObjectID("ba4f184e126b751d1bffad5897f263108befc780") -) - func TestRepo_ContainsRef(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master")) testcases := []struct { desc string @@ -73,7 +68,8 @@ func TestRepo_ContainsRef(t *testing.T) { func TestRepo_GetReference(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master")) testcases := []struct { desc string @@ -84,7 +80,7 @@ func TestRepo_GetReference(t *testing.T) { { desc: "fully qualified master branch", ref: "refs/heads/master", - expected: git.NewReference("refs/heads/master", masterOID.String()), + expected: git.NewReference("refs/heads/master", commitID.String()), }, { desc: "unqualified master branch fails", @@ -120,13 +116,10 @@ func TestRepo_GetReference(t *testing.T) { func TestRepo_GetReferenceWithAmbiguousRefs(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t, withDisabledHooks()) - - currentOID, err := repo.ResolveRevision(ctx, "refs/heads/master") - require.NoError(t, err) + cfg, repo, repoPath := setupRepo(t, withDisabledHooks()) - prevOID, err := repo.ResolveRevision(ctx, "refs/heads/master~") - require.NoError(t, err) + prevOID := gittest.WriteCommit(t, cfg, repoPath) + currentOID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(prevOID), gittest.WithBranch("master")) for _, ref := range []git.ReferenceName{ "refs/heads/something/master", @@ -138,7 +131,7 @@ func TestRepo_GetReferenceWithAmbiguousRefs(t *testing.T) { "refs/master", "refs/tags/master", } { - require.NoError(t, repo.UpdateRef(ctx, ref, prevOID, git.ObjectHashSHA1.ZeroOID)) + require.NoError(t, repo.UpdateRef(ctx, ref, prevOID, gittest.DefaultObjectHash.ZeroOID)) } ref, err := repo.GetReference(ctx, "refs/heads/master") @@ -152,59 +145,65 @@ func TestRepo_GetReferenceWithAmbiguousRefs(t *testing.T) { func TestRepo_GetReferences(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main")) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("feature")) + gittest.WriteTag(t, cfg, repoPath, "v1.0.0", "refs/heads/main") - masterBranch, err := repo.GetReference(ctx, "refs/heads/master") + mainReference, err := repo.GetReference(ctx, "refs/heads/main") + require.NoError(t, err) + featureReference, err := repo.GetReference(ctx, "refs/heads/feature") + require.NoError(t, err) + tagReference, err := repo.GetReference(ctx, "refs/tags/v1.0.0") require.NoError(t, err) testcases := []struct { - desc string - patterns []string - match func(t *testing.T, refs []git.Reference) + desc string + patterns []string + expectedRefs []git.Reference }{ { - desc: "master branch", - patterns: []string{"refs/heads/master"}, - match: func(t *testing.T, refs []git.Reference) { - require.Equal(t, []git.Reference{masterBranch}, refs) + desc: "main branch", + patterns: []string{"refs/heads/main"}, + expectedRefs: []git.Reference{ + mainReference, }, }, { desc: "two branches", - patterns: []string{"refs/heads/master", "refs/heads/feature"}, - match: func(t *testing.T, refs []git.Reference) { - featureBranch, err := repo.GetReference(ctx, "refs/heads/feature") - require.NoError(t, err) - - require.Equal(t, []git.Reference{featureBranch, masterBranch}, refs) + patterns: []string{"refs/heads/main", "refs/heads/feature"}, + expectedRefs: []git.Reference{ + featureReference, + mainReference, }, }, { desc: "matching subset is returned", - patterns: []string{"refs/heads/master", "refs/heads/nonexistent"}, - match: func(t *testing.T, refs []git.Reference) { - require.Equal(t, []git.Reference{masterBranch}, refs) + patterns: []string{"refs/heads/main", "refs/heads/nonexistent"}, + expectedRefs: []git.Reference{ + mainReference, }, }, { desc: "all references", - match: func(t *testing.T, refs []git.Reference) { - require.Len(t, refs, 97) + expectedRefs: []git.Reference{ + featureReference, + mainReference, + tagReference, }, }, { desc: "branches", patterns: []string{"refs/heads/"}, - match: func(t *testing.T, refs []git.Reference) { - require.Len(t, refs, 94) + expectedRefs: []git.Reference{ + featureReference, + mainReference, }, }, { desc: "non-existent branch", patterns: []string{"refs/heads/nonexistent"}, - match: func(t *testing.T, refs []git.Reference) { - require.Empty(t, refs) - }, }, } @@ -212,7 +211,7 @@ func TestRepo_GetReferences(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { refs, err := repo.GetReferences(ctx, tc.patterns...) require.NoError(t, err) - tc.match(t, refs) + require.Equal(t, tc.expectedRefs, refs) }) } } @@ -338,21 +337,42 @@ func TestRepo_GetRemoteReferences(t *testing.T) { func TestRepo_GetBranches(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + + mainID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithMessage("main")) + featureID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("feature"), gittest.WithMessage("feature")) + thirdID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("third"), gittest.WithMessage("third")) + + gittest.WriteCommit(t, cfg, repoPath, gittest.WithReference("refs/different/namespace")) + gittest.WriteTag(t, cfg, repoPath, "v1.0.0", mainID.Revision()) refs, err := repo.GetBranches(ctx) require.NoError(t, err) - require.Len(t, refs, 94) + require.Equal(t, []git.Reference{ + {Name: "refs/heads/feature", Target: featureID.String()}, + {Name: "refs/heads/main", Target: mainID.String()}, + {Name: "refs/heads/third", Target: thirdID.String()}, + }, refs) } func TestRepo_UpdateRef(t *testing.T) { ctx := testhelper.Context(t) - cfg, repo, _ := setupRepo(t, withDisabledHooks()) + cfg, repo, repoPath := setupRepo(t, withDisabledHooks()) - otherRef, err := repo.GetReference(ctx, "refs/heads/gitaly-test-ref") + // We move this into a function so that we can re-seed the repository for each test. + seedRepo := func(t *testing.T, repoPath string) { + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithMessage("main")) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("other"), gittest.WithMessage("other")) + } + seedRepo(t, repoPath) + + mainCommitID := gittest.ResolveRevision(t, cfg, repoPath, "refs/heads/main") + otherRef, err := repo.GetReference(ctx, "refs/heads/other") require.NoError(t, err) + nonexistentOID := git.ObjectID(strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen())) + testcases := []struct { desc string ref string @@ -361,95 +381,98 @@ func TestRepo_UpdateRef(t *testing.T) { verify func(t *testing.T, repo *Repo, err error) }{ { - desc: "successfully update master", - ref: "refs/heads/master", + desc: "successfully update main", + ref: "refs/heads/main", newValue: git.ObjectID(otherRef.Target), - oldValue: masterOID, + oldValue: mainCommitID, verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) - ref, err := repo.GetReference(ctx, "refs/heads/master") + ref, err := repo.GetReference(ctx, "refs/heads/main") require.NoError(t, err) require.Equal(t, ref.Target, otherRef.Target) }, }, { desc: "update fails with stale oldValue", - ref: "refs/heads/master", + ref: "refs/heads/main", newValue: git.ObjectID(otherRef.Target), oldValue: nonexistentOID, verify: func(t *testing.T, repo *Repo, err error) { require.Error(t, err) - ref, err := repo.GetReference(ctx, "refs/heads/master") + ref, err := repo.GetReference(ctx, "refs/heads/main") require.NoError(t, err) - require.Equal(t, ref.Target, masterOID.String()) + require.Equal(t, ref.Target, mainCommitID.String()) }, }, { desc: "update fails with invalid newValue", - ref: "refs/heads/master", + ref: "refs/heads/main", newValue: nonexistentOID, - oldValue: masterOID, + oldValue: mainCommitID, verify: func(t *testing.T, repo *Repo, err error) { require.Error(t, err) - ref, err := repo.GetReference(ctx, "refs/heads/master") + ref, err := repo.GetReference(ctx, "refs/heads/main") require.NoError(t, err) - require.Equal(t, ref.Target, masterOID.String()) + require.Equal(t, ref.Target, mainCommitID.String()) }, }, { - desc: "successfully update master with empty oldValue", - ref: "refs/heads/master", + desc: "successfully update main with empty oldValue", + ref: "refs/heads/main", newValue: git.ObjectID(otherRef.Target), oldValue: "", verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) - ref, err := repo.GetReference(ctx, "refs/heads/master") + ref, err := repo.GetReference(ctx, "refs/heads/main") require.NoError(t, err) require.Equal(t, ref.Target, otherRef.Target) }, }, { desc: "updating unqualified branch fails", - ref: "master", + ref: "main", newValue: git.ObjectID(otherRef.Target), - oldValue: masterOID, + oldValue: mainCommitID, verify: func(t *testing.T, repo *Repo, err error) { require.Error(t, err) - ref, err := repo.GetReference(ctx, "refs/heads/master") + ref, err := repo.GetReference(ctx, "refs/heads/main") require.NoError(t, err) - require.Equal(t, ref.Target, masterOID.String()) + require.Equal(t, ref.Target, mainCommitID.String()) }, }, { - desc: "deleting master succeeds", - ref: "refs/heads/master", - newValue: git.ObjectHashSHA1.ZeroOID, - oldValue: masterOID, + desc: "deleting main succeeds", + ref: "refs/heads/main", + newValue: gittest.DefaultObjectHash.ZeroOID, + oldValue: mainCommitID, verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) - _, err = repo.GetReference(ctx, "refs/heads/master") + _, err = repo.GetReference(ctx, "refs/heads/main") require.Error(t, err) }, }, { desc: "creating new branch succeeds", ref: "refs/heads/new", - newValue: masterOID, - oldValue: git.ObjectHashSHA1.ZeroOID, + newValue: mainCommitID, + oldValue: gittest.DefaultObjectHash.ZeroOID, verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/new") require.NoError(t, err) - require.Equal(t, ref.Target, masterOID.String()) + require.Equal(t, ref.Target, mainCommitID.String()) }, }, } for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { - // Re-create repo for each testcase. - repoProto, _ := gittest.CloneRepo(t, cfg, cfg.Storages[0]) + // We need to re-seed the repository every time so that we don't carry over + // the state. + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := New(repo.locator, repo.gitCmdFactory, repo.catfileCache, repoProto) + seedRepo(t, repoPath) + err := repo.UpdateRef(ctx, git.ReferenceName(tc.ref), tc.newValue, tc.oldValue) tc.verify(t, repo, err) }) @@ -458,7 +481,10 @@ func TestRepo_UpdateRef(t *testing.T) { func TestRepo_SetDefaultBranch(t *testing.T) { ctx := testhelper.Context(t) - _, repo, _ := setupRepo(t) + cfg, repo, repoPath := setupRepo(t) + + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master")) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("feature")) txManager := transaction.NewTrackingManager() @@ -620,8 +646,8 @@ func TestRepo_SetDefaultBranch_errors(t *testing.T) { func TestGuessHead(t *testing.T) { cfg, repo, repoPath := setupRepo(t) - commit1 := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/heads/master")) - commit2 := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/heads/feature")) + commit1 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithMessage("main")) + commit2 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("feature"), gittest.WithMessage("feature")) for _, tc := range []struct { desc string @@ -638,37 +664,37 @@ func TestGuessHead(t *testing.T) { { desc: "matching default branch", cmds: [][]string{ - {"update-ref", git.DefaultRef.String(), commit1}, - {"update-ref", git.LegacyDefaultRef.String(), commit2}, - {"update-ref", "refs/heads/apple", commit1}, - {"update-ref", "refs/heads/feature", commit1}, - {"update-ref", "refs/heads/zucchini", commit1}, + {"update-ref", git.DefaultRef.String(), commit1.String()}, + {"update-ref", git.LegacyDefaultRef.String(), commit2.String()}, + {"update-ref", "refs/heads/apple", commit1.String()}, + {"update-ref", "refs/heads/feature", commit1.String()}, + {"update-ref", "refs/heads/zucchini", commit1.String()}, }, - head: git.NewReference("HEAD", commit1), + head: git.NewReference("HEAD", commit1.String()), expected: git.DefaultRef, }, { desc: "matching default legacy branch", cmds: [][]string{ - {"update-ref", git.DefaultRef.String(), commit2}, - {"update-ref", git.LegacyDefaultRef.String(), commit1}, - {"update-ref", "refs/heads/apple", commit1}, - {"update-ref", "refs/heads/feature", commit1}, - {"update-ref", "refs/heads/zucchini", commit1}, + {"update-ref", git.DefaultRef.String(), commit2.String()}, + {"update-ref", git.LegacyDefaultRef.String(), commit1.String()}, + {"update-ref", "refs/heads/apple", commit1.String()}, + {"update-ref", "refs/heads/feature", commit1.String()}, + {"update-ref", "refs/heads/zucchini", commit1.String()}, }, - head: git.NewReference("HEAD", commit1), + head: git.NewReference("HEAD", commit1.String()), expected: git.LegacyDefaultRef, }, { desc: "matching other branch", cmds: [][]string{ - {"update-ref", git.DefaultRef.String(), commit2}, - {"update-ref", git.LegacyDefaultRef.String(), commit2}, - {"update-ref", "refs/heads/apple", commit1}, - {"update-ref", "refs/heads/feature", commit1}, - {"update-ref", "refs/heads/zucchini", commit1}, + {"update-ref", git.DefaultRef.String(), commit2.String()}, + {"update-ref", git.LegacyDefaultRef.String(), commit2.String()}, + {"update-ref", "refs/heads/apple", commit1.String()}, + {"update-ref", "refs/heads/feature", commit1.String()}, + {"update-ref", "refs/heads/zucchini", commit1.String()}, }, - head: git.NewReference("HEAD", commit1), + head: git.NewReference("HEAD", commit1.String()), expected: "refs/heads/apple", }, { @@ -676,23 +702,23 @@ func TestGuessHead(t *testing.T) { cmds: [][]string{ {"update-ref", "-d", git.DefaultRef.String()}, {"update-ref", "-d", git.LegacyDefaultRef.String()}, - {"update-ref", "refs/heads/apple", commit1}, - {"update-ref", "refs/heads/feature", commit1}, - {"update-ref", "refs/heads/zucchini", commit1}, + {"update-ref", "refs/heads/apple", commit1.String()}, + {"update-ref", "refs/heads/feature", commit1.String()}, + {"update-ref", "refs/heads/zucchini", commit1.String()}, }, - head: git.NewReference("HEAD", commit1), + head: git.NewReference("HEAD", commit1.String()), expected: "refs/heads/apple", }, { desc: "no match", cmds: [][]string{ - {"update-ref", git.DefaultRef.String(), commit2}, - {"update-ref", git.LegacyDefaultRef.String(), commit2}, - {"update-ref", "refs/heads/apple", commit2}, - {"update-ref", "refs/heads/feature", commit2}, - {"update-ref", "refs/heads/zucchini", commit2}, + {"update-ref", git.DefaultRef.String(), commit2.String()}, + {"update-ref", git.LegacyDefaultRef.String(), commit2.String()}, + {"update-ref", "refs/heads/apple", commit2.String()}, + {"update-ref", "refs/heads/feature", commit2.String()}, + {"update-ref", "refs/heads/zucchini", commit2.String()}, }, - head: git.NewReference("HEAD", commit1), + head: git.NewReference("HEAD", commit1.String()), expectedErr: fmt.Errorf("guess head: %w", git.ErrReferenceNotFound), }, } { diff --git a/internal/git/localrepo/remote_extra_test.go b/internal/git/localrepo/remote_extra_test.go index 634229054..dafd7e8ae 100644 --- a/internal/git/localrepo/remote_extra_test.go +++ b/internal/git/localrepo/remote_extra_test.go @@ -52,22 +52,17 @@ func TestRepo_FetchInternal(t *testing.T) { )) }, testserver.WithGitCommandFactory(protocolDetectingFactory)) - remoteRepoProto, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) - - remoteRepo := localrepo.NewTestRepo(t, cfg, remoteRepoProto) testcfg.BuildGitalySSH(t, cfg) testcfg.BuildGitalyHooks(t, cfg) - remoteOID, err := remoteRepo.ResolveRevision(ctx, git.Revision("refs/heads/master")) - require.NoError(t, err) - - tagV100OID, err := remoteRepo.ResolveRevision(ctx, git.Revision("refs/tags/v1.0.0")) - require.NoError(t, err) - - tagV110OID, err := remoteRepo.ResolveRevision(ctx, git.Revision("refs/tags/v1.1.0")) - require.NoError(t, err) + remoteRepoProto, remoteRepoPath := gittest.CreateRepository(ctx, t, cfg) + remoteOID := gittest.WriteCommit(t, cfg, remoteRepoPath, gittest.WithBranch("master")) + tagV100OID := gittest.WriteTag(t, cfg, remoteRepoPath, "v1.0.0", remoteOID.Revision(), gittest.WriteTagConfig{ + Message: "v1.0.0", + }) + tagV110OID := gittest.WriteTag(t, cfg, remoteRepoPath, "v1.1.0", remoteOID.Revision(), gittest.WriteTagConfig{ + Message: "v1.1.0", + }) t.Run("refspec with tag", func(t *testing.T) { ctx := testhelper.MergeIncomingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg)) @@ -205,14 +200,12 @@ func TestRepo_FetchInternal(t *testing.T) { t.Run("pruning", func(t *testing.T) { ctx := testhelper.MergeIncomingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg)) - repoProto, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: gittest.SeedGitLabTest, - }) + repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg) repo := localrepo.NewTestRepo(t, cfg, repoProto) // Create a local reference. Given that it doesn't exist on the remote side, it // would get pruned if we pass `--prune`. - require.NoError(t, repo.UpdateRef(ctx, "refs/heads/prune-me", remoteOID, git.ObjectHashSHA1.ZeroOID)) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("prune-me")) // By default, refs are not pruned. require.NoError(t, repo.FetchInternal( diff --git a/internal/git/localrepo/remote_test.go b/internal/git/localrepo/remote_test.go index 18b2e7ded..fe65206d6 100644 --- a/internal/git/localrepo/remote_test.go +++ b/internal/git/localrepo/remote_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo import ( @@ -30,11 +28,15 @@ func TestRepo_FetchRemote(t *testing.T) { defer catfileCache.Stop() locator := config.NewLocator(cfg) + _, remoteRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) + commitID := gittest.WriteCommit(t, cfg, remoteRepoPath, gittest.WithBranch("main")) + tagID := gittest.WriteTag(t, cfg, remoteRepoPath, "v1.0.0", commitID.Revision(), gittest.WriteTagConfig{ + Message: "annotated tag", + }) + initBareWithRemote := func(t *testing.T, remote string) (*Repo, string) { t.Helper() - _, remoteRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) - clientRepo, clientRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) cmd := gittest.NewCommand(t, cfg, "-C", clientRepoPath, "remote", "add", remote, remoteRepoPath) @@ -74,22 +76,21 @@ func TestRepo_FetchRemote(t *testing.T) { refs, err := repo.GetReferences(ctx) require.NoError(t, err) - require.Contains(t, refs, git.Reference{Name: "refs/remotes/origin/'test'", Target: "e56497bb5f03a90a51293fc6d516788730953899"}) - require.Contains(t, refs, git.Reference{Name: "refs/tags/v1.1.0", Target: "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b"}) + require.Contains(t, refs, git.Reference{Name: "refs/remotes/origin/main", Target: commitID.String()}) + require.Contains(t, refs, git.Reference{Name: "refs/tags/v1.0.0", Target: tagID.String()}) - sha, err := repo.ResolveRevision(ctx, git.Revision("refs/remotes/origin/master^{commit}")) + fetchedCommitID, err := repo.ResolveRevision(ctx, git.Revision("refs/remotes/origin/main^{commit}")) require.NoError(t, err, "the object from remote should exists in local after fetch done") - require.Equal(t, git.ObjectID("1e292f8fedd741b75372e19097c76d327140c312"), sha) + require.Equal(t, commitID, fetchedCommitID) require.NoFileExists(t, filepath.Join(repoPath, "FETCH_HEAD")) }) t.Run("with env", func(t *testing.T) { - _, sourceRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) - testRepo, testRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) + testRepo, testRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := New(locator, gitCmdFactory, catfileCache, testRepo) - gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) + gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", remoteRepoPath) var stderr bytes.Buffer require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{Stderr: &stderr, Env: []string{"GIT_TRACE=1"}})) @@ -97,11 +98,10 @@ func TestRepo_FetchRemote(t *testing.T) { }) t.Run("with disabled transactions", func(t *testing.T) { - _, sourceRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) - testRepo, testRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) + testRepo, testRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := New(locator, gitCmdFactory, catfileCache, testRepo) - gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) + gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", remoteRepoPath) var stderr bytes.Buffer require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{ @@ -113,16 +113,16 @@ func TestRepo_FetchRemote(t *testing.T) { }) t.Run("with globals", func(t *testing.T) { - _, sourceRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) - testRepo, testRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) + testRepo, testRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := New(locator, gitCmdFactory, catfileCache, testRepo) - gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) + gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", remoteRepoPath) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{})) - gittest.Exec(t, cfg, "-C", testRepoPath, "branch", "--track", "testing-fetch-prune", "refs/remotes/source/markdown") - gittest.Exec(t, cfg, "-C", sourceRepoPath, "branch", "-D", "markdown") + // Write a commit into the remote's reference namespace that doesn't exist in the + // remote and that would thus be pruned. + gittest.WriteCommit(t, cfg, testRepoPath, gittest.WithReference("refs/remotes/source/markdown")) require.NoError(t, repo.FetchRemote( ctx, @@ -140,16 +140,15 @@ func TestRepo_FetchRemote(t *testing.T) { }) t.Run("with prune", func(t *testing.T) { - _, sourceRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) - testRepo, testRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0]) + testRepo, testRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) repo := New(locator, gitCmdFactory, catfileCache, testRepo) - gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) + gittest.Exec(t, cfg, "-C", testRepoPath, "remote", "add", "source", remoteRepoPath) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{})) - - gittest.Exec(t, cfg, "-C", testRepoPath, "branch", "--track", "testing-fetch-prune", "refs/remotes/source/markdown") - gittest.Exec(t, cfg, "-C", sourceRepoPath, "branch", "-D", "markdown") + // Write a commit into the remote's reference namespace that doesn't exist in the + // remote and that would thus be pruned. + gittest.WriteCommit(t, cfg, testRepoPath, gittest.WithReference("refs/remotes/source/markdown")) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{Prune: true})) @@ -241,14 +240,17 @@ func captureGitSSHCommand(ctx context.Context, t testing.TB, cfg config.Cfg) (gi func TestRepo_Push(t *testing.T) { ctx := testhelper.Context(t) - cfg, sourceRepoPb, _ := testcfg.BuildWithRepo(t) + cfg := testcfg.Build(t) gitCmdFactory, readSSHCommand := captureGitSSHCommand(ctx, t, cfg) catfileCache := catfile.NewCache(cfg) t.Cleanup(catfileCache.Stop) locator := config.NewLocator(cfg) - sourceRepo := New(locator, gitCmdFactory, catfileCache, sourceRepoPb) + sourceRepoProto, sourceRepoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) + sourceRepo := New(locator, gitCmdFactory, catfileCache, sourceRepoProto) + gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithBranch("master")) + gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithBranch("feature")) setupPushRepo := func(t testing.TB) (*Repo, string, []git.ConfigPair) { repoProto, repopath := gittest.InitRepo(t, cfg, cfg.Storages[0]) diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go index 730176543..26e097350 100644 --- a/internal/git/localrepo/repo.go +++ b/internal/git/localrepo/repo.go @@ -9,6 +9,7 @@ import ( "os" "strconv" "strings" + "sync" "testing" "github.com/stretchr/testify/require" @@ -30,6 +31,10 @@ type Repo struct { locator storage.Locator gitCmdFactory git.CommandFactory catfileCache catfile.Cache + + detectObjectHashOnce sync.Once + objectHash git.ObjectHash + objectHashErr error } // New creates a new Repo from its protobuf representation. @@ -220,3 +225,11 @@ func (repo *Repo) StorageTempDir() (string, error) { return tempPath, nil } + +// ObjectHash detects the object hash used by this particular repository. +func (repo *Repo) ObjectHash(ctx context.Context) (git.ObjectHash, error) { + repo.detectObjectHashOnce.Do(func() { + repo.objectHash, repo.objectHashErr = git.DetectObjectHash(ctx, repo) + }) + return repo.objectHash, repo.objectHashErr +} diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go index 8d68dc821..e2f8fa8ff 100644 --- a/internal/git/localrepo/repo_test.go +++ b/internal/git/localrepo/repo_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo import ( @@ -24,24 +22,15 @@ import ( func TestRepo(t *testing.T) { cfg := testcfg.Build(t) - gittest.TestRepository(t, cfg, func(ctx context.Context, t testing.TB, seeded bool) (git.Repository, string) { + gittest.TestRepository(t, cfg, func(ctx context.Context, t testing.TB) (git.Repository, string) { t.Helper() - var ( - pbRepo *gitalypb.Repository - repoPath string - ) - - if seeded { - pbRepo, repoPath = gittest.CloneRepo(t, cfg, cfg.Storages[0]) - } else { - pbRepo, repoPath = gittest.InitRepo(t, cfg, cfg.Storages[0]) - } + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) gitCmdFactory := gittest.NewCommandFactory(t, cfg) catfileCache := catfile.NewCache(cfg) t.Cleanup(catfileCache.Stop) - return New(config.NewLocator(cfg), gitCmdFactory, catfileCache, pbRepo), repoPath + return New(config.NewLocator(cfg), gitCmdFactory, catfileCache, repoProto), repoPath }) } @@ -62,6 +51,13 @@ func TestSize(t *testing.T) { `, commandArgFile, execEnv.BinaryPath) }) + hashDependentSize := func(sha1Size, sha256Size int64) int64 { + if gittest.ObjectHashIsSHA256() { + return sha256Size + } + return sha1Size + } + testCases := []struct { desc string setup func(t *testing.T) *gitalypb.Repository @@ -92,7 +88,7 @@ func TestSize(t *testing.T) { return repoProto }, - expectedSize: 203, + expectedSize: hashDependentSize(203, 230), expectedUseBitmap: true, }, { @@ -133,7 +129,7 @@ func TestSize(t *testing.T) { return repoProto }, - expectedSize: 439, + expectedSize: hashDependentSize(439, 510), expectedUseBitmap: true, }, { @@ -160,7 +156,7 @@ func TestSize(t *testing.T) { return repoProto }, - expectedSize: 398, + expectedSize: hashDependentSize(398, 465), expectedUseBitmap: true, }, { @@ -187,7 +183,7 @@ func TestSize(t *testing.T) { opts: []RepoSizeOption{ WithExcludeRefs("refs/heads/exclude-me"), }, - expectedSize: 217, + expectedSize: hashDependentSize(217, 245), expectedUseBitmap: true, }, { @@ -235,7 +231,7 @@ func TestSize(t *testing.T) { }, // While both repositories have the same contents, we should still return // the actual repository's size because we don't exclude the alternate. - expectedSize: 207, + expectedSize: hashDependentSize(207, 234), // Even though we have an alternate, we should still use bitmap indices // given that we don't use `--not --alternate-refs`. expectedUseBitmap: true, @@ -309,7 +305,7 @@ func TestSize(t *testing.T) { opts: []RepoSizeOption{ WithoutAlternates(), }, - expectedSize: 224, + expectedSize: hashDependentSize(224, 268), expectedUseBitmap: false, }, } @@ -342,8 +338,8 @@ func TestRepo_StorageTempDir(t *testing.T) { t.Cleanup(catfileCache.Stop) locator := config.NewLocator(cfg) - pbRepo, _ := gittest.CloneRepo(t, cfg, cfg.Storages[0]) - repo := New(locator, gitCmdFactory, catfileCache, pbRepo) + repoProto, _ := gittest.InitRepo(t, cfg, cfg.Storages[0]) + repo := New(locator, gitCmdFactory, catfileCache, repoProto) expected, err := locator.TempDir(cfg.Storages[0].Name) require.NoError(t, err) @@ -354,3 +350,44 @@ func TestRepo_StorageTempDir(t *testing.T) { require.DirExists(t, expected) require.Equal(t, expected, tempPath) } + +func TestRepo_ObjectHash(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + cfg := testcfg.Build(t) + + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + locator := config.NewLocator(cfg) + + outputFile := filepath.Join(testhelper.TempDir(t), "output") + + // We create an intercepting command factory that detects when we run our object hash + // detection logic and, if so, writes a sentinel value into our output file. Like this we + // can test how often the logic runs. + gitCmdFactory := gittest.NewInterceptingCommandFactory(ctx, t, cfg, func(execEnv git.ExecutionEnvironment) string { + return fmt.Sprintf(`#!/bin/sh + ( echo "$@" | grep --silent -- '--show-object-format' ) && echo detection-logic >>%q + exec %q "$@"`, outputFile, execEnv.BinaryPath) + }) + + repoProto, _ := gittest.InitRepo(t, cfg, cfg.Storages[0]) + repo := New(locator, gitCmdFactory, catfileCache, repoProto) + + objectHash, err := repo.ObjectHash(ctx) + require.NoError(t, err) + require.Equal(t, gittest.DefaultObjectHash.EmptyTreeOID, objectHash.EmptyTreeOID) + + // We should see that the detection logic has been executed once. + require.Equal(t, "detection-logic\n", string(testhelper.MustReadFile(t, outputFile))) + + // Verify that running this a second time continues to return the object hash alright + // regardless of the cache. + objectHash, err = repo.ObjectHash(ctx) + require.NoError(t, err) + require.Equal(t, gittest.DefaultObjectHash.EmptyTreeOID, objectHash.EmptyTreeOID) + + // But the detection logic should not have been executed a second time. + require.Equal(t, "detection-logic\n", string(testhelper.MustReadFile(t, outputFile))) +} diff --git a/internal/git/localrepo/testhelper_test.go b/internal/git/localrepo/testhelper_test.go index 8fb0bd4f7..28fc6a851 100644 --- a/internal/git/localrepo/testhelper_test.go +++ b/internal/git/localrepo/testhelper_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package localrepo import ( @@ -11,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" - "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func TestMain(m *testing.M) { @@ -19,21 +16,12 @@ func TestMain(m *testing.M) { } type setupRepoConfig struct { - // emptyRepo will cause `setupRepo()` to create a new, empty repository instead of - // cloning our test repository. - emptyRepo bool // disableHooks will disable the use of hooks. disableHooks bool } type setupRepoOption func(*setupRepoConfig) -func withEmptyRepo() setupRepoOption { - return func(cfg *setupRepoConfig) { - cfg.emptyRepo = true - } -} - func withDisabledHooks() setupRepoOption { return func(cfg *setupRepoConfig) { cfg.disableHooks = true @@ -55,13 +43,7 @@ func setupRepo(t *testing.T, opts ...setupRepoOption) (config.Cfg, *Repo, string commandFactoryOpts = append(commandFactoryOpts, git.WithSkipHooks()) } - var repoProto *gitalypb.Repository - var repoPath string - if setupRepoCfg.emptyRepo { - repoProto, repoPath = gittest.InitRepo(t, cfg, cfg.Storages[0]) - } else { - repoProto, repoPath = gittest.CloneRepo(t, cfg, cfg.Storages[0]) - } + repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) gitCmdFactory := gittest.NewCommandFactory(t, cfg, commandFactoryOpts...) catfileCache := catfile.NewCache(cfg) diff --git a/internal/git/object_id.go b/internal/git/object_id.go index 88db74d44..2826215d3 100644 --- a/internal/git/object_id.go +++ b/internal/git/object_id.go @@ -71,6 +71,11 @@ func DetectObjectHash(ctx context.Context, repoExecutor RepositoryExecutor) (Obj } } +// EncodedLen returns the length of the hex-encoded string of a full object ID. +func (h ObjectHash) EncodedLen() int { + return hex.EncodedLen(h.Hash().Size()) +} + // FromHex constructs a new ObjectID from the given hex representation of the object ID. Returns // ErrInvalidObjectID if the given object ID is not valid. func (h ObjectHash) FromHex(hex string) (ObjectID, error) { diff --git a/internal/git/object_id_test.go b/internal/git/object_id_test.go index 44ac98a46..8c8502ef4 100644 --- a/internal/git/object_id_test.go +++ b/internal/git/object_id_test.go @@ -267,6 +267,12 @@ func TestObjectHash_FromHex(t *testing.T) { } } +func TestObjectHash_EncodedLen(t *testing.T) { + t.Parallel() + require.Equal(t, 40, git.ObjectHashSHA1.EncodedLen()) + require.Equal(t, 64, git.ObjectHashSHA256.EncodedLen()) +} + func TestObjectID_Bytes(t *testing.T) { for _, tc := range []struct { desc string diff --git a/internal/git/remoterepo/repository_test.go b/internal/git/remoterepo/repository_test.go index d99af35f3..7057718bb 100644 --- a/internal/git/remoterepo/repository_test.go +++ b/internal/git/remoterepo/repository_test.go @@ -55,22 +55,15 @@ func TestRepository(t *testing.T) { pool := client.NewPool() defer pool.Close() - gittest.TestRepository(t, cfg, func(ctx context.Context, t testing.TB, seeded bool) (git.Repository, string) { + gittest.TestRepository(t, cfg, func(ctx context.Context, t testing.TB) (git.Repository, string) { t.Helper() - seed := "" - if seeded { - seed = gittest.SeedGitLabTest - } - ctx, err := storage.InjectGitalyServers(ctx, "default", cfg.SocketPath, cfg.Auth.Token) require.NoError(t, err) - pbRepo, repoPath := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{ - Seed: seed, - }) + repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg) - repo, err := remoterepo.New(metadata.OutgoingToIncoming(ctx), pbRepo, pool) + repo, err := remoterepo.New(metadata.OutgoingToIncoming(ctx), repoProto, pool) require.NoError(t, err) return repo, repoPath }) diff --git a/internal/git/repository.go b/internal/git/repository.go index b8b764e6c..8f969bdf5 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -55,4 +55,5 @@ type RepositoryExecutor interface { Exec(ctx context.Context, cmd Cmd, opts ...CmdOpt) (*command.Command, error) ExecAndWait(ctx context.Context, cmd Cmd, opts ...CmdOpt) error GitVersion(ctx context.Context) (Version, error) + ObjectHash(ctx context.Context) (ObjectHash, error) } diff --git a/internal/gitaly/service/commit/find_commit_test.go b/internal/gitaly/service/commit/find_commit_test.go index 42470db91..583c5f755 100644 --- a/internal/gitaly/service/commit/find_commit_test.go +++ b/internal/gitaly/service/commit/find_commit_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -27,15 +26,11 @@ func TestSuccessfulFindCommitRequest(t *testing.T) { ctx := testhelper.Context(t) cfg, repoProto, repoPath, client := setupCommitServiceWithRepo(ctx, t) - repo := localrepo.NewTestRepo(t, cfg, repoProto) - bigMessage := "An empty commit with REALLY BIG message\n\n" + strings.Repeat("MOAR!\n", 20*1024) bigCommitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("local-big-commits"), gittest.WithMessage(bigMessage), gittest.WithParents("60ecb67744cb56576c30214ff52294f8ce2def98"), ) - bigCommit, err := repo.ReadCommit(ctx, git.Revision(bigCommitID)) - require.NoError(t, err) testCases := []struct { description string @@ -200,20 +195,10 @@ func TestSuccessfulFindCommitRequest(t *testing.T) { description: "with a very large message", revision: bigCommitID.String(), commit: &gitalypb.GitCommit{ - Id: bigCommitID.String(), - Subject: []byte("An empty commit with REALLY BIG message"), - Author: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: ×tamppb.Timestamp{Seconds: bigCommit.Author.Date.Seconds}, - Timezone: []byte("+0100"), - }, - Committer: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: ×tamppb.Timestamp{Seconds: bigCommit.Committer.Date.Seconds}, - Timezone: []byte("+0100"), - }, + Id: bigCommitID.String(), + Subject: []byte("An empty commit with REALLY BIG message"), + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, ParentIds: []string{"60ecb67744cb56576c30214ff52294f8ce2def98"}, Body: []byte(bigMessage[:helper.MaxCommitOrTagMessageSize]), BodySize: int64(len(bigMessage)), diff --git a/internal/gitaly/service/commit/list_all_commits_test.go b/internal/gitaly/service/commit/list_all_commits_test.go index c0af3db84..ae30a7fa3 100644 --- a/internal/gitaly/service/commit/list_all_commits_test.go +++ b/internal/gitaly/service/commit/list_all_commits_test.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" - "google.golang.org/protobuf/types/known/timestamppb" ) func TestListAllCommits(t *testing.T) { @@ -126,23 +125,13 @@ func TestListAllCommits(t *testing.T) { require.NoError(t, err) require.Equal(t, []*gitalypb.GitCommit{{ - Id: commitID.String(), - Subject: []byte("message"), - Body: []byte("message"), - BodySize: 7, - TreeId: "4b825dc642cb6eb9a060e54bf8d69288fbee4904", - Author: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: ×tamppb.Timestamp{Seconds: 1572776879}, - Timezone: []byte("+0100"), - }, - Committer: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: ×tamppb.Timestamp{Seconds: 1572776879}, - Timezone: []byte("+0100"), - }, + Id: commitID.String(), + Subject: []byte("message"), + Body: []byte("message"), + BodySize: 7, + TreeId: "4b825dc642cb6eb9a060e54bf8d69288fbee4904", + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, }}, receiveCommits(t, stream)) }) } diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go index 7bfbf37d6..8f7742c25 100644 --- a/internal/gitaly/service/operations/rebase_test.go +++ b/internal/gitaly/service/operations/rebase_test.go @@ -173,12 +173,7 @@ func TestUserRebaseConfirmable_skipEmptyCommits(t *testing.T) { Id: "ef7f98be1f753f1a9fa895d999a855611d691629", ParentIds: []string{theirs.String()}, TreeId: "b68aeb18813d7f2e180f2cc0bccc128511438b29", - Author: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Author: gittest.DefaultCommitAuthor, Committer: &gitalypb.CommitAuthor{ Name: gittest.TestUser.Name, Email: gittest.TestUser.Email, diff --git a/internal/gitaly/service/ref/find_all_tags_test.go b/internal/gitaly/service/ref/find_all_tags_test.go index 6ae5ec5fe..d270ad956 100644 --- a/internal/gitaly/service/ref/find_all_tags_test.go +++ b/internal/gitaly/service/ref/find_all_tags_test.go @@ -11,7 +11,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" @@ -97,12 +96,7 @@ func TestFindAllTags_successful(t *testing.T) { TargetCommit: gitCommit, Message: []byte("commit tag with a commit sha as the name"), MessageSize: 40, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, { Name: []byte("tag-of-tag"), @@ -110,12 +104,7 @@ func TestFindAllTags_successful(t *testing.T) { TargetCommit: gitCommit, Message: []byte("tag of a tag"), MessageSize: 12, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, { Name: []byte("v1.0.0"), @@ -176,12 +165,7 @@ func TestFindAllTags_successful(t *testing.T) { Id: annotatedTagID.String(), Message: []byte("Blob tag"), MessageSize: 8, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, { Name: []byte("v1.3.0"), @@ -208,12 +192,7 @@ func TestFindAllTags_successful(t *testing.T) { Message: []byte(bigMessage[:helper.MaxCommitOrTagMessageSize]), MessageSize: int64(len(bigMessage)), TargetCommit: gitCommit, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, } @@ -242,23 +221,13 @@ func TestFindAllTags_simpleNestedTags(t *testing.T) { Name: []byte("my/nested/tag"), Id: tagID.String(), TargetCommit: &gitalypb.GitCommit{ - Id: commitID.String(), - Body: []byte("message"), - BodySize: 7, - Subject: []byte("message"), - TreeId: git.ObjectHashSHA1.EmptyTreeOID.String(), - Author: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, - Committer: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Id: commitID.String(), + Body: []byte("message"), + BodySize: 7, + Subject: []byte("message"), + TreeId: git.ObjectHashSHA1.EmptyTreeOID.String(), + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, }, }, }, @@ -303,20 +272,14 @@ func TestFindAllTags_duplicateAnnotatedTags(t *testing.T) { receivedTags = append(receivedTags, r.GetTags()...) } - commitAuthor := &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - } commit := &gitalypb.GitCommit{ Id: commitID.String(), TreeId: "4b825dc642cb6eb9a060e54bf8d69288fbee4904", Body: []byte("message"), BodySize: 7, Subject: []byte("message"), - Author: commitAuthor, - Committer: commitAuthor, + Author: gittest.DefaultCommitAuthor, + Committer: gittest.DefaultCommitAuthor, } testhelper.ProtoEqual(t, []*gitalypb.Tag{ @@ -426,12 +389,7 @@ func TestFindAllTags_nestedTags(t *testing.T) { Id: tagID.String(), Message: []byte(tagMessage), MessageSize: int64(len([]byte(tagMessage))), - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, } if info.Type == "commit" { @@ -460,7 +418,7 @@ func TestFindAllTags_nestedTags(t *testing.T) { require.Len(t, receivedTags, len(expectedTags)) for _, receivedTag := range receivedTags { - assert.Equal(t, expectedTags[string(receivedTag.Name)], receivedTag) + testhelper.ProtoEqual(t, expectedTags[string(receivedTag.Name)], receivedTag) } }) } diff --git a/internal/gitaly/service/ref/find_tag_test.go b/internal/gitaly/service/ref/find_tag_test.go index 6461e9e77..d9359a31b 100644 --- a/internal/gitaly/service/ref/find_tag_test.go +++ b/internal/gitaly/service/ref/find_tag_test.go @@ -69,12 +69,7 @@ func TestFindTag_successful(t *testing.T) { TargetCommit: gitCommit, Message: []byte("commit tag with a commit sha as the name"), MessageSize: 40, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, { Name: []byte("tag-of-tag"), @@ -82,12 +77,7 @@ func TestFindTag_successful(t *testing.T) { TargetCommit: gitCommit, Message: []byte("tag of a tag"), MessageSize: 12, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, { Name: []byte("v1.0.0"), @@ -135,12 +125,7 @@ func TestFindTag_successful(t *testing.T) { Id: annotatedTagID.String(), Message: []byte("Blob tag"), MessageSize: 8, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, { Name: []byte("v1.3.0"), @@ -167,12 +152,7 @@ func TestFindTag_successful(t *testing.T) { Message: []byte(bigMessage[:helper.MaxCommitOrTagMessageSize]), MessageSize: int64(len(bigMessage)), TargetCommit: gitCommit, - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, }, } @@ -263,12 +243,7 @@ func TestFindTag_nestedTag(t *testing.T) { Id: tagID.String(), Message: []byte(tagMessage), MessageSize: int64(len([]byte(tagMessage))), - Tagger: &gitalypb.CommitAuthor{ - Name: []byte(gittest.DefaultCommitterName), - Email: []byte(gittest.DefaultCommitterMail), - Date: timestamppb.New(gittest.DefaultCommitTime), - Timezone: []byte("+0100"), - }, + Tagger: gittest.DefaultCommitAuthor, } if info.Type == "commit" { commit, err := catfile.GetCommit(ctx, objectReader, git.Revision(tc.originalOid)) @@ -279,7 +254,7 @@ func TestFindTag_nestedTag(t *testing.T) { resp, err := client.FindTag(ctx, rpcRequest) require.NoError(t, err) - require.Equal(t, expectedTag, resp.GetTag()) + testhelper.ProtoEqual(t, expectedTag, resp.GetTag()) }) } } |