From cacb8ff9c207dafea9e918c0e2c4151076e1b5c6 Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Wed, 13 Sep 2017 18:32:14 +0300 Subject: [PATCH 1/2] add SourceURLsForPath to SourceManager ineterface to expose the deduced possible source URLs for a given import path. This information is useful for importing configuration from other tools that support vendoring forks --- internal/gps/deduce.go | 16 +++++++--------- internal/gps/deduce_test.go | 30 +++++++++++++++++------------ internal/gps/maybe_source.go | 33 +++++++++++++++++--------------- internal/gps/solve_basic_test.go | 5 +++++ internal/gps/source_manager.go | 15 +++++++++++++++ 5 files changed, 63 insertions(+), 36 deletions(-) diff --git a/internal/gps/deduce.go b/internal/gps/deduce.go index 9b4e49e08d..1033498f36 100644 --- a/internal/gps/deduce.go +++ b/internal/gps/deduce.go @@ -21,10 +21,11 @@ import ( ) var ( - gitSchemes = []string{"https", "ssh", "git", "http"} - bzrSchemes = []string{"https", "bzr+ssh", "bzr", "http"} - hgSchemes = []string{"https", "ssh", "http"} - svnSchemes = []string{"https", "http", "svn", "svn+ssh"} + gitSchemes = []string{"https", "ssh", "git", "http"} + bzrSchemes = []string{"https", "bzr+ssh", "bzr", "http"} + hgSchemes = []string{"https", "ssh", "http"} + svnSchemes = []string{"https", "http", "svn", "svn+ssh"} + gopkginSchemes = []string{"https", "http"} ) const gopkgUnstableSuffix = "-unstable" @@ -292,12 +293,9 @@ func (m gopkginDeducer) deduceSource(p string, u *url.URL) (maybeSource, error) return nil, fmt.Errorf("could not parse %q as a gopkg.in major version", majorStr[1:]) } - mb := make(maybeSources, len(gitSchemes)) - for k, scheme := range gitSchemes { + mb := make(maybeSources, len(gopkginSchemes)) + for k, scheme := range gopkginSchemes { u2 := *u - if scheme == "ssh" { - u2.User = url.User("git") - } u2.Scheme = scheme mb[k] = maybeGopkginSource{ opath: v[1], diff --git a/internal/gps/deduce_test.go b/internal/gps/deduce_test.go index 961fcf7e8f..5002b49eb4 100644 --- a/internal/gps/deduce_test.go +++ b/internal/gps/deduce_test.go @@ -132,8 +132,6 @@ var pathDeductionFixtures = map[string][]pathDeductionFixture{ root: "gopkg.in/sdboyer/gps.v0", mb: maybeSources{ maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("https://github.com/sdboyer/gps"), major: 0}, - maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("ssh://git@github.com/sdboyer/gps"), major: 0}, - maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("git://github.com/sdboyer/gps"), major: 0}, maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("http://github.com/sdboyer/gps"), major: 0}, }, }, @@ -142,8 +140,6 @@ var pathDeductionFixtures = map[string][]pathDeductionFixture{ root: "gopkg.in/sdboyer/gps.v0", mb: maybeSources{ maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("https://github.com/sdboyer/gps"), major: 0}, - maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("ssh://git@github.com/sdboyer/gps"), major: 0}, - maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("git://github.com/sdboyer/gps"), major: 0}, maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v0", url: mkurl("http://github.com/sdboyer/gps"), major: 0}, }, }, @@ -152,8 +148,6 @@ var pathDeductionFixtures = map[string][]pathDeductionFixture{ root: "gopkg.in/sdboyer/gps.v1", mb: maybeSources{ maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v1", url: mkurl("https://github.com/sdboyer/gps"), major: 1}, - maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v1", url: mkurl("ssh://git@github.com/sdboyer/gps"), major: 1}, - maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v1", url: mkurl("git://github.com/sdboyer/gps"), major: 1}, maybeGopkginSource{opath: "gopkg.in/sdboyer/gps.v1", url: mkurl("http://github.com/sdboyer/gps"), major: 1}, }, }, @@ -162,8 +156,6 @@ var pathDeductionFixtures = map[string][]pathDeductionFixture{ root: "gopkg.in/yaml.v1", mb: maybeSources{ maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("https://github.com/go-yaml/yaml"), major: 1}, - maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("ssh://git@github.com/go-yaml/yaml"), major: 1}, - maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("git://github.com/go-yaml/yaml"), major: 1}, maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("http://github.com/go-yaml/yaml"), major: 1}, }, }, @@ -172,8 +164,6 @@ var pathDeductionFixtures = map[string][]pathDeductionFixture{ root: "gopkg.in/yaml.v1", mb: maybeSources{ maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("https://github.com/go-yaml/yaml"), major: 1}, - maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("ssh://git@github.com/go-yaml/yaml"), major: 1}, - maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("git://github.com/go-yaml/yaml"), major: 1}, maybeGopkginSource{opath: "gopkg.in/yaml.v1", url: mkurl("http://github.com/go-yaml/yaml"), major: 1}, }, }, @@ -182,8 +172,6 @@ var pathDeductionFixtures = map[string][]pathDeductionFixture{ root: "gopkg.in/inf.v0", mb: maybeSources{ maybeGopkginSource{opath: "gopkg.in/inf.v0", url: mkurl("https://github.com/go-inf/inf"), major: 0}, - maybeGopkginSource{opath: "gopkg.in/inf.v0", url: mkurl("ssh://git@github.com/go-inf/inf"), major: 0}, - maybeGopkginSource{opath: "gopkg.in/inf.v0", url: mkurl("git://github.com/go-inf/inf"), major: 0}, maybeGopkginSource{opath: "gopkg.in/inf.v0", url: mkurl("http://github.com/go-inf/inf"), major: 0}, }, }, @@ -582,6 +570,11 @@ func TestDeduceFromPath(t *testing.T) { } else { t.Errorf("Deducer did not return expected source:\n\t(GOT) %s\n\t(WNT) %s", printmb(mb, t), printmb(fix.mb, t)) } + } else { + gotURLs, wantURLs := mb.possibleURLs(), fix.mb.possibleURLs() + if !reflect.DeepEqual(gotURLs, wantURLs) { + t.Errorf("Deducer did not return expected source:\n\t(GOT) %s\n\t(WNT) %s", gotURLs, wantURLs) + } } }) } @@ -634,6 +627,19 @@ func TestVanityDeduction(t *testing.T) { if goturl != wanturl { t.Errorf("Deduced repo ident does not match fixture:\n\t(GOT) %s\n\t(WNT) %s", goturl, wanturl) } + + urls, err := sm.SourceURLsForPath(fix.in) + if err != nil { + t.Errorf("Unexpected err on deducing source urls: %s", err) + return + } + if len(urls) != 1 { + t.Errorf("Deduced source URLs count for a vanity import should be 1, got %d", len(urls)) + } + goturl = urls[0].String() + if goturl != wanturl { + t.Errorf("Deduced source URL does not match fixture:\n\t(GOT) %s\n\t(WNT) %s", goturl, wanturl) + } }) } } diff --git a/internal/gps/maybe_source.go b/internal/gps/maybe_source.go index 07c2cb502b..15bfe98c3b 100644 --- a/internal/gps/maybe_source.go +++ b/internal/gps/maybe_source.go @@ -10,7 +10,6 @@ import ( "fmt" "net/url" "path/filepath" - "strings" "github.com/Masterminds/vcs" "github.com/pkg/errors" @@ -25,7 +24,7 @@ import ( // * Makes it easy to attempt multiple URLs for a given import path type maybeSource interface { try(ctx context.Context, cachedir string, c singleSourceCache, superv *supervisor) (source, sourceState, error) - getURL() string + possibleURLs() []*url.URL } type errorSlice []error @@ -47,7 +46,11 @@ func (mbs maybeSources) try(ctx context.Context, cachedir string, c singleSource if err == nil { return src, state, nil } - errs = append(errs, errors.Wrapf(err, "failed to set up %q", mb.getURL())) + urls := "" + for _, url := range mb.possibleURLs() { + urls += url.String() + "\n" + } + errs = append(errs, errors.Wrapf(err, "failed to set up %q", urls)) } return nil, 0, errors.Wrap(&errs, "no valid source could be created") @@ -56,12 +59,12 @@ func (mbs maybeSources) try(ctx context.Context, cachedir string, c singleSource // This really isn't generally intended to be used - the interface is for // maybeSources to be able to interrogate its members, not other things to // interrogate a maybeSources. -func (mbs maybeSources) getURL() string { - strslice := make([]string, 0, len(mbs)) +func (mbs maybeSources) possibleURLs() []*url.URL { + urlslice := make([]*url.URL, 0, len(mbs)) for _, mb := range mbs { - strslice = append(strslice, mb.getURL()) + urlslice = append(urlslice, mb.possibleURLs()...) } - return strings.Join(strslice, "\n") + return urlslice } // sourceCachePath returns a url-sanitized source cache dir path. @@ -107,8 +110,8 @@ func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSource return src, state, nil } -func (m maybeGitSource) getURL() string { - return m.url.String() +func (m maybeGitSource) possibleURLs() []*url.URL { + return []*url.URL{m.url} } type maybeGopkginSource struct { @@ -168,8 +171,8 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo return src, state, nil } -func (m maybeGopkginSource) getURL() string { - return m.opath +func (m maybeGopkginSource) possibleURLs() []*url.URL { + return []*url.URL{m.url} } type maybeBzrSource struct { @@ -207,8 +210,8 @@ func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSource return src, state, nil } -func (m maybeBzrSource) getURL() string { - return m.url.String() +func (m maybeBzrSource) possibleURLs() []*url.URL { + return []*url.URL{m.url} } type maybeHgSource struct { @@ -246,6 +249,6 @@ func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceC return src, state, nil } -func (m maybeHgSource) getURL() string { - return m.url.String() +func (m maybeHgSource) possibleURLs() []*url.URL { + return []*url.URL{m.url} } diff --git a/internal/gps/solve_basic_test.go b/internal/gps/solve_basic_test.go index 14403e3614..064a798fbf 100644 --- a/internal/gps/solve_basic_test.go +++ b/internal/gps/solve_basic_test.go @@ -7,6 +7,7 @@ package gps import ( "context" "fmt" + "net/url" "regexp" "strings" @@ -1475,6 +1476,10 @@ func (sm *depspecSourceManager) DeduceProjectRoot(ip string) (ProjectRoot, error return "", fmt.Errorf("Could not find %s, or any parent, in list of known fixtures", ip) } +func (sm *depspecSourceManager) SourceURLsForPath(ip string) ([]*url.URL, error) { + return nil, fmt.Errorf("dummy sm doesn't implement SourceURLsForPath") +} + func (sm *depspecSourceManager) rootSpec() depspec { return sm.specs[0] } diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index c1d23f3687..7380cd6c3c 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -9,6 +9,7 @@ import ( "fmt" "io/ioutil" "log" + "net/url" "os" "os/signal" "path/filepath" @@ -71,6 +72,9 @@ type SourceManager interface { // project/source root. DeduceProjectRoot(ip string) (ProjectRoot, error) + // SourceURLsForPath takes an import path and deduces the possible source URLs + SourceURLsForPath(ip string) ([]*url.URL, error) + // Release lets go of any locks held by the SourceManager. Once called, it is // no longer safe to call methods against it; all method calls will // immediately result in errors. @@ -536,6 +540,17 @@ func (sm *SourceMgr) InferConstraint(s string, pi ProjectIdentifier) (Constraint return nil, errors.Errorf("%s is not a valid version for the package %s(%s)", s, pi.ProjectRoot, pi.Source) } +// SourceURLsForPath takes an import path, deduces it's root path, +// and returns a list of possible souce URLs for fetching it +func (sm *SourceMgr) SourceURLsForPath(ip string) ([]*url.URL, error) { + deduced, err := sm.deduceCoord.deduceRootPath(context.TODO(), ip) + if err != nil { + return nil, err + } + + return deduced.mb.possibleURLs(), nil +} + // disambiguateRevision looks up a revision in the underlying source, spitting // it back out in an unabbreviated, disambiguated form. // From 348b8694c175b8b37fca4f1d34735d188ff89eb4 Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Mon, 18 Sep 2017 10:32:03 +0300 Subject: [PATCH 2/2] improve doc for SourceURLsForPath() and an error msg in `maybeSources.try()` --- internal/gps/maybe_source.go | 2 +- internal/gps/source_manager.go | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/gps/maybe_source.go b/internal/gps/maybe_source.go index 15bfe98c3b..f5f3f69e27 100644 --- a/internal/gps/maybe_source.go +++ b/internal/gps/maybe_source.go @@ -50,7 +50,7 @@ func (mbs maybeSources) try(ctx context.Context, cachedir string, c singleSource for _, url := range mb.possibleURLs() { urls += url.String() + "\n" } - errs = append(errs, errors.Wrapf(err, "failed to set up %q", urls)) + errs = append(errs, errors.Wrapf(err, "failed to set up sources from the following URLs:\n%s", urls)) } return nil, 0, errors.Wrap(&errs, "no valid source could be created") diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 7380cd6c3c..0e885414c0 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -72,7 +72,9 @@ type SourceManager interface { // project/source root. DeduceProjectRoot(ip string) (ProjectRoot, error) - // SourceURLsForPath takes an import path and deduces the possible source URLs + // SourceURLsForPath takes an import path and deduces the set of source URLs + // that may refer to a canonical upstream source. + // In general, these URLs differ only by protocol (e.g. https vs. ssh), not path SourceURLsForPath(ip string) ([]*url.URL, error) // Release lets go of any locks held by the SourceManager. Once called, it is @@ -540,8 +542,9 @@ func (sm *SourceMgr) InferConstraint(s string, pi ProjectIdentifier) (Constraint return nil, errors.Errorf("%s is not a valid version for the package %s(%s)", s, pi.ProjectRoot, pi.Source) } -// SourceURLsForPath takes an import path, deduces it's root path, -// and returns a list of possible souce URLs for fetching it +// SourceURLsForPath takes an import path and deduces the set of source URLs +// that may refer to a canonical upstream source. +// In general, these URLs differ only by protocol (e.g. https vs. ssh), not path func (sm *SourceMgr) SourceURLsForPath(ip string) ([]*url.URL, error) { deduced, err := sm.deduceCoord.deduceRootPath(context.TODO(), ip) if err != nil {