diff options
-rw-r--r-- | cmd/gitaly-git2go/main.go | 1 | ||||
-rw-r--r-- | cmd/gitaly-git2go/merge.go | 2 | ||||
-rw-r--r-- | cmd/gitaly-git2go/merge_test.go | 12 | ||||
-rw-r--r-- | cmd/gitaly-git2go/revert.go | 78 | ||||
-rw-r--r-- | cmd/gitaly-git2go/revert_test.go | 206 | ||||
-rw-r--r-- | internal/git2go/merge_test.go | 1 | ||||
-rw-r--r-- | internal/git2go/revert.go | 100 | ||||
-rw-r--r-- | internal/git2go/revert_test.go | 160 |
8 files changed, 555 insertions, 5 deletions
diff --git a/cmd/gitaly-git2go/main.go b/cmd/gitaly-git2go/main.go index 2d2d1626a..c9a46a13f 100644 --- a/cmd/gitaly-git2go/main.go +++ b/cmd/gitaly-git2go/main.go @@ -16,6 +16,7 @@ type subcmd interface { var subcommands = map[string]subcmd{ "conflicts": &conflictsSubcommand{}, "merge": &mergeSubcommand{}, + "revert": &revertSubcommand{}, } const programName = "gitaly-git2go" diff --git a/cmd/gitaly-git2go/merge.go b/cmd/gitaly-git2go/merge.go index 97ea6f49a..3abf34e83 100644 --- a/cmd/gitaly-git2go/merge.go +++ b/cmd/gitaly-git2go/merge.go @@ -49,6 +49,7 @@ func (cmd *mergeSubcommand) Run() error { if err != nil { return fmt.Errorf("could not open repository: %w", err) } + defer repo.Free() ours, err := lookupCommit(repo, request.Ours) if err != nil { @@ -64,6 +65,7 @@ func (cmd *mergeSubcommand) Run() error { if err != nil { return fmt.Errorf("could not merge commits: %w", err) } + defer index.Free() if index.HasConflicts() { return errors.New("could not auto-merge due to conflicts") diff --git a/cmd/gitaly-git2go/merge_test.go b/cmd/gitaly-git2go/merge_test.go index c5ae2a5d9..c201560a0 100644 --- a/cmd/gitaly-git2go/merge_test.go +++ b/cmd/gitaly-git2go/merge_test.go @@ -82,9 +82,10 @@ func TestMergeFailsWithInvalidRepositoryPath(t *testing.T) { require.Contains(t, err.Error(), "merge: could not open repository") } -func buildCommit(t *testing.T, repoPath string, parent *git.Oid, fileContents map[string]string) *git.Oid { +func buildCommit(t testing.TB, repoPath string, parent *git.Oid, fileContents map[string]string) *git.Oid { repo, err := git.OpenRepository(repoPath) require.NoError(t, err) + defer repo.Free() odb, err := repo.Odb() require.NoError(t, err) @@ -230,16 +231,17 @@ func TestMergeTrees(t *testing.T) { }.Run(ctx, config.Config) if tc.expectedStderr != "" { - assert.Error(t, err) - assert.Contains(t, err.Error(), tc.expectedStderr) + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedStderr) return } - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectedResponse, response) repo, err := git.OpenRepository(repoPath) require.NoError(t, err) + defer repo.Free() commitOid, err := git.NewOid(response.CommitID) require.NoError(t, err) @@ -249,7 +251,7 @@ func TestMergeTrees(t *testing.T) { tree, err := commit.Tree() require.NoError(t, err) - require.Equal(t, uint64(len(tc.expected)), tree.EntryCount()) + require.EqualValues(t, len(tc.expected), tree.EntryCount()) for name, contents := range tc.expected { entry := tree.EntryByName(name) diff --git a/cmd/gitaly-git2go/revert.go b/cmd/gitaly-git2go/revert.go new file mode 100644 index 000000000..c1a4d4920 --- /dev/null +++ b/cmd/gitaly-git2go/revert.go @@ -0,0 +1,78 @@ +// +build static,system_libgit2 + +package main + +import ( + "errors" + "flag" + "fmt" + "os" + + git "github.com/libgit2/git2go/v30" + "gitlab.com/gitlab-org/gitaly/internal/git2go" +) + +type revertSubcommand struct { + request string +} + +func (cmd *revertSubcommand) Flags() *flag.FlagSet { + flags := flag.NewFlagSet("revert", flag.ExitOnError) + flags.StringVar(&cmd.request, "request", "", "git2go.RevertCommand") + return flags +} + +func (cmd *revertSubcommand) Run() error { + request, err := git2go.RevertCommandFromSerialized(cmd.request) + if err != nil { + return err + } + + repo, err := git.OpenRepository(request.Repository) + if err != nil { + return fmt.Errorf("open repository: %w", err) + } + defer repo.Free() + + ours, err := lookupCommit(repo, request.Ours) + if err != nil { + return fmt.Errorf("ours commit lookup: %w", err) + } + + revert, err := lookupCommit(repo, request.Revert) + if err != nil { + return fmt.Errorf("revert commit lookup: %w", err) + } + + index, err := repo.RevertCommit(revert, ours, request.Mainline, nil) + if err != nil { + return fmt.Errorf("revert: %w", err) + } + defer index.Free() + + if index.HasConflicts() { + return errors.New("could not revert due to conflicts") + } + + tree, err := index.WriteTreeTo(repo) + if err != nil { + return fmt.Errorf("write tree: %w", err) + } + + committer := git.Signature{ + Name: sanitizeSignatureInfo(request.AuthorName), + Email: sanitizeSignatureInfo(request.AuthorMail), + When: request.AuthorDate, + } + + commit, err := repo.CreateCommitFromIds("", &committer, &committer, request.Message, tree, ours.Id()) + if err != nil { + return fmt.Errorf("create revert commit: %w", err) + } + + response := git2go.RevertResult{ + CommitID: commit.String(), + } + + return response.SerializeTo(os.Stdout) +} diff --git a/cmd/gitaly-git2go/revert_test.go b/cmd/gitaly-git2go/revert_test.go new file mode 100644 index 000000000..d624f5784 --- /dev/null +++ b/cmd/gitaly-git2go/revert_test.go @@ -0,0 +1,206 @@ +// +build static,system_libgit2 + +package main + +import ( + "testing" + "time" + + git "github.com/libgit2/git2go/v30" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/git2go" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/internal/testhelper" +) + +func TestRevert_validation(t *testing.T) { + _, repoPath, cleanup := testhelper.NewTestRepo(t) + defer cleanup() + + testcases := []struct { + desc string + request git2go.RevertCommand + expectedErr string + }{ + { + desc: "no arguments", + expectedErr: "revert: invalid parameters: missing repository", + }, + { + desc: "missing repository", + request: git2go.RevertCommand{AuthorName: "Foo", AuthorMail: "foo@example.com", Message: "Foo", Ours: "HEAD", Revert: "HEAD"}, + expectedErr: "revert: invalid parameters: missing repository", + }, + { + desc: "missing author name", + request: git2go.RevertCommand{Repository: repoPath, AuthorMail: "foo@example.com", Message: "Foo", Ours: "HEAD", Revert: "HEAD"}, + expectedErr: "revert: invalid parameters: missing author name", + }, + { + desc: "missing author mail", + request: git2go.RevertCommand{Repository: repoPath, AuthorName: "Foo", Message: "Foo", Ours: "HEAD", Revert: "HEAD"}, + expectedErr: "revert: invalid parameters: missing author mail", + }, + { + desc: "missing message", + request: git2go.RevertCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", Ours: "HEAD", Revert: "HEAD"}, + expectedErr: "revert: invalid parameters: missing message", + }, + { + desc: "missing ours", + request: git2go.RevertCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", Message: "Foo", Revert: "HEAD"}, + expectedErr: "revert: invalid parameters: missing ours", + }, + { + desc: "missing revert", + request: git2go.RevertCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", Message: "Foo", Ours: "HEAD"}, + expectedErr: "revert: invalid parameters: missing revert", + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() + + _, err := tc.request.Run(ctx, config.Config) + require.Error(t, err) + require.Equal(t, tc.expectedErr, err.Error()) + }) + } +} + +func TestRevert_trees(t *testing.T) { + testcases := []struct { + desc string + setupRepo func(t testing.TB, repoPath string) (ours, revert string) + expected map[string]string + expectedResponse git2go.RevertResult + expectedStderr string + }{ + { + desc: "trivial revert succeeds", + setupRepo: func(t testing.TB, repoPath string) (ours, revert string) { + baseOid := buildCommit(t, repoPath, nil, map[string]string{ + "a": "apple", + "b": "banana", + }) + revertOid := buildCommit(t, repoPath, baseOid, map[string]string{ + "a": "apple", + "b": "pineapple", + }) + oursOid := buildCommit(t, repoPath, revertOid, map[string]string{ + "a": "apple", + "b": "pineapple", + "c": "carrot", + }) + + return oursOid.String(), revertOid.String() + }, + expected: map[string]string{ + "a": "apple", + "b": "banana", + "c": "carrot", + }, + expectedResponse: git2go.RevertResult{ + CommitID: "0be709b57f18ad3f592a8cb7c57f920861392573", + }, + }, + { + desc: "conflicting revert fails", + setupRepo: func(t testing.TB, repoPath string) (ours, revert string) { + baseOid := buildCommit(t, repoPath, nil, map[string]string{ + "a": "apple", + }) + revertOid := buildCommit(t, repoPath, baseOid, map[string]string{ + "a": "pineapple", + }) + oursOid := buildCommit(t, repoPath, revertOid, map[string]string{ + "a": "carrot", + }) + + return oursOid.String(), revertOid.String() + }, + expectedStderr: "revert: could not revert due to conflicts\n", + }, + { + desc: "nonexistent ours fails", + setupRepo: func(t testing.TB, repoPath string) (ours, revert string) { + revertOid := buildCommit(t, repoPath, nil, map[string]string{ + "a": "apple", + }) + + return "nonexistent", revertOid.String() + }, + expectedStderr: "revert: ours commit lookup: could not lookup reference: revspec 'nonexistent' not found\n", + }, + { + desc: "nonexistent revert fails", + setupRepo: func(t testing.TB, repoPath string) (ours, revert string) { + oursOid := buildCommit(t, repoPath, nil, map[string]string{ + "a": "apple", + }) + + return oursOid.String(), "nonexistent" + }, + expectedStderr: "revert: revert commit lookup: could not lookup reference: revspec 'nonexistent' not found\n", + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + _, repoPath, cleanup := testhelper.NewTestRepo(t) + defer cleanup() + + ours, revert := tc.setupRepo(t, repoPath) + + ctx, cancel := testhelper.Context() + defer cancel() + + authorDate := time.Date(2020, 7, 30, 7, 45, 50, 0, time.FixedZone("UTC+2", +2*60*60)) + + request := git2go.RevertCommand{ + Repository: repoPath, + AuthorName: "Foo", + AuthorMail: "foo@example.com", + AuthorDate: authorDate, + Message: "Foo", + Ours: ours, + Revert: revert, + } + + response, err := request.Run(ctx, config.Config) + + if tc.expectedStderr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedStderr) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.expectedResponse, response) + + repo, err := git.OpenRepository(repoPath) + require.NoError(t, err) + defer repo.Free() + + commitOid, err := git.NewOid(response.CommitID) + require.NoError(t, err) + + commit, err := repo.LookupCommit(commitOid) + require.NoError(t, err) + + tree, err := commit.Tree() + require.NoError(t, err) + require.EqualValues(t, len(tc.expected), tree.EntryCount()) + + for name, contents := range tc.expected { + entry := tree.EntryByName(name) + require.NotNil(t, entry) + + blob, err := repo.LookupBlob(entry.Id) + require.NoError(t, err) + require.Equal(t, []byte(contents), blob.Contents()) + } + }) + } +} diff --git a/internal/git2go/merge_test.go b/internal/git2go/merge_test.go index 5f0bc60c4..227475471 100644 --- a/internal/git2go/merge_test.go +++ b/internal/git2go/merge_test.go @@ -109,6 +109,7 @@ func TestGit2Go_MergeCommandSerialization(t *testing.T) { func TestGit2Go_MergeResultSerialization(t *testing.T) { serializeResult := func(t *testing.T, result MergeResult) string { + t.Helper() var buf bytes.Buffer err := result.SerializeTo(&buf) require.NoError(t, err) diff --git a/internal/git2go/revert.go b/internal/git2go/revert.go new file mode 100644 index 000000000..ca11169bf --- /dev/null +++ b/internal/git2go/revert.go @@ -0,0 +1,100 @@ +package git2go + +import ( + "context" + "errors" + "fmt" + "io" + "time" + + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" +) + +// RevertResult contains results from a revert. +type RevertResult struct { + // CommitID is the object ID of the generated revert commit. + CommitID string `json:"commit_id"` +} + +// SerializeTo serializes the revert result and writes it into the writer. +func (r RevertResult) SerializeTo(w io.Writer) error { + return serializeTo(w, r) +} + +type RevertCommand struct { + // Repository is the path to execute the revert in. + Repository string `json:"repository"` + // AuthorName is the author name of revert commit. + AuthorName string `json:"author_name"` + // AuthorMail is the author mail of revert commit. + AuthorMail string `json:"author_mail"` + // AuthorDate is the author date of revert commit. + AuthorDate time.Time `json:"author_date"` + // Message is the message to be used for the revert commit. + Message string `json:"message"` + // Ours is the commit that the revert is applied to. + Ours string `json:"ours"` + // Revert is the commit to be reverted. + Revert string `json:"revert"` + // Mainline is the parent to be considered the mainline + Mainline uint `json:"mainline"` +} + +func (r RevertCommand) Run(ctx context.Context, cfg config.Cfg) (RevertResult, error) { + if err := r.verify(); err != nil { + return RevertResult{}, fmt.Errorf("revert: %w: %s", ErrInvalidArgument, err.Error()) + } + + serialized, err := serialize(r) + if err != nil { + return RevertResult{}, err + } + + stdout, err := run(ctx, cfg, "revert", serialized) + if err != nil { + return RevertResult{}, err + } + + var response RevertResult + if err := deserialize(stdout, &response); err != nil { + return RevertResult{}, err + } + + return response, nil +} + +func (r RevertCommand) verify() error { + if r.Repository == "" { + return errors.New("missing repository") + } + if r.AuthorName == "" { + return errors.New("missing author name") + } + if r.AuthorMail == "" { + return errors.New("missing author mail") + } + if r.Message == "" { + return errors.New("missing message") + } + if r.Ours == "" { + return errors.New("missing ours") + } + if r.Revert == "" { + return errors.New("missing revert") + } + return nil +} + +// RevertCommandFromSerialized deserializes the revert request from its JSON representation encoded with base64. +func RevertCommandFromSerialized(serialized string) (RevertCommand, error) { + var request RevertCommand + if err := deserialize(serialized, &request); err != nil { + return RevertCommand{}, err + } + + if err := request.verify(); err != nil { + return RevertCommand{}, fmt.Errorf("revert: %w: %s", ErrInvalidArgument, err.Error()) + } + + return request, nil +} diff --git a/internal/git2go/revert_test.go b/internal/git2go/revert_test.go new file mode 100644 index 000000000..c56e4da4e --- /dev/null +++ b/internal/git2go/revert_test.go @@ -0,0 +1,160 @@ +package git2go + +import ( + "bytes" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestGit2Go_RevertCommandSerialization(t *testing.T) { + testcases := []struct { + desc string + cmd RevertCommand + err string + }{ + { + desc: "missing repository", + cmd: RevertCommand{}, + err: "missing repository", + }, + { + desc: "missing author name", + cmd: RevertCommand{ + Repository: "foo", + }, + err: "missing author name", + }, + { + desc: "missing author mail", + cmd: RevertCommand{ + Repository: "foo", + AuthorName: "Au Thor", + }, + err: "missing author mail", + }, + { + desc: "missing author message", + cmd: RevertCommand{ + Repository: "foo", + AuthorName: "Au Thor", + AuthorMail: "au@thor.com", + }, + err: "missing message", + }, + { + desc: "missing author ours", + cmd: RevertCommand{ + Repository: "foo", + AuthorName: "Au Thor", + AuthorMail: "au@thor.com", + Message: "Message", + }, + err: "missing ours", + }, + { + desc: "missing revert", + cmd: RevertCommand{ + Repository: "foo", + AuthorName: "Au Thor", + AuthorMail: "au@thor.com", + Message: "Message", + Ours: "refs/heads/master", + }, + err: "missing revert", + }, + { + desc: "valid command", + cmd: RevertCommand{ + Repository: "foo", + AuthorName: "Au Thor", + AuthorMail: "au@thor.com", + Message: "Message", + Ours: "refs/heads/master", + Revert: "refs/heads/master", + }, + }, + { + desc: "valid command with date", + cmd: RevertCommand{ + Repository: "foo", + AuthorName: "Au Thor", + AuthorMail: "au@thor.com", + AuthorDate: time.Now().UTC(), + Message: "Message", + Ours: "refs/heads/master", + Revert: "refs/heads/master", + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + serialized, err := serialize(tc.cmd) + require.NoError(t, err) + + deserialized, err := RevertCommandFromSerialized(serialized) + + if tc.err != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.err) + } else { + require.NoError(t, err) + require.Equal(t, tc.cmd, deserialized) + } + }) + } +} + +func TestGit2Go_RevertResultSerialization(t *testing.T) { + serializeResult := func(t *testing.T, result RevertResult) string { + t.Helper() + var buf bytes.Buffer + err := result.SerializeTo(&buf) + require.NoError(t, err) + return buf.String() + } + + testcases := []struct { + desc string + serialized string + expected RevertResult + err string + }{ + { + desc: "empty revert result", + serialized: serializeResult(t, RevertResult{}), + expected: RevertResult{}, + }, + { + desc: "revert result with commit", + serialized: serializeResult(t, RevertResult{ + CommitID: "1234", + }), + expected: RevertResult{ + CommitID: "1234", + }, + }, + { + desc: "invalid serialized representation", + serialized: "xvlc", + err: "invalid character", + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + var deserialized RevertResult + err := deserialize(tc.serialized, &deserialized) + + if tc.err != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.err) + } else { + require.NoError(t, err) + require.Equal(t, tc.expected, deserialized) + } + }) + } +} |