diff options
author | Stan Hu <stanhu@gmail.com> | 2020-06-10 00:43:08 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2020-06-11 02:44:20 +0300 |
commit | b10788c42fdedcc60c960aa5169b6b30cc3a4881 (patch) | |
tree | af0b18eac6daafcc589fffb4e8b3373c709c86a1 | |
parent | cdc388153a3c8d353367a96173b7460ce7dfbcde (diff) |
Enforce literal paths for LastCommitForPath
By default, Git allows the pathspec to have glob patterns
(https://css-tricks.com/git-pathspecs-and-how-to-use-them), but the
LastCommitForPath RPC only uses the path argument as a literal. This can
be done via the --literal-pathspecs argument
(https://git-scm.com/docs/git#Documentation/git.txt---literal-pathspecs)
or by adding `(:literal)` before the pathspec. This commit does the
former because it seems clearer.
Closes https://gitlab.com/gitlab-org/gitaly/-/issues/2857
-rw-r--r-- | changelogs/unreleased/sh-fix-last-commit-pathspec.yml | 5 | ||||
-rw-r--r-- | internal/git/log/last_commit.go | 2 | ||||
-rw-r--r-- | internal/service/commit/last_commit_for_path_test.go | 45 |
3 files changed, 51 insertions, 1 deletions
diff --git a/changelogs/unreleased/sh-fix-last-commit-pathspec.yml b/changelogs/unreleased/sh-fix-last-commit-pathspec.yml new file mode 100644 index 000000000..1fe986254 --- /dev/null +++ b/changelogs/unreleased/sh-fix-last-commit-pathspec.yml @@ -0,0 +1,5 @@ +--- +title: Enforce literal paths for LastCommitForPath +merge_request: 2271 +author: +type: fixed diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go index 76b3c9915..e99914ae6 100644 --- a/internal/git/log/last_commit.go +++ b/internal/git/log/last_commit.go @@ -13,7 +13,7 @@ import ( // LastCommitForPath returns the last commit which modified path. func LastCommitForPath(ctx context.Context, batch *catfile.Batch, repo *gitalypb.Repository, revision string, path string) (*gitalypb.GitCommit, error) { - cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{ + cmd, err := git.SafeCmd(ctx, repo, []git.Option{git.Flag{"--literal-pathspecs"}}, git.SubCmd{ Name: "log", Flags: []git.Option{git.Flag{"--format=%H"}, git.Flag{"--max-count=1"}}, Args: []string{revision}, diff --git a/internal/service/commit/last_commit_for_path_test.go b/internal/service/commit/last_commit_for_path_test.go index 2b6f2ec75..152575848 100644 --- a/internal/service/commit/last_commit_for_path_test.go +++ b/internal/service/commit/last_commit_for_path_test.go @@ -2,6 +2,9 @@ package commit import ( "context" + "os" + "path/filepath" + "strings" "testing" "github.com/golang/protobuf/ptypes/timestamp" @@ -142,3 +145,45 @@ func TestFailedLastCommitForPathRequest(t *testing.T) { }) } } + +func TestSuccessfulLastCommitWithGlobCharacters(t *testing.T) { + server, serverSocketPath := startTestServices(t) + defer server.Stop() + + client, conn := newCommitServiceClient(t, serverSocketPath) + defer conn.Close() + + testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepoWithWorktree(t) + defer cleanupFn() + + branch := "last-commit-test" + filename := filepath.Join(testRepoPath, ":wq") + f, err := os.Create(filename) + require.NoError(t, err) + f.WriteString("hello world\n") + f.Close() + + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", branch, "master") + testhelper.MustRunCommand(t, nil, "git", "--literal-pathspecs", "-C", testRepoPath, "add", ":wq") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-a", "-m", "add file") + + branchSha := getBranchSha(t, testRepoPath, branch) + + request := &gitalypb.LastCommitForPathRequest{ + Repository: testRepo, + Revision: []byte(branchSha), + Path: []byte(":wq"), + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + response, err := client.LastCommitForPath(ctx, request) + require.NoError(t, err) + require.NotNil(t, response.GetCommit()) + require.Equal(t, branchSha, response.GetCommit().Id) +} + +func getBranchSha(t *testing.T, repoPath string, branchName string) string { + branchSha := string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "rev-parse", branchName)) + return strings.TrimSpace(branchSha) +} |