From 9b6394d518a5c87fe19324659ad77581dce2834f Mon Sep 17 00:00:00 2001 From: John Cai Date: Fri, 11 Nov 2022 09:28:59 -0500 Subject: commit: Add a commit package for writing commits There are times when we will need to write commits without a worktree. This is quite a manual process, and having a helper can be very convenient. --- internal/git/commit/commit.go | 174 +++++++++++++++ internal/git/commit/commit_test.go | 233 +++++++++++++++++++++ internal/git/tree/write_test.go | 13 +- internal/gitaly/service/operations/submodules.go | 113 +++++++++- .../gitaly/service/operations/submodules_test.go | 4 +- 5 files changed, 523 insertions(+), 14 deletions(-) create mode 100644 internal/git/commit/commit.go create mode 100644 internal/git/commit/commit_test.go diff --git a/internal/git/commit/commit.go b/internal/git/commit/commit.go new file mode 100644 index 000000000..5e5e7575d --- /dev/null +++ b/internal/git/commit/commit.go @@ -0,0 +1,174 @@ +package commit + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" +) + +var ( + ErrMissingTree = errors.New("tree is missing") + // ErrMissingAuthor indicates the author is missing + ErrMissingAuthor = errors.New("author is missing") + // ErrMissingCommitter indicates the committer is missing + ErrMissingCommitter = errors.New("committer is missing") + // ErrMissingAuthorEmail indicates the author email is missing + ErrMissingAuthorEmail = errors.New("author email is missing") + // ErrMissingCommmitterEmail indicates the committer email is missing + ErrMissingCommitterEmail = errors.New("committer email is missing") +) + +type CommitConfig struct { + Reference string + Parents []git.ObjectID + AuthorDate time.Time + AuthorName string + AuthorEmail string + CommitterName string + CommitterEmail string + CommitterDate time.Time + Message string + TreeID git.ObjectID + AlternateObjectDir string +} + +// Write writes a new commit into the target repository. +func Write(ctx context.Context, repo *localrepo.Repo, cfg CommitConfig) (git.ObjectID, error) { + var tree git.ObjectID + var err error + + if cfg.TreeID == "" { + return "", ErrMissingTree + } + + tree = cfg.TreeID + + if cfg.AuthorName == "" { + return "", ErrMissingAuthor + } + + if cfg.CommitterName == "" { + return "", ErrMissingCommitter + } + + if cfg.AuthorEmail == "" { + return "", ErrMissingAuthorEmail + } + + if cfg.CommitterEmail == "" { + return "", ErrMissingCommitterEmail + } + + if cfg.AuthorDate.IsZero() { + cfg.AuthorDate = time.Now() + } + + if cfg.CommitterDate.IsZero() { + cfg.CommitterDate = time.Now() + } + + // Use 'commit-tree' instead of 'commit' because we are in a bare + // repository. What we do here is the same as "commit -m message + // --allow-empty". + commitArgs := []string{string(tree)} + + flags := []git.Option{ + git.ValueFlag{Name: "-m", Value: cfg.Message}, + } + + repoPath, err := repo.Path() + if err != nil { + return "", err + } + + var env []string + if cfg.AlternateObjectDir != "" { + if !filepath.IsAbs(cfg.AlternateObjectDir) { + return "", errors.New("alternate object directory must be an absolute path") + } + + if err := os.MkdirAll(cfg.AlternateObjectDir, 0o755); err != nil { + return "", err + } + + env = append(env, + fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", cfg.AlternateObjectDir), + fmt.Sprintf("GIT_ALTERNATE_OBJECT_DIRECTORIES=%s", filepath.Join(repoPath, "objects")), + ) + } + + env = append(env, + fmt.Sprintf("GIT_AUTHOR_DATE=%d %s", cfg.AuthorDate.Unix(), cfg.AuthorDate.Format("-0700")), + fmt.Sprintf("GIT_AUTHOR_NAME=%s", cfg.AuthorName), + fmt.Sprintf("GIT_AUTHOR_EMAIL=%s", cfg.AuthorEmail), + fmt.Sprintf("GIT_COMMITTER_DATE=%d %s", cfg.CommitterDate.Unix(), cfg.CommitterDate.Format("-0700")), + fmt.Sprintf("GIT_COMMITTER_NAME=%s", cfg.CommitterName), + fmt.Sprintf("GIT_COMMITTER_EMAIL=%s", cfg.CommitterEmail), + ) + + for _, parent := range cfg.Parents { + flags = append(flags, git.ValueFlag{Name: "-p", Value: parent.String()}) + } + + var stdout, stderr bytes.Buffer + + if err := repo.ExecAndWait(ctx, + git.SubCmd{ + Name: "commit-tree", + Flags: flags, + Args: commitArgs, + }, + git.WithStdout(&stdout), + git.WithStderr(&stderr), + git.WithEnv(env...), + ); err != nil { + return "", fmt.Errorf("commit-tree: %w: %s %s", err, stderr.String(), stdout.String()) + } + + oid, err := git.ObjectHashSHA1.FromHex(text.ChompBytes(stdout.Bytes())) + if err != nil { + return "", err + } + + if cfg.Reference != "" { + if err := repo.UpdateRef( + ctx, + git.ReferenceName(cfg.Reference), + oid, + git.ObjectHashSHA1.ZeroOID, + ); err != nil { + return "", err + } + } + + return oid, nil +} + +func authorEqualIgnoringDate(tb testing.TB, expected *gitalypb.CommitAuthor, actual *gitalypb.CommitAuthor) { + tb.Helper() + require.Equal(tb, expected.GetName(), actual.GetName(), "author name does not match") + require.Equal(tb, expected.GetEmail(), actual.GetEmail(), "author mail does not match") +} + +// CommitEqual tests if two `GitCommit`s are equal +func CommitEqual(tb testing.TB, expected, actual *gitalypb.GitCommit) { + tb.Helper() + + authorEqualIgnoringDate(tb, expected.GetAuthor(), actual.GetAuthor()) + authorEqualIgnoringDate(tb, expected.GetCommitter(), actual.GetCommitter()) + require.Equal(tb, expected.GetBody(), actual.GetBody(), "body does not match") + require.Equal(tb, expected.GetSubject(), actual.GetSubject(), "subject does not match") + require.Equal(tb, expected.GetId(), actual.GetId(), "object ID does not match") + require.Equal(tb, expected.GetParentIds(), actual.GetParentIds(), "parent IDs do not match") +} diff --git a/internal/git/commit/commit_test.go b/internal/git/commit/commit_test.go new file mode 100644 index 000000000..c4fb09eea --- /dev/null +++ b/internal/git/commit/commit_test.go @@ -0,0 +1,233 @@ +package commit + +import ( + "bytes" + "fmt" + "strings" + "testing" + "time" + + "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/git/tree" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" + "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" +) + +func TestMain(m *testing.M) { + testhelper.Run(m) +} + +func TestWrite(t *testing.T) { + t.Helper() + + cfg := testcfg.Build(t) + ctx := testhelper.Context(t) + + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + SkipCreationViaService: true, + }) + repo := localrepo.NewTestRepo(t, cfg, repoProto) + + blobID, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("something")) + require.NoError(t, err) + changedBlobID, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("changed")) + require.NoError(t, err) + + treeEntryA := &tree.Entry{Path: "file", Mode: []byte("100644"), ObjectID: blobID, Type: tree.Blob} + treeA, err := tree.Write(ctx, repo, []*tree.Entry{treeEntryA}) + require.NoError(t, err) + + treeB, err := tree.Write(ctx, repo, []*tree.Entry{ + {Path: "file", Mode: []byte("100644"), ObjectID: changedBlobID, Type: tree.Blob}, + }) + require.NoError(t, err) + commitA, err := Write( + ctx, + repo, + CommitConfig{ + AuthorName: "Tazmanian Devil", + AuthorEmail: "taz@devils.org", + CommitterName: "Tazmanian Devil", + CommitterEmail: "taz@devils.org", + Message: "I ❤️ Tazmania", + TreeID: treeA, + }, + ) + commitB, err := Write( + ctx, + repo, + CommitConfig{ + AuthorName: "Daffy Duck", + AuthorEmail: "daffy@ducks.org", + CommitterName: "Daffy Duck", + CommitterEmail: "daffy@ducks.org", + Message: "Big beak", + TreeID: treeB, + }, + ) + + commitDate := time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC) + + for _, tc := range []struct { + desc string + cfg CommitConfig + expectedError error + expectedCommit string + expectedRevUpdate git.Revision + }{ + { + desc: "missing tree", + expectedError: ErrMissingTree, + }, + { + desc: "missing author", + expectedError: ErrMissingAuthor, + cfg: CommitConfig{ + TreeID: treeA, + }, + }, + { + desc: "missing committer", + expectedError: ErrMissingCommitter, + cfg: CommitConfig{ + TreeID: treeA, + AuthorName: "Scrooge Mcduck", + }, + }, + { + desc: "missing author email", + expectedError: ErrMissingAuthorEmail, + cfg: CommitConfig{ + TreeID: treeA, + AuthorName: "Scrooge Mcduck", + CommitterName: "Mickey Mouse", + }, + }, + { + desc: "missing committer email", + expectedError: ErrMissingCommitterEmail, + cfg: CommitConfig{ + TreeID: treeA, + AuthorName: "Scrooge Mcduck", + AuthorEmail: "chief@ducks.org", + CommitterName: "Mickey Mouse", + }, + }, + { + desc: "with commit message", + cfg: CommitConfig{ + TreeID: treeA, + AuthorName: "Scrooge Mcduck", + AuthorEmail: "chief@ducks.org", + CommitterName: "Mickey Mouse", + CommitterEmail: "mickey@mouse.org", + AuthorDate: commitDate, + CommitterDate: commitDate, + Message: "my custom message\n\ntrailer\n", + }, + expectedCommit: strings.Join([]string{ + "tree " + string(treeA), + fmt.Sprintf( + "author Scrooge Mcduck %d %s", + commitDate.Unix(), + commitDate.Format("-0700"), + ), + fmt.Sprintf( + "committer Mickey Mouse %d %s", + commitDate.Unix(), + commitDate.Format("-0700"), + ), + "", + "my custom message", + "", + "trailer", + }, "\n"), + }, + { + desc: "with multiple parents", + cfg: CommitConfig{ + TreeID: treeA, + Parents: []git.ObjectID{commitA, commitB}, + AuthorName: "Scrooge Mcduck", + AuthorEmail: "chief@ducks.org", + CommitterName: "Mickey Mouse", + CommitterEmail: "mickey@mouse.org", + AuthorDate: commitDate, + CommitterDate: commitDate, + Message: "my custom message", + }, + expectedCommit: strings.Join([]string{ + "tree " + treeA.String(), + "parent " + commitA.String(), + "parent " + commitB.String(), + fmt.Sprintf( + "author Scrooge Mcduck %d %s", + commitDate.Unix(), + commitDate.Format("-0700"), + ), + fmt.Sprintf( + "committer Mickey Mouse %d %s", + commitDate.Unix(), + commitDate.Format("-0700"), + ), + "", + "my custom message", + }, "\n"), + }, + { + desc: "with reference", + cfg: CommitConfig{ + TreeID: treeA, + Parents: []git.ObjectID{commitA, commitB}, + AuthorName: "Scrooge Mcduck", + AuthorEmail: "chief@ducks.org", + CommitterName: "Mickey Mouse", + CommitterEmail: "mickey@mouse.org", + AuthorDate: commitDate, + CommitterDate: commitDate, + Message: "my custom message", + Reference: "refs/heads/foo", + }, + expectedCommit: strings.Join([]string{ + "tree " + treeA.String(), + "parent " + commitA.String(), + "parent " + commitB.String(), + fmt.Sprintf( + "author Scrooge Mcduck %d %s", + commitDate.Unix(), + commitDate.Format("-0700"), + ), + fmt.Sprintf( + "committer Mickey Mouse %d %s", + commitDate.Unix(), + commitDate.Format("-0700"), + ), + "", + "my custom message", + }, "\n"), + expectedRevUpdate: "refs/heads/foo", + }, + } { + t.Run(tc.desc, func(t *testing.T) { + oid, err := Write(ctx, repo, tc.cfg) + require.Equal(t, tc.expectedError, err) + if err != nil { + return + } + + commit, err := repo.ReadObject(ctx, oid) + require.NoError(t, err) + + require.Equal(t, tc.expectedCommit, text.ChompBytes(commit)) + + if tc.expectedRevUpdate != "" { + updatedOID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tc.expectedRevUpdate.String()) + require.Equal(t, oid, git.ObjectID(text.ChompBytes(updatedOID))) + } + }) + } +} diff --git a/internal/git/tree/write_test.go b/internal/git/tree/write_test.go index a87ee17c2..7b8d32b68 100644 --- a/internal/git/tree/write_test.go +++ b/internal/git/tree/write_test.go @@ -8,14 +8,21 @@ 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/text" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" + "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" ) func TestWriteTree(t *testing.T) { - cfg, repo, repoPath := setupRepo(t) + cfg := testcfg.Build(t) ctx := testhelper.Context(t) + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + SkipCreationViaService: true, + }) + repo := localrepo.NewTestRepo(t, cfg, repoProto) + differentContentBlobID, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("different content")) require.NoError(t, err) @@ -147,7 +154,7 @@ func TestWriteTree(t *testing.T) { output := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "ls-tree", "-r", string(oid))) if len(output) > 0 { - var actualEntries []Entry + var actualEntries []*Entry for _, line := range bytes.Split([]byte(output), []byte("\n")) { // Format: SP SP TAB tabSplit := bytes.Split(line, []byte("\t")) @@ -173,7 +180,7 @@ func TestWriteTree(t *testing.T) { t.Errorf("unknowr type %s", spaceSplit[1]) } - actualEntries = append(actualEntries, Entry{ + actualEntries = append(actualEntries, &Entry{ ObjectID: objectID, Mode: spaceSplit[0], Path: path, diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go index 03c6be7c2..44117e7f3 100644 --- a/internal/gitaly/service/operations/submodules.go +++ b/internal/gitaly/service/operations/submodules.go @@ -1,18 +1,24 @@ package operations import ( + "bytes" "context" "errors" "fmt" + "path/filepath" "regexp" "strings" + "time" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/commit" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/tree" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -62,16 +68,106 @@ func validateUserUpdateSubmoduleRequest(req *gitalypb.UserUpdateSubmoduleRequest } func (s *Server) updateSubmodule(ctx context.Context, quarantineRepo *localrepo.Repo, req *gitalypb.UserUpdateSubmoduleRequest) (string, error) { - /* - commit, err := quarantineRepo.ReadCommit(ctx, req.GetBranch()) + var stdout bytes.Buffer + + revAndPath := fmt.Sprintf("%s:%s", string(req.GetBranch()), string(req.GetSubmodule())) + if err := quarantineRepo.ExecAndWait(ctx, + git.SubCmd{ + Name: "cat-file", + Flags: []git.Option{git.Flag{Name: "-t"}}, + Args: []string{revAndPath}, + }, + git.WithStdout(&stdout), + ); err != nil { + return "", err + } + + currentBranchCommit, err := quarantineRepo.ResolveRevision( + ctx, + git.Revision(string(req.GetBranch())), + ) + if err != nil { + return "", fmt.Errorf("getting branch commit: %w", err) + } + + if text.ChompBytes(stdout.Bytes()) != "commit" { + return "", errors.New("submodule is not a commit") + } + + objectID, err := quarantineRepo.ResolveRevision( + ctx, + git.Revision(fmt.Sprintf("%s:%s", string(req.GetBranch()), string(req.GetSubmodule()))), + ) + if err != nil { + return "", fmt.Errorf("reading %s: %w", revAndPath, err) + } + + // The first tree to modify + path := filepath.Dir(string(req.GetSubmodule())) + base := filepath.Base(string(req.GetSubmodule())) + replaceWith := git.ObjectID(req.GetCommitSha()) + + for path != "" { + entries, err := tree.ListEntries(ctx, quarantineRepo, git.Revision(objectID+"^{tree}"), &tree.ListEntriesConfig{ + RelativePath: path, + }) if err != nil { - return "", fmt.Errorf("reading branch %s: %w", req.GetBranch(), err) + return "", fmt.Errorf("error reading tree %s: %w", objectID, err) + } + + var newEntries []*tree.Entry + var newTreeID git.ObjectID + + for _, entry := range entries { + if entry.Path != base { + newEntries = append(newEntries, entry) + continue + } + + newEntries = append(newEntries, &tree.Entry{ + Mode: entry.Mode, + Type: entry.Type, + Path: entry.Path, + ObjectID: replaceWith, + }) + + newTreeID, err = tree.Write(ctx, quarantineRepo, newEntries) + if err != nil { + return "", err + } } - */ - /* get the blob of the submodule path of submdulepath/HEAD */ - /* write a new blob for the HEAD */ - /* write a new tree object for the submodule */ - /* write a new commit with the tree */ + + replaceWith = newTreeID + path = filepath.Dir(path) + base = filepath.Base(path) + } + + commitDate := time.Now() + newCommitID, err := commit.Write(ctx, quarantineRepo, commit.CommitConfig{ + Parents: []git.ObjectID{currentBranchCommit}, + AuthorDate: commitDate, + AuthorName: string(req.GetUser().GetName()), + AuthorEmail: string(req.GetUser().GetEmail()), + CommitterName: string(req.GetUser().GetName()), + CommitterEmail: string(req.GetUser().GetEmail()), + CommitterDate: commitDate, + Message: string(req.GetCommitMessage()), + TreeID: replaceWith, + }) + if err != nil { + return "", fmt.Errorf("creating commit %w", err) + } + + if err := quarantineRepo.UpdateRef( + ctx, + git.ReferenceName(string(req.GetBranch())), + newCommitID, + currentBranchCommit, + ); err != nil { + return "", fmt.Errorf("updating ref %w", err) + } + + // update reference of branch return "", nil } @@ -96,7 +192,6 @@ func (s *Server) updateSubmoduleWithGit2Go(ctx context.Context, quarantineRepo * Submodule: string(req.GetSubmodule()), Message: string(req.GetCommitMessage()), }) - if err != nil { return "", err } diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go index e572224b5..f465cad46 100644 --- a/internal/gitaly/service/operations/submodules_test.go +++ b/internal/gitaly/service/operations/submodules_test.go @@ -12,7 +12,7 @@ import ( "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/git/lstree" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/tree" "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" @@ -93,7 +93,7 @@ func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) { require.Equal(t, commitMessage, commit.Subject) entry := gittest.Exec(t, cfg, "-C", repoPath, "ls-tree", "-z", fmt.Sprintf("%s^{tree}:", response.BranchUpdate.CommitId), testCase.submodule) - parser := lstree.NewParser(bytes.NewReader(entry), git.ObjectHashSHA1) + parser := tree.NewParser(bytes.NewReader(entry), git.ObjectHashSHA1) parsedEntry, err := parser.NextEntry() require.NoError(t, err) require.Equal(t, testCase.submodule, parsedEntry.Path) -- cgit v1.2.3