Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Add squash-merge support into oc adm release info
Repositories, like insights-operator, use squash-merge and
`oc adm release info --changes-from` does not show changes for these repos.

This PR adds handling also squash merging and shows changes.
  • Loading branch information
ardaguclu committed Apr 26, 2022
commit 4299014c8238bc9f1aec60a28c80eb2807cc65a6
39 changes: 32 additions & 7 deletions pkg/cli/admin/release/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,9 @@ func gitOutputToError(err error, out string) error {
}

var (
rePR = regexp.MustCompile(`^Merge pull request #(\d+) from`)
rePrefix = regexp.MustCompile(`^(\[[\w\.\-]+\]\s*)+`)
squashRePR = regexp.MustCompile(`#(\d+)`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at Insights history:

$ git clone --depth 10 --branch release-4.11 https://github.com/openshift/insights-operator.git
$ cd insights-operator
$ git --no-pager log --no-merges --topo-order HEAD^^^..
commit 95961e7113ce179bf1621ac38842cad691d5b870 (HEAD -> release-4.11, origin/release-4.11)
Author: Tomas Remes <[email protected]>
Date:   Fri Apr 22 20:03:08 2022 +0200

    Fix vendoring of the build-machinery-go (#613)

commit 8f0ed50a6e1eaecf66650e364ee676339697c4ad
Author: Tomas Remes <[email protected]>
Date:   Thu Apr 21 16:23:10 2022 +0200

    Minor gatherer documentation update (#606)

commit 91b6fb4e66f52d44696ad6c3bd4cf06aae290dfe
Author: Tomas Remes <[email protected]>
Date:   Thu Apr 21 16:23:03 2022 +0200

    Create a new Prometheus metric providing Insights gathering time (#600)
    
    * Create a new Prometheus metric providing Insights gathering time
    
    * Rename the metric

Perhaps this regexp should require the wrapping parens too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regexp extracts;

Fix vendoring of the build-machinery-go (#613)
Minor gatherer documentation update (#606)
Create a new Prometheus metric providing Insights gathering time (#600)

What do you mean by "wrapping parents"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will also match 123 in

Fix vendoring from #123 (#613)

Using [(]#(\d+)[)] or similar will only match 613.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right @wking, thanks. I'll open a fix PR.

rePR = regexp.MustCompile(`^Merge pull request #(\d+) from`)
rePrefix = regexp.MustCompile(`^(\[[\w\.\-]+\]\s*)+`)
)

func mergeLogForRepo(g gitInterface, repo string, from, to string) ([]MergeCommit, error) {
Expand Down Expand Up @@ -194,6 +195,20 @@ func mergeLogForRepo(g gitInterface, repo string, from, to string) ([]MergeCommi
}
}

squash := false
if out == "" {
// some repositories use squash merging(like insights-operator) and
// out which is populated by --merges flag is empty. Thereby,
// we are trying to get git log with --no-merges flag for that repositories
// in order to get the logs.
args = []string{"log", "--no-merges", "--topo-order", "-z", "--pretty=format:%H %P%x1E%ct%x1E%s%x1E%b", fmt.Sprintf("%s..%s", from, to)}
out, err = g.exec(args...)
if err != nil {
return nil, gitOutputToError(err, out)
}
squash = true
}

if klog.V(5).Enabled() {
klog.Infof("Got commit info:\n%s", strconv.Quote(out))
}
Expand All @@ -220,20 +235,30 @@ func mergeLogForRepo(g gitInterface, repo string, from, to string) ([]MergeCommi
}

msg := records[3]
if squash {
msg = records[2]
}

mergeCommit.Bugs, msg = extractBugs(msg)
msg = strings.TrimSpace(msg)
msg = strings.SplitN(msg, "\n", 2)[0]

mergeMsg := records[2]
if m := rePR.FindStringSubmatch(mergeMsg); m != nil {
mergeCommit.PullRequest, err = strconv.Atoi(m[1])
if err != nil {
return nil, fmt.Errorf("could not extract PR number from %q: %v", mergeMsg, err)
}
var m []string
if !squash {
m = rePR.FindStringSubmatch(mergeMsg)
} else {
m = squashRePR.FindStringSubmatch(mergeMsg)
}

if m == nil || len(m) < 2 {
klog.V(2).Infof("Omitted commit %s which has no pull-request", mergeCommit.Commit)
continue
}
mergeCommit.PullRequest, err = strconv.Atoi(m[1])
if err != nil {
return nil, fmt.Errorf("could not extract PR number from %q: %v", mergeMsg, err)
}
if len(msg) == 0 {
msg = "Merge"
}
Expand Down
45 changes: 43 additions & 2 deletions pkg/cli/admin/release/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ import (
)

type fakeGit struct {
input string
input string
squash bool
}

func (g fakeGit) exec(commands ...string) (string, error) {
if commands[1] == "--merges" && g.squash {
return "", nil
}
return g.input, nil
}

Expand All @@ -23,6 +27,7 @@ func Test_mergeLogForRepo(t *testing.T) {
repo string
from string
to string
squash bool
want []MergeCommit
wantErr bool
}{
Expand Down Expand Up @@ -205,10 +210,46 @@ func Test_mergeLogForRepo(t *testing.T) {
},
},
},
{
input: "abc\x1e1\x1eBugs 1743564: fix typo (#145)\x1e * fix typo",
squash: true,
want: []MergeCommit{
{
ParentCommits: []string{}, Commit: "abc", PullRequest: 145, CommitDate: time.Unix(1, 0).UTC(),
Bugs: BugList{
Bugs: []Bug{
{
ID: 1743564,
Source: Bugzilla,
},
},
},
Subject: "fix typo (#145)",
},
},
},
{
input: "abc\x1e1\x1eOCPBUGS-1743564: fix typo (#145)\x1e * fix typo",
squash: true,
want: []MergeCommit{
{
ParentCommits: []string{}, Commit: "abc", PullRequest: 145, CommitDate: time.Unix(1, 0).UTC(),
Bugs: BugList{
Bugs: []Bug{
{
ID: 1743564,
Source: Jira,
},
},
},
Subject: "fix typo (#145)",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := fakeGit{input: tt.input}
g := fakeGit{input: tt.input, squash: tt.squash}
got, err := mergeLogForRepo(g, tt.repo, "a", "b")
if (err != nil) != tt.wantErr {
t.Errorf("mergeLogForRepo() error = %v, wantErr %v", err, tt.wantErr)
Expand Down