From 9ea82ea20d801cde88d60826cff147e6965bc003 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 12 Feb 2019 23:35:35 -0500 Subject: [PATCH] Add simple changelog generator $ oc adm release info --changelog=/tmp/git ... Will generate an opinionated changelog in markdown from the payload. It requires `oc adm release extract --git=DIR` to be invoked first, and will scan for PR merge commits in those repos and expose them. Issues that begin with `Bug [0-9]+: ` will be linked to bugzilla. This will eventually also be able to print a list of bugs that can be marked as part of the payload. --- contrib/completions/bash/oc | 4 + contrib/completions/zsh/oc | 4 + pkg/oc/cli/admin/release/extract.go | 26 +- pkg/oc/cli/admin/release/info.go | 659 +++++++++++++++++++++++++++- 4 files changed, 662 insertions(+), 31 deletions(-) diff --git a/contrib/completions/bash/oc b/contrib/completions/bash/oc index 54412b6e9e91..f85a524e1f8e 100644 --- a/contrib/completions/bash/oc +++ b/contrib/completions/bash/oc @@ -4645,6 +4645,10 @@ _oc_adm_release_info() flags_with_completion=() flags_completion=() + flags+=("--bugs=") + local_nonpersistent_flags+=("--bugs=") + flags+=("--changelog=") + local_nonpersistent_flags+=("--changelog=") flags+=("--changes-from=") local_nonpersistent_flags+=("--changes-from=") flags+=("--commits") diff --git a/contrib/completions/zsh/oc b/contrib/completions/zsh/oc index 3fd947366e26..74262f2d4822 100644 --- a/contrib/completions/zsh/oc +++ b/contrib/completions/zsh/oc @@ -4787,6 +4787,10 @@ _oc_adm_release_info() flags_with_completion=() flags_completion=() + flags+=("--bugs=") + local_nonpersistent_flags+=("--bugs=") + flags+=("--changelog=") + local_nonpersistent_flags+=("--changelog=") flags+=("--changes-from=") local_nonpersistent_flags+=("--changes-from=") flags+=("--commits") diff --git a/pkg/oc/cli/admin/release/extract.go b/pkg/oc/cli/admin/release/extract.go index 92d4b15f0669..ab858f16406b 100644 --- a/pkg/oc/cli/admin/release/extract.go +++ b/pkg/oc/cli/admin/release/extract.go @@ -240,16 +240,10 @@ func (o *ExtractOptions) extractGit(dir string) error { } alreadyExtracted[repo] = commit - u, err := sourceLocationAsURL(repo) + basePath, err := sourceLocationAsRelativePath(dir, repo) if err != nil { return err } - gitPath := u.Path - if strings.HasSuffix(gitPath, ".git") { - gitPath = strings.TrimSuffix(gitPath, ".git") - } - gitPath = path.Clean(gitPath) - basePath := filepath.Join(dir, u.Host, filepath.FromSlash(gitPath)) var git *git fi, err := os.Stat(basePath) @@ -341,14 +335,14 @@ func (g *git) basename() string { } func (g *git) CheckoutCommit(repo, commit string) error { - _, err := g.exec("rev-parse", commit) + _, err := g.exec("show-ref", commit) if err == nil { return nil } // try to fetch by URL if _, err := g.exec("fetch", repo); err == nil { - if _, err := g.exec("rev-parse", commit); err == nil { + if _, err := g.exec("show-ref", commit); err == nil { return nil } } @@ -366,3 +360,17 @@ func sourceLocationAsURL(location string) (*url.URL, error) { } return url.Parse(location) } + +func sourceLocationAsRelativePath(dir, location string) (string, error) { + u, err := sourceLocationAsURL(location) + if err != nil { + return "", err + } + gitPath := u.Path + if strings.HasSuffix(gitPath, ".git") { + gitPath = strings.TrimSuffix(gitPath, ".git") + } + gitPath = path.Clean(gitPath) + basePath := filepath.Join(dir, u.Host, filepath.FromSlash(gitPath)) + return basePath, nil +} diff --git a/pkg/oc/cli/admin/release/info.go b/pkg/oc/cli/admin/release/info.go index cd8cd84857cf..46862bfee7b4 100644 --- a/pkg/oc/cli/admin/release/info.go +++ b/pkg/oc/cli/admin/release/info.go @@ -7,13 +7,18 @@ import ( "fmt" "io" "io/ioutil" + "net/http" "net/url" "path" + "regexp" "sort" + "strconv" "strings" "text/tabwriter" "time" + "github.com/MakeNowJust/heredoc" + "github.com/blang/semver" "github.com/golang/glog" @@ -23,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/duration" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -63,6 +69,8 @@ func NewInfo(f kcmdutil.Factory, parentName string, streams genericclioptions.IO flags.BoolVar(&o.ShowPullSpec, "pullspecs", o.ShowPullSpec, "Display the pull spec of each image instead of the digest.") flags.StringVar(&o.ImageFor, "image-for", o.ImageFor, "Print the pull spec of the specified image or an error if it does not exist.") flags.StringVarP(&o.Output, "output", "o", o.Output, "Display the release info in an alternative format: json") + flags.StringVar(&o.ChangelogDir, "changelog", o.ChangelogDir, "Generate changelog output from the git directories extracted to this path.") + flags.StringVar(&o.BugsDir, "bugs", o.BugsDir, "Generate bug listings from the changelogs in the git repositories extracted to this path.") return cmd } @@ -78,6 +86,9 @@ type InfoOptions struct { ShowPullSpec bool Verify bool + ChangelogDir string + BugsDir string + RegistryConfig string } @@ -111,6 +122,10 @@ func (o *InfoOptions) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []st return fmt.Errorf("info expects at least one argument, a release image pull spec") } o.Images = args + if len(o.From) == 0 && len(o.Images) == 2 { + o.From = o.Images[0] + o.Images = o.Images[1:] + } return nil } @@ -118,15 +133,36 @@ func (o *InfoOptions) Validate() error { if len(o.ImageFor) > 0 && len(o.Output) > 0 { return fmt.Errorf("--output and --image-for may not both be specified") } - switch o.Output { - case "", "json": + if len(o.ChangelogDir) > 0 || len(o.BugsDir) > 0 { + if len(o.From) == 0 { + return fmt.Errorf("--changelog/--bugs require --from") + } + if len(o.ImageFor) > 0 || o.ShowCommit || o.ShowPullSpec || o.Verify { + return fmt.Errorf("--changelog/--bugs may not be specified with any other flag except --from") + } + } + if len(o.ChangelogDir) > 0 && len(o.BugsDir) > 0 { + return fmt.Errorf("--changelog and --bugs may not both be specified") + } + switch { + case len(o.BugsDir) > 0: + switch o.Output { + case "", "name": + default: + return fmt.Errorf("--output only supports 'name' for --bugs") + } + case len(o.ChangelogDir) > 0: + if len(o.Output) > 0 { + return fmt.Errorf("--output is not supported for this mode") + } default: - return fmt.Errorf("--output only supports 'json'") + switch o.Output { + case "", "json": + default: + return fmt.Errorf("--output only supports 'json'") + } } - return nil -} -func (o *InfoOptions) Run() error { if len(o.Images) == 0 { return fmt.Errorf("must specify a release image as an argument") } @@ -134,6 +170,10 @@ func (o *InfoOptions) Run() error { return fmt.Errorf("must specify a single release image as argument when comparing to another release image") } + return nil +} + +func (o *InfoOptions) Run() error { if len(o.From) > 0 { var baseRelease *ReleaseInfo var baseErr error @@ -156,6 +196,12 @@ func (o *InfoOptions) Run() error { if err != nil { return err } + if len(o.BugsDir) > 0 { + return describeBugs(o.Out, o.ErrOut, diff, o.BugsDir, o.Output) + } + if len(o.ChangelogDir) > 0 { + return describeChangelog(o.Out, o.ErrOut, diff, o.ChangelogDir) + } return describeReleaseDiff(o.Out, diff, o.ShowCommit) } @@ -452,14 +498,26 @@ func describeReleaseDiff(out io.Writer, diff *ReleaseDiff, showCommit bool) erro if len(diff.ChangedImages) > 0 { var keys []string + maxLen := 0 for k := range diff.ChangedImages { + if len(k) > maxLen { + maxLen = len(k) + } keys = append(keys, k) } + justify := func(s string) string { + return s + strings.Repeat(" ", maxLen-len(s)) + } sort.Strings(keys) + var rebuilt []string writeTabSection(w, func(w io.Writer) { count := 0 for _, k := range keys { if image := diff.ChangedImages[k]; image.To != nil && image.From != nil { + if !codeChanged(image.From, image.To) { + rebuilt = append(rebuilt, k) + continue + } if count == 0 { fmt.Fprintln(w) fmt.Fprintf(w, "Images Changed:\n") @@ -468,15 +526,38 @@ func describeReleaseDiff(out io.Writer, diff *ReleaseDiff, showCommit bool) erro old, new := digestOrRef(image.From.From.Name), digestOrRef(image.To.From.Name) if old != new { if showCommit { - fmt.Fprintf(w, " %s\t%s\n", image.Name, gitDiffOrCommit(image.From, image.To)) + fmt.Fprintf(w, " %s\t%s\n", justify(image.Name), gitDiffOrCommit(image.From, image.To)) } else { - fmt.Fprintf(w, " %s\t%s\t%s\n", image.Name, old, new) + fmt.Fprintf(w, " %s\t%s\t%s\n", justify(image.Name), old, new) } } } } }) + if len(rebuilt) > 0 { + writeTabSection(w, func(w io.Writer) { + count := 0 + for _, k := range rebuilt { + if image := diff.ChangedImages[k]; image.To != nil && image.From != nil { + if count == 0 { + fmt.Fprintln(w) + fmt.Fprintf(w, "Images Rebuilt:\n") + } + count++ + old, new := digestOrRef(image.From.From.Name), digestOrRef(image.To.From.Name) + if old != new { + if showCommit { + fmt.Fprintf(w, " %s\t%s\n", justify(image.Name), gitDiffOrCommit(image.From, image.To)) + } else { + fmt.Fprintf(w, " %s\t%s\t%s\n", justify(image.Name), old, new) + } + } + } + } + }) + } + writeTabSection(w, func(w io.Writer) { count := 0 for _, k := range keys { @@ -487,9 +568,9 @@ func describeReleaseDiff(out io.Writer, diff *ReleaseDiff, showCommit bool) erro } count++ if showCommit { - fmt.Fprintf(w, " %s\t%s\n", image.Name, repoAndCommit(image.To)) + fmt.Fprintf(w, " %s\t%s\n", justify(image.Name), repoAndCommit(image.To)) } else { - fmt.Fprintf(w, " %s\t%s\n", image.Name, digestOrRef(image.To.From.Name)) + fmt.Fprintf(w, " %s\t%s\n", justify(image.Name), digestOrRef(image.To.From.Name)) } } } @@ -504,7 +585,7 @@ func describeReleaseDiff(out io.Writer, diff *ReleaseDiff, showCommit bool) erro fmt.Fprintf(w, "Images Removed:\n") } count++ - fmt.Fprintf(w, " %s\n", image.Name) + fmt.Fprintf(w, " %s\n", justify(image.Name)) } } }) @@ -519,7 +600,7 @@ func repoAndCommit(ref *imageapi.TagReference) string { if len(repo) == 0 || len(commit) == 0 { return "" } - return fmt.Sprintf("%s %s", repo, commit) + return urlForRepoAndCommit(repo, commit) } func gitDiffOrCommit(from, to *imageapi.TagReference) string { @@ -530,20 +611,42 @@ func gitDiffOrCommit(from, to *imageapi.TagReference) string { } if oldRepo == newRepo { if oldCommit == newCommit { - return fmt.Sprintf("%s %s", newRepo, newCommit) - } - if strings.HasPrefix(newRepo, "https://github.com/") { - if u, err := url.Parse(newRepo); err == nil { - u.Path = path.Join(u.Path, "compare", fmt.Sprintf("%s...%s", oldCommit, newCommit)) - return u.String() - } + return urlForRepoAndCommit(newRepo, newCommit) } - return fmt.Sprintf("%s %s %s", newRepo, oldCommit, newCommit) + return urlForRepoAndCommitRange(newRepo, oldCommit, newCommit) } if len(oldCommit) == 0 { - return fmt.Sprintf("%s %s", newRepo, newCommit) + return fmt.Sprintf("%s -> %s", oldRepo, urlForRepoAndCommit(newRepo, newCommit)) } - return fmt.Sprintf("%s %s %s", newRepo, oldCommit, newCommit) + if oldCommit == newCommit { + return fmt.Sprintf("%s -> %s", oldRepo, urlForRepoAndCommit(newRepo, newCommit)) + } + return fmt.Sprintf("%s -> %s", urlForRepoAndCommit(oldRepo, oldCommit), urlForRepoAndCommit(newRepo, newCommit)) +} + +func urlForRepoAndCommit(repo, commit string) string { + if strings.HasPrefix(repo, "https://github.com/") { + if u, err := url.Parse(repo); err == nil { + u.Path = path.Join(u.Path, "commit", fmt.Sprintf("%s", commit)) + return u.String() + } + } + return fmt.Sprintf("%s %s", repo, commit) +} + +func urlForRepoAndCommitRange(repo, from, to string) string { + if strings.HasPrefix(repo, "https://github.com/") { + if u, err := url.Parse(repo); err == nil { + u.Path = path.Join(u.Path, "changes", fmt.Sprintf("%s...%s", from, to)) + return u.String() + } + } + return fmt.Sprintf("%s %s %s", repo, from, to) +} + +func codeChanged(from, to *imageapi.TagReference) bool { + oldCommit, newCommit := from.Annotations["io.openshift.build.commit.id"], to.Annotations["io.openshift.build.commit.id"] + return len(oldCommit) > 0 && len(newCommit) > 0 && oldCommit != newCommit } func describeReleaseInfo(out io.Writer, release *ReleaseInfo, showCommit, pullSpec bool) error { @@ -648,3 +751,515 @@ func digestOrRef(ref string) string { } return ref } + +func describeChangelog(out, errOut io.Writer, diff *ReleaseDiff, dir string) error { + if diff.To.Digest == diff.From.Digest { + return fmt.Errorf("releases are identical") + } + + fmt.Fprintf(out, heredoc.Docf(` + # %s + + Created: %s + Image Digest: %s + + ## Changes from %s + + `, diff.To.PreferredName(), diff.To.References.CreationTimestamp.UTC(), "`"+diff.To.Digest+"`", diff.From.PreferredName())) + + var hasError bool + + var added, removed []string + for k, imageDiff := range diff.ChangedImages { + switch { + case imageDiff.From == nil: + added = append(added, k) + case imageDiff.To == nil: + removed = append(removed, k) + } + } + codeChanges, imageChanges, incorrectImageChanges := releaseDiffContentChanges(diff) + + sort.Strings(added) + sort.Strings(removed) + + if len(added) > 0 { + fmt.Fprintf(out, "### New images\n\n") + for _, k := range added { + fmt.Fprintf(out, "* %s\n", refToShortDescription(diff.ChangedImages[k].To)) + } + fmt.Fprintln(out) + fmt.Fprintln(out) + } + + if len(removed) > 0 { + fmt.Fprintf(out, "### Removed images\n\n") + for _, k := range removed { + fmt.Fprintf(out, "* %s\n", k) + } + fmt.Fprintln(out) + fmt.Fprintln(out) + } + + if len(imageChanges) > 0 || len(incorrectImageChanges) > 0 { + fmt.Fprintf(out, "### Rebuilt images without code change\n\n") + for _, change := range imageChanges { + switch { + case len(change.To.ID) > 0: + fmt.Fprintf(out, "* %s `@%s`\n", change.Name, change.To.ID) + case len(change.To.Tag) > 0: + fmt.Fprintf(out, "* %s `:%s`\n", change.Name, change.To.Tag) + default: + fmt.Fprintf(out, "* %s `%s`\n", change.Name, change.To.Exact()) + } + } + for _, k := range incorrectImageChanges { + fmt.Fprintf(out, "* %s\n", k) + } + fmt.Fprintln(out) + fmt.Fprintln(out) + } + + for _, change := range codeChanges { + u, commits, err := commitsForRepo(dir, change) + if err != nil { + fmt.Fprintf(errOut, "error: %v\n", err) + hasError = true + continue + } + if len(commits) > 0 { + fmt.Fprintf(out, "### %s\n\n", strings.Join(change.ImagesAffected, ", ")) + for _, commit := range commits { + var suffix string + switch { + case commit.PullRequest > 0: + suffix = fmt.Sprintf("[#%d](%s)", commit.PullRequest, fmt.Sprintf("https://%s%s/pull/%d", u.Host, u.Path, commit.PullRequest)) + case u.Host == "github.com": + commit := commit.Commit[:8] + suffix = fmt.Sprintf("[%s](%s)", commit, fmt.Sprintf("https://%s%s/commit/%s", u.Host, u.Path, commit)) + default: + suffix = commit.Commit[:8] + } + switch { + case commit.Bug > 0: + fmt.Fprintf(out, + "* [Bug %d](%s): %s %s\n", + commit.Bug, + fmt.Sprintf("https://bugzilla.redhat.com/show_bug.cgi?id=%d", commit.Bug), + commit.Subject, + suffix, + ) + default: + fmt.Fprintf(out, + "* %s %s\n", + commit.Subject, + suffix, + ) + } + } + if u.Host == "github.com" { + fmt.Fprintf(out, "* [Full changelog](%s)\n\n", fmt.Sprintf("https://%s%s/compare/%s...%s", u.Host, u.Path, change.From, change.To)) + } else { + fmt.Fprintf(out, "* %s from %s to %s\n\n", change.Repo, change.FromShort(), change.ToShort()) + } + fmt.Fprintln(out) + } + } + if hasError { + return kcmdutil.ErrExit + } + return nil +} + +func describeBugs(out, errOut io.Writer, diff *ReleaseDiff, dir string, format string) error { + if diff.To.Digest == diff.From.Digest { + return fmt.Errorf("releases are identical") + } + + var hasError bool + codeChanges, _, _ := releaseDiffContentChanges(diff) + + bugIDs := sets.NewInt() + for _, change := range codeChanges { + _, commits, err := commitsForRepo(dir, change) + if err != nil { + fmt.Fprintf(errOut, "error: %v\n", err) + hasError = true + continue + } + for _, commit := range commits { + if commit.Bug == 0 { + continue + } + bugIDs.Insert(commit.Bug) + } + } + + bugs := make(map[int]BugInfo) + + u, err := url.Parse("https://bugzilla.redhat.com/rest/bug") + if err != nil { + return err + } + client := http.DefaultClient + allBugIDs := bugIDs.List() + for len(allBugIDs) > 0 { + var next []int + if len(allBugIDs) > 10 { + next = allBugIDs[:10] + allBugIDs = allBugIDs[10:] + } else { + next = allBugIDs + allBugIDs = nil + } + q := url.Values{} + for _, id := range next { + q.Add("id", strconv.Itoa(id)) + } + u.RawQuery = q.Encode() + resp, err := client.Get(u.String()) + if err != nil || resp.StatusCode != 200 { + // retry once + resp, err = client.Get(u.String()) + if err != nil || resp.StatusCode != 200 { + return fmt.Errorf("unable to get bug values: %v", err) + } + } + data, err := ioutil.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("unable to get body contents: %v", err) + } + resp.Body.Close() + var bugList BugList + if err := json.Unmarshal(data, &bugList); err != nil { + return fmt.Errorf("unable to parse bug list: %v", err) + } + for _, bug := range bugList.Bugs { + bugs[bug.ID] = bug + } + } + + var valid []int + for _, id := range bugIDs.List() { + if _, ok := bugs[id]; !ok { + fmt.Fprintf(errOut, "error: Bug %d was not retrieved\n", id) + hasError = true + continue + } + valid = append(valid, id) + } + + if len(valid) > 0 { + switch format { + case "name": + for _, id := range valid { + fmt.Fprintln(out, id) + } + default: + tw := tabwriter.NewWriter(out, 0, 0, 1, ' ', 0) + fmt.Fprintln(tw, "ID\tSTATUS\tPRIORITY\tSUMMARY") + for _, id := range valid { + bug := bugs[id] + fmt.Fprintf(tw, "%d\t%s\t%s\t%s\n", id, bug.Status, bug.Priority, bug.Summary) + } + tw.Flush() + } + } + + if hasError { + return kcmdutil.ErrExit + } + return nil +} + +func retrieveBugs(client *http.Client, server *url.URL, bugs []int, retries int) (*BugList, error) { + q := url.Values{} + for _, id := range bugs { + q.Add("id", strconv.Itoa(id)) + } + u := *server + u.RawQuery = q.Encode() + var lastErr error + for i := 0; i < retries; i++ { + resp, err := client.Get(u.String()) + if err != nil { + lastErr = err + continue + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + lastErr = fmt.Errorf("server responded with %d", resp.StatusCode) + continue + } + data, err := ioutil.ReadAll(resp.Body) + if err != nil { + lastErr = fmt.Errorf("unable to get body contents: %v", err) + continue + } + resp.Body.Close() + var bugList BugList + if err := json.Unmarshal(data, &bugList); err != nil { + lastErr = fmt.Errorf("unable to parse bug list: %v", err) + continue + } + return &bugList, nil + } + return nil, lastErr +} + +type BugList struct { + Bugs []BugInfo `json:"bugs"` +} + +type BugInfo struct { + ID int `json:"id` + Status string `json:"status"` + Priority string `json:"priority"` + Summary string `json:"summary"` +} + +type ImageChange struct { + Name string + From, To imagereference.DockerImageReference +} + +type CodeChange struct { + Repo string + From, To string + + ImagesAffected []string +} + +func (c CodeChange) FromShort() string { + if len(c.From) > 8 { + return c.From[:8] + } + return c.From +} + +func (c CodeChange) ToShort() string { + if len(c.To) > 8 { + return c.To[:8] + } + return c.To +} + +func commitsForRepo(dir string, change CodeChange) (*url.URL, []MergeCommit, error) { + basePath, err := sourceLocationAsRelativePath(dir, change.Repo) + if err != nil { + return nil, nil, fmt.Errorf("Unable to load change info for %s: %v", change.ImagesAffected[0], err) + } + u, err := sourceLocationAsURL(change.Repo) + if err != nil { + return nil, nil, fmt.Errorf("The source repository cannot be parsed %s: %v", change.Repo, err) + } + g := &git{} + g, err = g.ChangeContext(basePath) + if err != nil { + return nil, nil, fmt.Errorf("%s is not a git repo: %v\n\n", basePath, err) + } + commits, err := mergeLogForRepo(g, change.From, change.To) + if err != nil { + return nil, nil, fmt.Errorf("Could not load commits for %s: %v", change.Repo, err) + } + return u, commits, nil +} + +func releaseDiffContentChanges(diff *ReleaseDiff) ([]CodeChange, []ImageChange, []string) { + var imageChanges []ImageChange + var unexpectedChanges []string + var keys []string + repoToCommitsToImages := make(map[string]map[string][]string) + for k, imageDiff := range diff.ChangedImages { + from, to := imageDiff.From, imageDiff.To + switch { + case from == nil, to == nil: + default: + newRepo := to.Annotations["io.openshift.build.source-location"] + oldCommit, newCommit := from.Annotations["io.openshift.build.commit.id"], to.Annotations["io.openshift.build.commit.id"] + if len(oldCommit) == 0 || oldCommit == newCommit { + if from.From != nil && to.From != nil { + if fromRef, err := imagereference.Parse(from.From.Name); err == nil { + if toRef, err := imagereference.Parse(to.From.Name); err == nil { + if len(fromRef.ID) > 0 && fromRef.ID == toRef.ID { + // no change or only location changed + break + } + imageChanges = append(imageChanges, ImageChange{ + Name: imageDiff.Name, + From: fromRef, + To: toRef, + }) + break + } + } + } + // before or after tag did not have a valid image reference + unexpectedChanges = append(unexpectedChanges, k) + break + } + commitRange, ok := repoToCommitsToImages[newRepo] + if !ok { + commitRange = make(map[string][]string) + repoToCommitsToImages[newRepo] = commitRange + } + rangeID := fmt.Sprintf("%s..%s", oldCommit, newCommit) + commitRange[rangeID] = append(commitRange[rangeID], k) + keys = append(keys, k) + } + } + sort.Slice(imageChanges, func(i, j int) bool { + return imageChanges[i].Name < imageChanges[j].Name + }) + sort.Strings(unexpectedChanges) + sort.Strings(keys) + var codeChanges []CodeChange + for _, key := range keys { + imageDiff := diff.ChangedImages[key] + from, to := imageDiff.From, imageDiff.To + _, newRepo := from.Annotations["io.openshift.build.source-location"], to.Annotations["io.openshift.build.source-location"] + oldCommit, newCommit := from.Annotations["io.openshift.build.commit.id"], to.Annotations["io.openshift.build.commit.id"] + + // only display a given chunk of changes once + commitRange := fmt.Sprintf("%s..%s", oldCommit, newCommit) + allKeys := repoToCommitsToImages[newRepo][commitRange] + if len(allKeys) == 0 { + continue + } + repoToCommitsToImages[newRepo][commitRange] = nil + sort.Strings(allKeys) + + codeChanges = append(codeChanges, CodeChange{ + Repo: newRepo, + From: oldCommit, + To: newCommit, + ImagesAffected: allKeys, + }) + } + return codeChanges, imageChanges, unexpectedChanges +} + +func refToShortDescription(ref *imageapi.TagReference) string { + if from := ref.From; from != nil { + imageRef, err := imagereference.Parse(from.Name) + if err == nil { + switch { + case len(imageRef.ID) > 0: + return fmt.Sprintf("%s `%s`", ref.Name, imageRef.ID) + case len(imageRef.Tag) > 0: + return fmt.Sprintf("%s `:%s`", ref.Name, imageRef.Tag) + default: + return fmt.Sprintf("%s `%s`", ref.Name, imageRef.Exact()) + } + } + return fmt.Sprintf("%s `%s`", ref.Name, from.Name) + } + return ref.Name +} + +type MergeCommit struct { + CommitDate time.Time + + Commit string + ParentCommits []string + + PullRequest int + Bug int + + Subject string +} + +func gitOutputToError(err error, out string) error { + out = strings.TrimSpace(out) + if strings.HasPrefix(out, "fatal: ") { + out = strings.TrimPrefix(out, "fatal: ") + } + if len(out) == 0 { + return err + } + return fmt.Errorf(out) +} + +func mergeLogForRepo(g *git, from, to string) ([]MergeCommit, error) { + if from == to { + return nil, nil + } + + rePR, err := regexp.Compile(`^Merge pull request #(\d+) from`) + if err != nil { + return nil, err + } + reBug, err := regexp.Compile(`^Bug (\d+)\s*(-|:)\s*`) + if err != nil { + return nil, err + } + + args := []string{"log", "--merges", "--topo-order", "-z", "--pretty=format:%H %P%x1E%ct%x1E%s%x1E%b", "--reverse", fmt.Sprintf("%s..%s", from, to)} + out, err := g.exec(args...) + if err != nil { + // retry once if there's a chance we haven't fetched the latest commits + if !strings.Contains(out, "Invalid revision range") { + return nil, gitOutputToError(err, out) + } + if _, err := g.exec("fetch", "--all"); err != nil { + return nil, gitOutputToError(err, out) + } + if _, err := g.exec("show-ref", from); err != nil { + return nil, fmt.Errorf("from commit %s does not exist", from) + } + if _, err := g.exec("show-ref", to); err != nil { + return nil, fmt.Errorf("to commit %s does not exist", to) + } + out, err = g.exec(args...) + if err != nil { + return nil, gitOutputToError(err, out) + } + } + + if glog.V(5) { + glog.Infof("Got commit info:\n%s", strconv.Quote(out)) + } + + var commits []MergeCommit + for _, entry := range strings.Split(out, "\x00") { + records := strings.Split(entry, "\x1e") + if len(records) != 4 { + return nil, fmt.Errorf("unexpected git log output width %d columns", len(records)) + } + unixTS, err := strconv.ParseInt(records[1], 10, 64) + if err != nil { + return nil, fmt.Errorf("unexpected timestamp: %v", err) + } + commitValues := strings.Split(records[0], " ") + + mergeCommit := MergeCommit{ + CommitDate: time.Unix(unixTS, 0).UTC(), + Commit: commitValues[0], + ParentCommits: commitValues[1:], + } + + msg := records[3] + if m := reBug.FindStringSubmatch(msg); m != nil { + mergeCommit.Subject = msg[len(m[0]):] + mergeCommit.Bug, err = strconv.Atoi(m[1]) + if err != nil { + return nil, fmt.Errorf("could not extract bug number from %q: %v", msg, err) + } + } else { + mergeCommit.Subject = msg + } + mergeCommit.Subject = strings.TrimSpace(mergeCommit.Subject) + + 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) + } + } + + commits = append(commits, mergeCommit) + } + + return commits, nil +}