Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor git history parsing #1009

Merged
merged 1 commit into from
Jun 21, 2024
+71 −217
Merged

Refactor git history parsing #1009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/pint/tests/0028_ci_git_error.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ level=DEBUG msg="Excluding git directory from glob results" path=.git glob=*
level=DEBUG msg="File parsed" path=.pint.hcl rules=0
level=DEBUG msg="File parsed" path=rules.yml rules=2
level=DEBUG msg="Glob finder completed" count=2
level=DEBUG msg="Running git command" args=["log","--format=%H","--no-abbrev-commit","--reverse","notmain..HEAD"]
level=ERROR msg="Fatal error" err="failed to get the list of commits to scan: fatal: ambiguous argument 'notmain..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
level=DEBUG msg="Running git command" args=["log","--reverse","--no-merges","--first-parent","--format=%H","--name-status","notmain..HEAD"]
level=ERROR msg="Fatal error" err="failed to get the list of modified files from git: fatal: ambiguous argument 'notmain..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
-- src/v1.yml --
- record: rule1
expr: sum(foo) by(job)
Expand Down
2 changes: 0 additions & 2 deletions cmd/pint/tests/0065_ci_include.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ exec git commit -am 'v2'
pint.ok -l debug --no-color ci
! stdout .
stderr 'level=DEBUG msg="Got branch information" base=main current=v2'
stderr 'level=DEBUG msg="Found commit to scan" commit=.*'
stderr 'level=DEBUG msg="Found commit to scan" commit=.*'
stderr 'level=DEBUG msg="Git file change" change=A path=a.yml commit=.*'
stderr 'level=DEBUG msg="Git file change" change=A path=b.yml commit=.*'
stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=a.yml'
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0134_ci_base_branch_flag_path.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ level=DEBUG msg="Excluding git directory from glob results" path=.git glob=*
level=DEBUG msg="File parsed" path=.pint.hcl rules=0
level=DEBUG msg="File parsed" path=rules.yml rules=1
level=DEBUG msg="Glob finder completed" count=1
level=DEBUG msg="Running git command" args=["log","--format=%H","--no-abbrev-commit","--reverse","origin/main..HEAD"]
level=ERROR msg="Fatal error" err="failed to get the list of commits to scan: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
level=DEBUG msg="Running git command" args=["log","--reverse","--no-merges","--first-parent","--format=%H","--name-status","origin/main..HEAD"]
level=ERROR msg="Fatal error" err="failed to get the list of modified files from git: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
-- src/v1.yml --
- record: rule1
expr: sum(foo) by(job)
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0135_ci_base_branch_config_path.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ level=DEBUG msg="Excluding git directory from glob results" path=.git glob=*
level=DEBUG msg="File parsed" path=.pint.hcl rules=0
level=DEBUG msg="File parsed" path=rules.yml rules=1
level=DEBUG msg="Glob finder completed" count=1
level=DEBUG msg="Running git command" args=["log","--format=%H","--no-abbrev-commit","--reverse","origin/main..HEAD"]
level=ERROR msg="Fatal error" err="failed to get the list of commits to scan: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
level=DEBUG msg="Running git command" args=["log","--reverse","--no-merges","--first-parent","--format=%H","--name-status","origin/main..HEAD"]
level=ERROR msg="Fatal error" err="failed to get the list of modified files from git: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
-- src/v1.yml --
- record: rule1
expr: sum(foo) by(job)
Expand Down
2 changes: 0 additions & 2 deletions cmd/pint/tests/0139_ci_exclude.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ exec git commit -am 'v2'
pint.ok -l debug --no-color ci
! stdout .
stderr 'level=DEBUG msg="Got branch information" base=main current=v2'
stderr 'level=DEBUG msg="Found commit to scan" commit=.*'
stderr 'level=DEBUG msg="Found commit to scan" commit=.*'
stderr 'level=DEBUG msg="Git file change" change=A path=a.yml commit=.*'
stderr 'level=DEBUG msg="Git file change" change=A path=b.yml commit=.*'
stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=a.yml'
Expand Down
2 changes: 0 additions & 2 deletions cmd/pint/tests/0140_ci_include_exclude.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ exec git commit -am 'v2'
pint.ok -l debug --no-color ci
! stdout .
stderr 'level=DEBUG msg="Got branch information" base=main current=v2'
stderr 'level=DEBUG msg="Found commit to scan" commit=.*'
stderr 'level=DEBUG msg="Found commit to scan" commit=.*'
stderr 'level=DEBUG msg="Git file change" change=A path=a.yml commit=.*'
stderr 'level=DEBUG msg="Git file change" change=A path=b.yml commit=.*'
stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=a.yml'
Expand Down
1 change: 1 addition & 0 deletions docs/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixed

- Don't suggest using `humanize` when alert template is already using printf on format the `$value`.
- Fixed git history parsing when running `pint ci` on a branch that include merge commits.

## v0.60.0

Expand Down
24 changes: 14 additions & 10 deletions internal/discovery/git_branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,13 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) {
allEntries[i].State = Excluded
}

cr, err := git.CommitRange(f.gitCmd, f.baseBranch)
changes, err := git.Changes(f.gitCmd, f.baseBranch, f.filter)
if err != nil {
return nil, fmt.Errorf("failed to get the list of commits to scan: %w", err)
return nil, err
}
slog.Debug("Got commit range from git", slog.String("from", cr.From), slog.String("to", cr.To))

if len(cr.Commits) > f.maxCommits {
return nil, fmt.Errorf("number of commits to check (%d) is higher than maxCommits (%d), exiting", len(cr.Commits), f.maxCommits)
}

changes, err := git.Changes(f.gitCmd, cr, f.filter)
if err != nil {
return nil, err
if totalCommits := countCommits(changes); totalCommits > f.maxCommits {
return nil, fmt.Errorf("number of commits to check (%d) is higher than maxCommits (%d), exiting", totalCommits, f.maxCommits)
}

shouldSkip, err := f.shouldSkipAllChecks(changes)
Expand Down Expand Up @@ -321,3 +315,13 @@ func findRulesByName(entries []Entry, name string, typ parser.RuleType) (nomatch
}
return nomatch, match
}

func countCommits(changes []*git.FileChange) int {
commits := map[string]struct{}{}
for _, change := range changes {
for _, commit := range change.Commits {
commits[commit] = struct{}{}
}
}
return len(commits)
}
40 changes: 17 additions & 23 deletions internal/discovery/git_branch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestGitBranchFinder(t *testing.T) {
50,
),
entries: nil,
err: "failed to get the list of commits to scan: mock git error: [log --format=%H --no-abbrev-commit --reverse main..HEAD]",
err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD]",
},
{
title: "git list PR commits error - master",
Expand All @@ -94,24 +94,22 @@ func TestGitBranchFinder(t *testing.T) {
50,
),
entries: nil,
err: "failed to get the list of commits to scan: mock git error: [log --format=%H --no-abbrev-commit --reverse master..HEAD]",
err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status master..HEAD]",
},
{
title: "too many commits",
setup: func(_ *testing.T) {},
finder: discovery.NewGitBranchFinder(
func(args ...string) ([]byte, error) {
switch strings.Join(args, " ") {
case "log --format=%H --no-abbrev-commit --reverse main..HEAD":
return []byte("c1\nc2\nc3\nc4\n"), nil
default:
return nil, fmt.Errorf("mock git error: %v", args)
}
},
git.NewPathFilter(includeAll, nil, nil),
"main",
3,
),
setup: func(t *testing.T) {
commitFile(t, "rules.yml", "# v1\n", "v1")

_, err := git.RunGit("checkout", "-b", "v2")
require.NoError(t, err, "git checkout v2")

commitFile(t, "rules.yml", "# v2-1\n", "v2-1")
commitFile(t, "rules.yml", "# v2-2\n", "v2-2")
commitFile(t, "rules.yml", "# v2-3\n", "v2-3")
commitFile(t, "rules.yml", "# v2-4\n", "v2-4")
},
finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, nil), "main", 3),
entries: nil,
err: "number of commits to check (4) is higher than maxCommits (3), exiting",
},
Expand All @@ -132,17 +130,15 @@ func TestGitBranchFinder(t *testing.T) {
4,
),
entries: nil,
err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status c1^..c4]",
err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD]",
},
{
title: "git get commit message error",
setup: func(_ *testing.T) {},
finder: discovery.NewGitBranchFinder(
func(args ...string) ([]byte, error) {
switch strings.Join(args, " ") {
case "log --format=%H --no-abbrev-commit --reverse main..HEAD":
return []byte("c1\nc2\nc3\nc4\n"), nil
case "log --reverse --no-merges --first-parent --format=%H --name-status c1^..c4":
case "log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD":
return []byte("c1\nA\trules.yml\n"), nil
default:
return nil, fmt.Errorf("mock git error: %v", args)
Expand All @@ -161,9 +157,7 @@ func TestGitBranchFinder(t *testing.T) {
finder: discovery.NewGitBranchFinder(
func(args ...string) ([]byte, error) {
switch strings.Join(args, " ") {
case "log --format=%H --no-abbrev-commit --reverse main..HEAD":
return []byte("c1\nc2\nc3\nc4\n"), nil
case "log --reverse --no-merges --first-parent --format=%H --name-status c1^..c4":
case "log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD":
return []byte("c1\nA\trules.yml\n"), nil
case "ls-tree c1^ rules.yml":
return []byte("100644 blob c0\trules.yml"), nil
Expand Down
4 changes: 2 additions & 2 deletions internal/git/changes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ type FileChange struct {
Status FileStatus
}

func Changes(cmd CommandRunner, cr CommitRangeResults, filter PathFilter) ([]*FileChange, error) {
out, err := cmd("log", "--reverse", "--no-merges", "--first-parent", "--format=%H", "--name-status", cr.String())
func Changes(cmd CommandRunner, baseBranch string, filter PathFilter) ([]*FileChange, error) {
out, err := cmd("log", "--reverse", "--no-merges", "--first-parent", "--format=%H", "--name-status", fmt.Sprintf("%s..HEAD", baseBranch))
if err != nil {
return nil, fmt.Errorf("failed to get the list of modified files from git: %w", err)
}
Expand Down
Loading
Loading