diff options
-rw-r--r-- | cmd/gitaly-git2go/apply.go | 224 | ||||
-rw-r--r-- | cmd/gitaly-git2go/commit/commit.go | 5 | ||||
-rw-r--r-- | cmd/gitaly-git2go/main.go | 1 | ||||
-rw-r--r-- | go.mod | 2 | ||||
-rw-r--r-- | go.sum | 5 | ||||
-rw-r--r-- | internal/git2go/apply.go | 112 | ||||
-rw-r--r-- | internal/git2go/apply_test.go | 217 | ||||
-rw-r--r-- | internal/git2go/commit.go | 2 | ||||
-rw-r--r-- | internal/git2go/commit_test.go | 7 | ||||
-rw-r--r-- | internal/git2go/executor.go | 10 | ||||
-rw-r--r-- | internal/gitaly/service/operations/commit_files.go | 5 | ||||
-rw-r--r-- | internal/gitaly/service/operations/server.go | 3 |
12 files changed, 581 insertions, 12 deletions
diff --git a/cmd/gitaly-git2go/apply.go b/cmd/gitaly-git2go/apply.go new file mode 100644 index 000000000..c4c6cd1d1 --- /dev/null +++ b/cmd/gitaly-git2go/apply.go @@ -0,0 +1,224 @@ +// +build static,system_libgit2 + +package main + +import ( + "bytes" + "context" + "encoding/gob" + "errors" + "flag" + "fmt" + "io" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + + git "github.com/libgit2/git2go/v30" + "gitlab.com/gitlab-org/gitaly/internal/git2go" +) + +type patchIterator struct { + value git2go.Patch + decoder *gob.Decoder + error error +} + +func (iter *patchIterator) Next() bool { + if err := iter.decoder.Decode(&iter.value); err != nil { + if !errors.Is(err, io.EOF) { + iter.error = fmt.Errorf("decode patch: %w", err) + } + + return false + } + + return true +} + +func (iter *patchIterator) Value() git2go.Patch { return iter.value } + +func (iter *patchIterator) Err() error { return iter.error } + +type applySubcommand struct { + gitBinaryPath string +} + +func (cmd *applySubcommand) Flags() *flag.FlagSet { + fs := flag.NewFlagSet("apply", flag.ExitOnError) + fs.StringVar(&cmd.gitBinaryPath, "git-binary-path", "", "Path to the Git binary.") + return fs +} + +// Run runs the subcommand. +func (cmd *applySubcommand) Run(ctx context.Context, stdin io.Reader, stdout io.Writer) error { + decoder := gob.NewDecoder(stdin) + + var params git2go.ApplyParams + if err := decoder.Decode(¶ms); err != nil { + return fmt.Errorf("decode params: %w", err) + } + + params.Patches = &patchIterator{decoder: decoder} + commitID, err := cmd.apply(ctx, params) + return gob.NewEncoder(stdout).Encode(git2go.Result{ + CommitID: commitID, + Error: git2go.SerializableError(err), + }) +} + +func (cmd *applySubcommand) apply(ctx context.Context, params git2go.ApplyParams) (string, error) { + repo, err := git.OpenRepository(params.Repository) + if err != nil { + return "", fmt.Errorf("open repository: %w", err) + } + + commitOID, err := git.NewOid(params.ParentCommit) + if err != nil { + return "", fmt.Errorf("parse parent commit oid: %w", err) + } + + committer := git.Signature(params.Committer) + for i := 0; params.Patches.Next(); i++ { + commitOID, err = cmd.applyPatch(ctx, repo, &committer, commitOID, params.Patches.Value()) + if err != nil { + return "", fmt.Errorf("apply patch [%d]: %w", i+1, err) + } + } + + if err := params.Patches.Err(); err != nil { + return "", fmt.Errorf("reading patches: %w", err) + } + + return commitOID.String(), nil +} + +func (cmd *applySubcommand) applyPatch( + ctx context.Context, + repo *git.Repository, + committer *git.Signature, + parentCommitOID *git.Oid, + patch git2go.Patch, +) (*git.Oid, error) { + parentCommit, err := repo.LookupCommit(parentCommitOID) + if err != nil { + return nil, fmt.Errorf("lookup commit: %w", err) + } + + parentTree, err := parentCommit.Tree() + if err != nil { + return nil, fmt.Errorf("lookup tree: %w", err) + } + + diff, err := git.DiffFromBuffer(patch.Diff, repo) + if err != nil { + return nil, fmt.Errorf("diff from buffer: %w", err) + } + + patchedIndex, err := repo.ApplyToTree(diff, parentTree, nil) + if err != nil { + if !git.IsErrorCode(err, git.ErrApplyFail) { + return nil, fmt.Errorf("apply to tree: %w", err) + } + + patchedIndex, err = cmd.threeWayMerge(ctx, repo, parentTree, diff, patch.Diff) + if err != nil { + return nil, fmt.Errorf("three way merge: %w", err) + } + } + + patchedTree, err := patchedIndex.WriteTreeTo(repo) + if err != nil { + return nil, fmt.Errorf("write patched tree: %w", err) + } + + author := git.Signature(patch.Author) + patchedCommitOID, err := repo.CreateCommitFromIds("", &author, committer, patch.Message, patchedTree, parentCommitOID) + if err != nil { + return nil, fmt.Errorf("create commit: %w", err) + } + + return patchedCommitOID, nil +} + +// threeWayMerge attempts a three-way merge as a fallback if applying the patch fails. +// Fallback three-way merge is only possible if the patch records the pre-image blobs +// and the repository contains them. It works as follows: +// +// 1. An index that contains only the pre-image blobs of the patch is built. This is done +// by calling `git apply --build-fake-ancestor`. The tree of the index is the fake +// ancestor tree. +// 2. The fake ancestor tree is patched to produce the post-image tree of the patch. +// 3. Three-way merge is performed with fake ancestor tree as the common ancestor, the +// base commit's tree as our tree and the patched fake ancestor tree as their tree. +func (cmd *applySubcommand) threeWayMerge( + ctx context.Context, + repo *git.Repository, + our *git.Tree, + diff *git.Diff, + rawDiff []byte, +) (*git.Index, error) { + ancestorTree, err := cmd.buildFakeAncestor(ctx, repo, rawDiff) + if err != nil { + return nil, fmt.Errorf("build fake ancestor: %w", err) + } + + patchedAncestorIndex, err := repo.ApplyToTree(diff, ancestorTree, nil) + if err != nil { + return nil, fmt.Errorf("patch fake ancestor: %w", err) + } + + patchedAncestorTreeOID, err := patchedAncestorIndex.WriteTreeTo(repo) + if err != nil { + return nil, fmt.Errorf("write patched fake ancestor: %w", err) + } + + patchedTree, err := repo.LookupTree(patchedAncestorTreeOID) + if err != nil { + return nil, fmt.Errorf("lookup patched tree: %w", err) + } + + patchedIndex, err := repo.MergeTrees(ancestorTree, our, patchedTree, nil) + if err != nil { + return nil, fmt.Errorf("merge trees: %w", err) + } + + if patchedIndex.HasConflicts() { + return nil, git2go.ErrMergeConflict + } + + return patchedIndex, nil +} + +func (cmd *applySubcommand) buildFakeAncestor(ctx context.Context, repo *git.Repository, diff []byte) (*git.Tree, error) { + dir, err := ioutil.TempDir("", "gitaly-git2go") + if err != nil { + return nil, fmt.Errorf("create temporary directory: %w", err) + } + defer os.RemoveAll(dir) + + file := filepath.Join(dir, "patch-merge-index") + gitCmd := exec.CommandContext(ctx, cmd.gitBinaryPath, "--git-dir", repo.Path(), "apply", "--build-fake-ancestor", file) + gitCmd.Stdin = bytes.NewReader(diff) + if _, err := gitCmd.Output(); err != nil { + var exitError *exec.ExitError + if errors.As(err, &exitError) { + err = fmt.Errorf("%w, stderr: %q", err, exitError.Stderr) + } + + return nil, fmt.Errorf("git: %w", err) + } + + fakeAncestor, err := git.OpenIndex(file) + if err != nil { + return nil, fmt.Errorf("open fake ancestor index: %w", err) + } + + ancestorTreeOID, err := fakeAncestor.WriteTreeTo(repo) + if err != nil { + return nil, fmt.Errorf("write fake ancestor tree: %w", err) + } + + return repo.LookupTree(ancestorTreeOID) +} diff --git a/cmd/gitaly-git2go/commit/commit.go b/cmd/gitaly-git2go/commit/commit.go index 90862e1a7..c8051c7a2 100644 --- a/cmd/gitaly-git2go/commit/commit.go +++ b/cmd/gitaly-git2go/commit/commit.go @@ -73,8 +73,9 @@ func commit(ctx context.Context, params git2go.CommitParams) (string, error) { return "", fmt.Errorf("write tree: %w", err) } - signature := git.Signature(params.Author) - commitID, err := repo.CreateCommitFromIds("", &signature, &signature, params.Message, treeOID, parents...) + author := git.Signature(params.Author) + committer := git.Signature(params.Committer) + commitID, err := repo.CreateCommitFromIds("", &author, &committer, params.Message, treeOID, parents...) if err != nil { if git.IsErrorClass(err, git.ErrClassInvalid) { return "", git2go.InvalidArgumentError(err.Error()) diff --git a/cmd/gitaly-git2go/main.go b/cmd/gitaly-git2go/main.go index 04eab4dff..82dad85a9 100644 --- a/cmd/gitaly-git2go/main.go +++ b/cmd/gitaly-git2go/main.go @@ -18,6 +18,7 @@ type subcmd interface { } var subcommands = map[string]subcmd{ + "apply": &applySubcommand{}, "commit": commitSubcommand{}, "conflicts": &conflicts.Subcommand{}, "merge": &mergeSubcommand{}, @@ -14,7 +14,7 @@ require ( github.com/hashicorp/golang-lru v0.5.4 github.com/kelseyhightower/envconfig v1.3.0 github.com/lib/pq v1.2.0 - github.com/libgit2/git2go/v30 v30.0.5 + github.com/libgit2/git2go/v30 v30.0.18 github.com/mattn/go-isatty v0.0.12 // indirect github.com/olekukonko/tablewriter v0.0.2 github.com/opentracing/opentracing-go v1.2.0 @@ -237,8 +237,8 @@ github.com/lib/pq v1.2.0 h1:LXpIM/LZ5xGFhOpXAQUIMM1HdyqzVYM13zNdjCEEcA0= github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/libgit2/git2go v0.0.0-20190104134018-ecaeb7a21d47 h1:HDt7WT3kpXSHq4mlOuLzgXH9LeOK1qlhyFdKIAzxxeM= github.com/libgit2/git2go v0.0.0-20190104134018-ecaeb7a21d47/go.mod h1:4bKN42efkbNYMZlvDfxGDxzl066GhpvIircZDsm8Y+Y= -github.com/libgit2/git2go/v30 v30.0.5 h1:gxKqXOslpvYDZNC62f8GV34TAk0qw4wZ++IdYw8V9I4= -github.com/libgit2/git2go/v30 v30.0.5/go.mod h1:YReiQ7xhMoyAL4ISYFLZt+OGqn6xtLqvTC1xJ9oAH7Y= +github.com/libgit2/git2go/v30 v30.0.18 h1:lsTvvjpLelHZQ3XXsmyFJKMd9HazBIJdValbdIjlBdA= +github.com/libgit2/git2go/v30 v30.0.18/go.mod h1:bEqWPWaJjDpnkerA2FlyUdsuhc5/3UPBjYJ6SV0X3gY= github.com/lightstep/lightstep-tracer-go v0.15.6 h1:D0GGa7afJ7GcQvu5as6ssLEEKYXvRgKI5d5cevtz8r4= github.com/lightstep/lightstep-tracer-go v0.15.6/go.mod h1:6AMpwZpsyCFwSovxzM78e+AsYxE8sGwiM6C3TytaWeI= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= @@ -455,6 +455,7 @@ golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a h1:vclmkQCjlDX5OydZ9wv8rBCcS0QyQY66Mpf/7BZbInM= golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= diff --git a/internal/git2go/apply.go b/internal/git2go/apply.go new file mode 100644 index 000000000..447e92530 --- /dev/null +++ b/internal/git2go/apply.go @@ -0,0 +1,112 @@ +package git2go + +import ( + "context" + "encoding/gob" + "fmt" + "io" +) + +// ErrMergeConflict is returned when there is a merge conflict. +var ErrMergeConflict = wrapError{Message: "merge conflict"} + +// Patch represents a single patch. +type Patch struct { + // Author is the author of the patch. + Author Signature + // Message is used as the commit message when applying the patch. + Message string + // Diff contains the diff of the patch. + Diff []byte +} + +// ApplyParams are the parameters for Apply. +type ApplyParams struct { + // Repository is the path to the repository. + Repository string + // Committer is the committer applying the patch. + Committer Signature + // ParentCommit is the OID of the commit to apply the patches against. + ParentCommit string + // Patches iterates over all the patches to be applied. + Patches PatchIterator +} + +// PatchIterator iterates over a stream of patches. +type PatchIterator interface { + // Next returns whether there is a next patch. + Next() bool + // Value returns the patch being currently iterated upon. + Value() Patch + // Err returns the iteration error. Err should + // be always checked after Next returns false. + Err() error +} + +type slicePatchIterator struct { + value Patch + patches []Patch +} + +// NewSlicePatchIterator returns a PatchIterator that iterates over the slice +// of patches. +func NewSlicePatchIterator(patches []Patch) PatchIterator { + return &slicePatchIterator{patches: patches} +} + +func (iter *slicePatchIterator) Next() bool { + if len(iter.patches) == 0 { + return false + } + + iter.value = iter.patches[0] + iter.patches = iter.patches[1:] + return true +} + +func (iter *slicePatchIterator) Value() Patch { return iter.value } + +func (iter *slicePatchIterator) Err() error { return nil } + +// Apply applies the provided patches and returns the OID of the commit with the patches +// applied. +func (b Executor) Apply(ctx context.Context, params ApplyParams) (string, error) { + reader, writer := io.Pipe() + defer writer.Close() + + go func() { + writer.CloseWithError(func() error { + patches := params.Patches + params.Patches = nil + + encoder := gob.NewEncoder(writer) + if err := encoder.Encode(params); err != nil { + return fmt.Errorf("encode header: %w", err) + } + + for patches.Next() { + if err := encoder.Encode(patches.Value()); err != nil { + return fmt.Errorf("encode patch: %w", err) + } + } + + if err := patches.Err(); err != nil { + return fmt.Errorf("patch iterator: %w", err) + } + + return nil + }()) + }() + + var result Result + output, err := run(ctx, b.binaryPath, reader, "apply", "-git-binary-path", b.gitBinaryPath) + if err != nil { + return "", fmt.Errorf("run: %w", err) + } + + if err := gob.NewDecoder(output).Decode(&result); err != nil { + return "", fmt.Errorf("decode: %w", err) + } + + return result.CommitID, result.Error +} diff --git a/internal/git2go/apply_test.go b/internal/git2go/apply_test.go new file mode 100644 index 000000000..1744432b6 --- /dev/null +++ b/internal/git2go/apply_test.go @@ -0,0 +1,217 @@ +package git2go + +import ( + "errors" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/internal/testhelper" +) + +func TestExecutor_Apply(t *testing.T) { + pbRepo, repoPath, clean := testhelper.InitBareRepo(t) + defer clean() + + repo := git.NewRepository(pbRepo) + executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go"), command.GitPath()) + + ctx, cancel := testhelper.Context() + defer cancel() + + oidBase, err := repo.WriteBlob(ctx, "file", strings.NewReader("base")) + require.NoError(t, err) + + oidA, err := repo.WriteBlob(ctx, "file", strings.NewReader("a")) + require.NoError(t, err) + + oidB, err := repo.WriteBlob(ctx, "file", strings.NewReader("b")) + require.NoError(t, err) + + author := NewSignature("Test Author", "test.author@example.com", time.Now()) + committer := NewSignature("Test Committer", "test.committer@example.com", time.Now()) + + parentCommitSHA, err := executor.Commit(ctx, CommitParams{ + Repository: repoPath, + Author: author, + Committer: committer, + Message: "base commit", + Actions: []Action{CreateFile{Path: "file", OID: oidBase}}, + }) + require.NoError(t, err) + + noCommonAncestor, err := executor.Commit(ctx, CommitParams{ + Repository: repoPath, + Author: author, + Committer: committer, + Message: "commit with ab", + Actions: []Action{CreateFile{Path: "file", OID: oidA}}, + }) + require.NoError(t, err) + + updateToA, err := executor.Commit(ctx, CommitParams{ + Repository: repoPath, + Author: author, + Committer: committer, + Message: "commit with a", + Parent: parentCommitSHA, + Actions: []Action{UpdateFile{Path: "file", OID: oidA}}, + }) + require.NoError(t, err) + + updateToB, err := executor.Commit(ctx, CommitParams{ + Repository: repoPath, + Author: author, + Committer: committer, + Message: "commit to b", + Parent: parentCommitSHA, + Actions: []Action{UpdateFile{Path: "file", OID: oidB}}, + }) + require.NoError(t, err) + + updateFromAToB, err := executor.Commit(ctx, CommitParams{ + Repository: repoPath, + Author: author, + Committer: committer, + Message: "commit a -> b", + Parent: updateToA, + Actions: []Action{UpdateFile{Path: "file", OID: oidB}}, + }) + require.NoError(t, err) + + otherFile, err := executor.Commit(ctx, CommitParams{ + Repository: repoPath, + Author: author, + Committer: committer, + Message: "commit with other-file", + Actions: []Action{CreateFile{Path: "other-file", OID: oidA}}, + }) + require.NoError(t, err) + + diffBetween := func(t testing.TB, fromCommit, toCommit string) []byte { + t.Helper() + return testhelper.MustRunCommand(t, nil, + "git", "-C", repoPath, "format-patch", "--stdout", fromCommit+".."+toCommit) + } + + for _, tc := range []struct { + desc string + patches []Patch + parentCommit string + tree []testhelper.TreeEntry + error error + }{ + { + desc: "patch applies cleanly", + patches: []Patch{ + { + Author: author, + Message: "test commit message", + Diff: diffBetween(t, parentCommitSHA, updateToA), + }, + }, + parentCommit: parentCommitSHA, + tree: []testhelper.TreeEntry{ + {Path: "file", Mode: "100644", Content: "a"}, + }, + }, + { + desc: "multiple patches apply cleanly", + patches: []Patch{ + { + Author: author, + Message: "commit with a", + Diff: diffBetween(t, parentCommitSHA, updateToA), + }, + { + Author: author, + Message: "commit with a -> b", + Diff: diffBetween(t, updateToA, updateFromAToB), + }, + }, + parentCommit: updateToA, + tree: []testhelper.TreeEntry{ + {Path: "file", Mode: "100644", Content: "b"}, + }, + }, + { + desc: "three way merged", + patches: []Patch{ + { + Author: author, + Message: "three-way merged files", + Diff: diffBetween(t, parentCommitSHA, otherFile), + }, + }, + parentCommit: parentCommitSHA, + tree: []testhelper.TreeEntry{ + {Path: "file", Mode: "100644", Content: "base"}, + {Path: "other-file", Mode: "100644", Content: "a"}, + }, + }, + { + // This test asserts incorrect behavior due to a bug in libgit2's + // git_apply_to_tree function. The correct behavior would be to + // return an error about merge conflict but we currently concatenate + // the blobs of the two trees together. This test will fail once the + // issue is fixed. + // + // See: https://gitlab.com/gitlab-org/gitaly/-/issues/3325 + desc: "no common ancestor", + patches: []Patch{ + { + Author: author, + Message: "three-way merged file", + Diff: diffBetween(t, parentCommitSHA, noCommonAncestor), + }, + }, + parentCommit: parentCommitSHA, + // error: ErrMergeConflict, <- correct output + tree: []testhelper.TreeEntry{ + {Path: "file", Mode: "100644", Content: "abase"}, + }, + }, + { + desc: "merge conflict", + patches: []Patch{ + { + Author: author, + Message: "applies cleanly", + Diff: diffBetween(t, parentCommitSHA, updateToA), + }, + { + Author: author, + Message: "conflicts", + Diff: diffBetween(t, parentCommitSHA, updateToB), + }, + }, + error: ErrMergeConflict, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + commitID, err := executor.Apply(ctx, ApplyParams{ + Repository: repoPath, + Committer: committer, + ParentCommit: parentCommitSHA, + Patches: NewSlicePatchIterator(tc.patches), + }) + if tc.error != nil { + require.True(t, errors.Is(err, tc.error), err) + return + } + + require.Equal(t, commit{ + Parent: tc.parentCommit, + Author: author, + Committer: committer, + Message: tc.patches[len(tc.patches)-1].Message, + }, getCommit(t, ctx, repo, commitID)) + testhelper.RequireTree(t, repoPath, commitID, tc.tree) + }) + } +} diff --git a/internal/git2go/commit.go b/internal/git2go/commit.go index ee24862d9..2a9478609 100644 --- a/internal/git2go/commit.go +++ b/internal/git2go/commit.go @@ -39,6 +39,8 @@ type CommitParams struct { Repository string // Author is the author of the commit. Author Signature + // Committer is the committer of the commit. + Committer Signature // Message is message of the commit. Message string // Parent is the OID of the commit to use as the parent of this commit. diff --git a/internal/git2go/commit_test.go b/internal/git2go/commit_test.go index 218c5b51d..0fbe24bd7 100644 --- a/internal/git2go/commit_test.go +++ b/internal/git2go/commit_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -63,7 +64,7 @@ func TestExecutor_Commit(t *testing.T) { updatedFile, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("updated")) require.NoError(t, err) - executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go")) + executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go"), command.GitPath()) for _, tc := range []struct { desc string @@ -461,12 +462,14 @@ func TestExecutor_Commit(t *testing.T) { } { t.Run(tc.desc, func(t *testing.T) { author := NewSignature("Author Name", "author.email@example.com", time.Now()) + committer := NewSignature("Committer Name", "committer.email@example.com", time.Now()) var parentCommit string for i, step := range tc.steps { message := fmt.Sprintf("commit %d", i+1) commitID, err := executor.Commit(ctx, CommitParams{ Repository: repoPath, Author: author, + Committer: committer, Message: message, Parent: parentCommit, Actions: step.actions, @@ -482,7 +485,7 @@ func TestExecutor_Commit(t *testing.T) { require.Equal(t, commit{ Parent: parentCommit, Author: author, - Committer: author, + Committer: committer, Message: message, }, getCommit(t, ctx, repo, commitID)) diff --git a/internal/git2go/executor.go b/internal/git2go/executor.go index 0007ab7e5..65058d2d2 100644 --- a/internal/git2go/executor.go +++ b/internal/git2go/executor.go @@ -2,10 +2,14 @@ package git2go // Executor executes gitaly-git2go. type Executor struct { - binaryPath string + binaryPath string + gitBinaryPath string } // New returns a new gitaly-git2go executor using the provided binary. -func New(binaryPath string) Executor { - return Executor{binaryPath: binaryPath} +func New(binaryPath, gitBinaryPath string) Executor { + return Executor{ + binaryPath: binaryPath, + gitBinaryPath: gitBinaryPath, + } } diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 59ce44677..13d8952cd 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -312,9 +312,12 @@ func (s *server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi authorEmail = header.CommitAuthorEmail } + signature := git2go.NewSignature(string(authorName), string(authorEmail), time.Now()) + commitID, err := s.git2go.Commit(ctx, git2go.CommitParams{ Repository: repoPath, - Author: git2go.NewSignature(string(authorName), string(authorEmail), time.Now()), + Author: signature, + Committer: signature, Message: string(header.CommitMessage), Parent: parentCommitOID, Actions: actions, diff --git a/internal/gitaly/service/operations/server.go b/internal/gitaly/service/operations/server.go index af2109508..3a5cd9d88 100644 --- a/internal/gitaly/service/operations/server.go +++ b/internal/gitaly/service/operations/server.go @@ -4,6 +4,7 @@ import ( "path/filepath" "gitlab.com/gitlab-org/gitaly/client" + "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" @@ -29,6 +30,6 @@ func NewServer(cfg config.Cfg, rs *rubyserver.Server, hookManager hook.Manager, hookManager: hookManager, locator: locator, conns: conns, - git2go: git2go.New(filepath.Join(cfg.BinDir, "gitaly-git2go")), + git2go: git2go.New(filepath.Join(cfg.BinDir, "gitaly-git2go"), command.GitPath()), } } |