changelog: Remove repo-ID
Use repo path instead of repo-id (repo-name + repo-path) to match repositories for comparison. This allows repositories with 2 different names but the same path to be directly compared instead of treating them as separate repos and grouping them together in the webapp as additions and removals. Also fixed case where an empty changelog is displayed if an error occurs after calling additions().
BUG=b/160901711
TEST=unittests, run local
Change-Id: I888b34d54b297d59bf84cac550fcd4c8681c912b
diff --git a/src/pkg/changelog/changelog.go b/src/pkg/changelog/changelog.go
index 265e399..3193619 100644
--- a/src/pkg/changelog/changelog.go
+++ b/src/pkg/changelog/changelog.go
@@ -108,16 +108,6 @@
return buildNum
}
-// Creates a unique identifier for a repo + branch pairing.
-// Path is used instead of dest-branch because some manifest files do not
-// indicate a dest-branch for a project.
-// Path itself is not sufficient to guarantee uniqueness, since some repos
-// share the same path.
-// ex. mirrors/cros/chromiumos/repohooks vs cos/repohooks
-func repoID(name, path string) string {
- return name + path
-}
-
// limitPageSize will restrict a request page size to min of pageSize (which grows exponentially)
// or remaining request size
func limitPageSize(pageSize, requestedSize int) int {
@@ -187,7 +177,7 @@
repos := make(map[string]*repo)
for _, project := range root.SelectElements("project") {
name, path := project.SelectAttr("name").Value, project.SelectAttrValue("path", "")
- repos[repoID(name, path)] = &repo{
+ repos[path] = &repo{
Repo: name,
Path: path,
InstanceURL: remoteMap[project.SelectAttrValue("remote", "")],
@@ -229,11 +219,24 @@
log.Debugf("Fetching changelog for repo: %s on committish %s\n", req.Repo, req.Committish)
commits, hasMoreCommits, err := utils.Commits(req.Client, req.Repo, req.Committish, req.Ancestor, req.QuerySize)
if err != nil {
- req.OutputChan <- commitsResult{Err: utils.InternalServerError}
+ if utils.GitilesErrCode(err) == "404" {
+ req.OutputChan <- commitsResult{
+ InstanceURL: req.InstanceURL,
+ Path: req.Path,
+ Repo: req.Repo,
+ }
+ } else {
+ log.Errorf("commits: error retrieving commit changelog on repo %s from commit %s to commit %s:\n%v", req.Repo, req.Committish, req.Ancestor, err)
+ req.OutputChan <- commitsResult{Err: utils.InternalServerError}
+ }
+ return
+ }
+ if commits == nil {
+ log.Info(req.Repo, req.Committish, req.Ancestor)
}
parsedCommits, err := ParseGitCommitLog(commits)
if err != nil {
- log.Errorf("commits: Error parsing Gitiles commits response\n%v", err)
+ log.Errorf("commits: error parsing Gitiles commits response\n%v", err)
req.OutputChan <- commitsResult{Err: utils.InternalServerError}
return
}
@@ -279,7 +282,7 @@
return
}
var sourceSHA string
- if sourceData, ok := sourceRepos[repoID(res.Repo, res.Path)]; ok {
+ if sourceData, ok := sourceRepos[res.Path]; ok {
sourceSHA = sourceData.Committish
}
if len(res.Commits) > 0 {
@@ -289,7 +292,7 @@
InstanceURL: res.InstanceURL,
Repo: res.Repo,
SourceSHA: sourceSHA,
- TargetSHA: targetRepos[repoID(res.Repo, res.Path)].Committish,
+ TargetSHA: targetRepos[res.Path].Committish,
}
}
}
@@ -361,11 +364,11 @@
go additions(clients, targetRepos, sourceRepos, querySize, missChan)
missRes := <-missChan
if missRes.Err != nil {
- return nil, nil, err
+ return nil, nil, missRes.Err
}
addRes := <-addChan
if addRes.Err != nil {
- return nil, nil, err
+ return nil, nil, addRes.Err
}
return addRes.Additions, missRes.Additions, nil
diff --git a/src/pkg/changelog/changelog_test.go b/src/pkg/changelog/changelog_test.go
index 8a17a8a..a555e7a 100644
--- a/src/pkg/changelog/changelog_test.go
+++ b/src/pkg/changelog/changelog_test.go
@@ -360,8 +360,8 @@
additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", querySize)
if err != nil {
t.Errorf("Changelog failed, expected no error, got %v", err)
- } else if len(additions) == 0 {
- t.Errorf("Changelog failed, expected non-empty additions, got %v", additions)
+ } else if len(additions) != 0 {
+ t.Errorf("Changelog failed, expected empty additions, got %v", additions)
} else if len(removals) == 0 {
t.Errorf("Changelog failed, expected non-empty removals, got %v", removals)
}
