From 1ebc40ebcc9673fbd081c4128c90fda11c58eb3c Mon Sep 17 00:00:00 2001 From: Dmitri Goutnik Date: Fri, 1 Nov 2019 11:55:48 -0500 Subject: [PATCH] ensure unique GH_PROJECT/GH_TAGNAME (ports 241637) --- apis/apis.go | 27 ++++++++++++ apis/github.go | 28 +++++++++++++ apis/github_test.go | 24 +++++++++++ apis/gitlab.go | 28 +++++++++++++ {gitlab => apis}/gitlab_test.go | 12 +++--- gitlab/gitlab.go | 50 ---------------------- go.mod | 2 +- tuple/parser.go | 15 ++++++- tuple/tuple.go | 73 ++++++++++++++++++++++++++++++--- tuple/tuple_online_test.go | 28 +++++++++++++ tuple/tuple_test.go | 6 +-- 11 files changed, 225 insertions(+), 68 deletions(-) create mode 100644 apis/apis.go create mode 100644 apis/github.go create mode 100644 apis/github_test.go create mode 100644 apis/gitlab.go rename {gitlab => apis}/gitlab_test.go (58%) delete mode 100644 gitlab/gitlab.go create mode 100644 tuple/tuple_online_test.go diff --git a/apis/apis.go b/apis/apis.go new file mode 100644 index 0000000..d2f0b70 --- /dev/null +++ b/apis/apis.go @@ -0,0 +1,27 @@ +package apis + +import ( + "fmt" + "io/ioutil" + "net/http" +) + +func get(url string) ([]byte, error) { + resp, err := http.Get(url) + if err != nil { + return nil, fmt.Errorf("GET %s: %v", url, err) + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusOK: + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("GET %s: %v", url, err) + } + return body, nil + default: + body, _ := ioutil.ReadAll(resp.Body) + return nil, fmt.Errorf("GET %s: %d, body: %v", url, resp.StatusCode, string(body)) + } +} diff --git a/apis/github.go b/apis/github.go new file mode 100644 index 0000000..e986f42 --- /dev/null +++ b/apis/github.go @@ -0,0 +1,28 @@ +package apis + +import ( + "encoding/json" + "fmt" + "net/url" +) + +type GithubCommit struct { + ID string `json:"sha"` +} + +func GetGithubCommit(account, project, ref string) (*GithubCommit, error) { + projectID := fmt.Sprintf("%s/%s", url.PathEscape(account), url.PathEscape(project)) + url := fmt.Sprintf("https://api.github.com/repos/%s/commits/%s", projectID, ref) + + resp, err := get(url) + if err != nil { + return nil, fmt.Errorf("error getting commit %s for %s/%s: %v", ref, account, project, err) + } + + var ret GithubCommit + if err := json.Unmarshal(resp, &ret); err != nil { + return nil, fmt.Errorf("error unmarshalling: %v, resp: %v", err, string(resp)) + } + + return &ret, nil +} diff --git a/apis/github_test.go b/apis/github_test.go new file mode 100644 index 0000000..137f5f2 --- /dev/null +++ b/apis/github_test.go @@ -0,0 +1,24 @@ +// +build online + +package apis + +import "testing" + +func TestGetGithubCommit(t *testing.T) { + examples := []struct { + site, account, project, ref, ID string + }{ + {"https://gitlab.com", "dmgk", "modules2tuple", "v1.9.0", "fc09878b93db35aafc74311f7ea6684ac08a3b83"}, + {"https://gitlab.com", "dmgk", "modules2tuple", "a0cdb416ca2c", "a0cdb416ca2cbf6d3dad67a97f4fdcfac954503e"}, + } + + for i, x := range examples { + c, err := GetGithubCommit(x.account, x.project, x.ref) + if err != nil { + t.Fatal(err) + } + if x.ID != c.ID { + t.Errorf("expected commit ID %s, got %s (example %d)", x.ID, c.ID, i) + } + } +} diff --git a/apis/gitlab.go b/apis/gitlab.go new file mode 100644 index 0000000..3c44062 --- /dev/null +++ b/apis/gitlab.go @@ -0,0 +1,28 @@ +package apis + +import ( + "encoding/json" + "fmt" + "net/url" +) + +type GitlabCommit struct { + ID string `json:"id"` +} + +func GetGitlabCommit(site, account, project, commit string) (*GitlabCommit, error) { + projectID := url.PathEscape(fmt.Sprintf("%s/%s", account, project)) + url := fmt.Sprintf("%s/api/v4/projects/%s/repository/commits/%s", site, projectID, commit) + + resp, err := get(url) + if err != nil { + return nil, fmt.Errorf("error getting commit %s for %s/%s: %v", commit, account, project, err) + } + + var ret GitlabCommit + if err := json.Unmarshal(resp, &ret); err != nil { + return nil, fmt.Errorf("error unmarshalling: %v, resp: %v", err, string(resp)) + } + + return &ret, nil +} diff --git a/gitlab/gitlab_test.go b/apis/gitlab_test.go similarity index 58% rename from gitlab/gitlab_test.go rename to apis/gitlab_test.go index 2569dae..e67cf5b 100644 --- a/gitlab/gitlab_test.go +++ b/apis/gitlab_test.go @@ -1,24 +1,24 @@ // +build online -package gitlab +package apis import "testing" -func TestGetCommit(t *testing.T) { +func TestGetGitlabCommit(t *testing.T) { examples := []struct { - site, account, project, tag, commitID string + site, account, project, ref, ID string }{ {"https://gitlab.com", "gitlab-org", "gitaly-proto", "v1.32.0", "f4db5d05d437abe1154d7308ca044d3577b5ccba"}, {"https://gitlab.com", "gitlab-org", "labkit", "0c3fc7cdd57c", "0c3fc7cdd57c57da5ab474aa72b6640d2bdc9ebb"}, } for i, x := range examples { - c, err := GetCommit(x.site, x.account, x.project, x.tag) + c, err := GetGitlabCommit(x.site, x.account, x.project, x.ref) if err != nil { t.Fatal(err) } - if x.commitID != c.ID { - t.Errorf("expected commitID %s, got %s (example %d)", x.commitID, c.ID, i) + if x.ID != c.ID { + t.Errorf("expected commit ID %s, got %s (example %d)", x.ID, c.ID, i) } } } diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go deleted file mode 100644 index 6ea68a0..0000000 --- a/gitlab/gitlab.go +++ /dev/null @@ -1,50 +0,0 @@ -package gitlab - -import ( - "encoding/json" - "fmt" - "io/ioutil" - "net/http" - "net/url" -) - -type Commit struct { - ID string `json:"id"` -} - -func GetCommit(site, account, project, commit string) (*Commit, error) { - projectID := url.PathEscape(fmt.Sprintf("%s/%s", account, project)) - url := fmt.Sprintf("%s/api/v4/projects/%s/repository/commits/%s", site, projectID, commit) - - resp, err := get(url) - if err != nil { - return nil, fmt.Errorf("error getting commit %s for %s/%s: %v", commit, account, project, err) - } - - var ret Commit - if err := json.Unmarshal(resp, &ret); err != nil { - return nil, fmt.Errorf("error unmarshalling: %v, resp: %v", err, string(resp)) - } - - return &ret, nil -} - -func get(url string) ([]byte, error) { - resp, err := http.Get(url) - if err != nil { - return nil, fmt.Errorf("GET %s: %v", url, err) - } - defer resp.Body.Close() - - switch resp.StatusCode { - case http.StatusOK: - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("GET %s: %v", url, err) - } - return body, nil - default: - body, _ := ioutil.ReadAll(resp.Body) - return nil, fmt.Errorf("GET %s: %d, body: %v", url, resp.StatusCode, string(body)) - } -} diff --git a/go.mod b/go.mod index ac7e330..9e35f71 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/dmgk/modules2tuple -go 1.12 +go 1.13 diff --git a/tuple/parser.go b/tuple/parser.go index 130f05e..64ec32e 100644 --- a/tuple/parser.go +++ b/tuple/parser.go @@ -9,7 +9,7 @@ import ( "strings" "sync" - "github.com/dmgk/modules2tuple/gitlab" + "github.com/dmgk/modules2tuple/apis" ) type Parser struct { @@ -53,7 +53,7 @@ func (p *Parser) Read(r io.Reader) (Tuples, error) { // Call Gitlab API to translate go.mod short commit IDs and tags // to the full 40-character commit IDs as required by bsd.sites.mk if _, ok := t.Source.(GL); ok { - c, err := gitlab.GetCommit(t.Source.Site(), t.Account, t.Project, t.Tag) + c, err := apis.GetGitlabCommit(t.Source.Site(), t.Account, t.Project, t.Tag) if err != nil { ch <- err return @@ -88,7 +88,18 @@ func (p *Parser) Read(r io.Reader) (Tuples, error) { } } sort.Sort(ByAccountAndProject(tuples)) + tuples.EnsureUniqueGroups() + if !p.offline { + if err := tuples.EnsureUniqueGithubProjectAndTag(); err != nil { + switch err := err.(type) { + case DuplicateProjectAndTag: + errors.DuplicateProjectAndTag = append(errors.DuplicateProjectAndTag, err) + default: + return nil, err + } + } + } if errors.Any() { return tuples, errors diff --git a/tuple/tuple.go b/tuple/tuple.go index 61c71a6..ab36403 100644 --- a/tuple/tuple.go +++ b/tuple/tuple.go @@ -2,11 +2,14 @@ package tuple import ( "bytes" + "errors" "fmt" "reflect" "regexp" "sort" "strings" + + "github.com/dmgk/modules2tuple/apis" ) type Tuple struct { @@ -38,8 +41,8 @@ func (tt Tuples) EnsureUniqueGroups() { var prevGroup string suffix := 1 - for i, t := range tt { - if i == 0 { + for _, t := range tt { + if prevGroup == "" { prevGroup = t.Group continue } @@ -53,6 +56,48 @@ func (tt Tuples) EnsureUniqueGroups() { } } +// EnsureUniqueGithubProjectAndTag checks that tuples have a unique GH_PROJECT/GH_TAGNAME +// combination. Due to the way Github prepares release tarballs and th way ports framework +// works, tuples sharing GH_PROJECT/GH_TAGNAME pair will be extracted to the same directory. +// Try avoiding this mess by switching one of the conflicting tuple's GH_TAGNAME from git tag +// to git commit ID. +// This function assumes that tt is pre-sorted in ByAccountAndProject order. +func (tt Tuples) EnsureUniqueGithubProjectAndTag() error { + if len(tt) < 2 { + return nil + } + + var prevTuple *Tuple + + for _, t := range tt { + if _, ok := t.Source.(GH); !ok { + // not a Github tuple, skip + continue + } + + if prevTuple == nil { + prevTuple = t + continue + } + + if t.Account != prevTuple.Account { + // different Account, but the same Project and Tag + if t.Project == prevTuple.Project && t.Tag == prevTuple.Tag { + c, err := apis.GetGithubCommit(t.Account, t.Project, t.Tag) + if err != nil { + return DuplicateProjectAndTag(t.String()) + } + if len(c.ID) < 12 { + return errors.New("unexpectedly short commit ID") + } + t.Tag = c.ID[:12] + } + } + } + + return nil +} + type ByAccountAndProject Tuples func (tt ByAccountAndProject) Len() int { @@ -116,12 +161,14 @@ type Errors struct { Source []SourceError ReplacementLocalFilesystem []ReplacementLocalFilesystemError ReplacementMissingCommit []ReplacementMissingCommitError + DuplicateProjectAndTag []DuplicateProjectAndTag } func (ee Errors) Any() bool { return len(ee.Source) > 0 || len(ee.ReplacementLocalFilesystem) > 0 || - len(ee.ReplacementMissingCommit) > 0 + len(ee.ReplacementMissingCommit) > 0 || + len(ee.DuplicateProjectAndTag) > 0 } func (ee Errors) Error() string { @@ -157,9 +204,25 @@ func (ee Errors) Error() string { } } + if len(ee.DuplicateProjectAndTag) > 0 { + buf.WriteString("\t\t# The following tuple has duplicate GH_PROJECT/GH_TAGNAME combinations and an attempt to fix it using Github API failed:\n") + sort.Slice(ee.DuplicateProjectAndTag, func(i, j int) bool { + return ee.DuplicateProjectAndTag[i] < ee.DuplicateProjectAndTag[j] + }) + for _, err := range ee.DuplicateProjectAndTag { + buf.WriteString(fmt.Sprintf("\t\t#\t%s\n", string(err))) + } + } + return buf.String() } +type SourceError string + +func (err SourceError) Error() string { + return string(err) +} + type ReplacementLocalFilesystemError string func (err ReplacementLocalFilesystemError) Error() string { @@ -172,9 +235,9 @@ func (err ReplacementMissingCommitError) Error() string { return string(err) } -type SourceError string +type DuplicateProjectAndTag string -func (err SourceError) Error() string { +func (err DuplicateProjectAndTag) Error() string { return string(err) } diff --git a/tuple/tuple_online_test.go b/tuple/tuple_online_test.go new file mode 100644 index 0000000..e86de47 --- /dev/null +++ b/tuple/tuple_online_test.go @@ -0,0 +1,28 @@ +// +build online + +package tuple + +import ( + "strings" + "testing" +) + +func TestUniqueProjectAndTag(t *testing.T) { + given := ` +# github.com/json-iterator/go v1.1.7 +# github.com/ugorji/go v1.1.7` + + expected := `GH_TUPLE= \ + json-iterator:go:v1.1.7:json_iterator_go/vendor/github.com/json-iterator/go \ + ugorji:go:23ab95ef5dc3:ugorji_go/vendor/github.com/ugorji/go +` + + tt, err := NewParser("vendor", false).Read(strings.NewReader(given)) + if err != nil { + t.Fatal(err) + } + out := tt.String() + if out != expected { + t.Errorf("expected output %s, got %s", expected, out) + } +} diff --git a/tuple/tuple_test.go b/tuple/tuple_test.go index ebb6cec..25f05d0 100644 --- a/tuple/tuple_test.go +++ b/tuple/tuple_test.go @@ -175,8 +175,7 @@ gitlab.com/gitlab-org/labkit/correlation # gitlab.com/gitlab-org/gitaly-proto v1.32.0 gitlab.com/gitlab-org/gitaly-proto/go/gitalypb # github.com/golang/lint v0.0.0-20190409202823-959b441ac422 => golang.org/x/lint v0.0.0-20190409202823-959b441ac422 -# github.com/ugorji/go v1.1.4 => github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43 -` +# github.com/ugorji/go v1.1.4 => github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43` expected := `GH_TUPLE= \ cockroachdb:cockroach-go:e0a95dfd547c:cockroachdb_cockroach_go/vendor/github.com/cockroachdb/cockroach-go \ @@ -210,8 +209,7 @@ func TestUniqueGroups(t *testing.T) { # github.com/minio/mc v0.0.0-20190924013003-643835013047 # github.com/minio/minio-go v0.0.0-20190327203652-5325257a208f # github.com/minio/minio-go/v6 v6.0.39 -# github.com/minio/parquet-go v0.0.0-20190318185229-9d767baf1679 -` +# github.com/minio/parquet-go v0.0.0-20190318185229-9d767baf1679` expected := `GH_TUPLE= \ minio:lsync:v1.0.1:minio_lsync/vendor/github.com/minio/lsync \