Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

add SourceURLsForPath() to SourceManager interface #1166

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
16 changes: 7 additions & 9 deletions internal/gps/deduce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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],
Expand Down
30 changes: 18 additions & 12 deletions internal/gps/deduce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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://[email protected]/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},
},
},
Expand All @@ -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://[email protected]/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},
},
},
Expand All @@ -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://[email protected]/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},
},
},
Expand All @@ -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://[email protected]/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},
},
},
Expand All @@ -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://[email protected]/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},
},
},
Expand All @@ -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://[email protected]/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},
},
},
Expand Down Expand Up @@ -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)
}
}
})
}
Expand Down Expand Up @@ -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)
}
})
}
}
Expand Down
33 changes: 18 additions & 15 deletions internal/gps/maybe_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"fmt"
"net/url"
"path/filepath"
"strings"

"github.com/Masterminds/vcs"
"github.com/pkg/errors"
Expand All @@ -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
Expand All @@ -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))
Copy link
Member

Choose a reason for hiding this comment

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

nit: let's make an incremental improvement on the output we currently have and change the message to "failed to set sources from the following URLs:\n%s".

(note also, use %s instead of %q, as otherwise we'll end up quoting the whole group of URLs, which i don't think makes a ton of sense.)

}

return nil, 0, errors.Wrap(&errs, "no valid source could be created")
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
}
5 changes: 5 additions & 0 deletions internal/gps/solve_basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package gps
import (
"context"
"fmt"
"net/url"
"regexp"
"strings"

Expand Down Expand Up @@ -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]
}
Expand Down
15 changes: 15 additions & 0 deletions internal/gps/source_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io/ioutil"
"log"
"net/url"
"os"
"os/signal"
"path/filepath"
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

let's be a little more verbose and explanatory with this:

"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
// no longer safe to call methods against it; all method calls will
// immediately result in errors.
Expand Down Expand Up @@ -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.
//
Expand Down