add SourceURLsForPath() to SourceManager interface #1166
Conversation
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
michael-go
changed the title
|
CodeClimate doesn't like me using |
|
Don't worry about the code climate error, we can ignore it when we merge. |
internal/gps/maybe_source.go
Outdated
| for _, url := range mb.possibleURLs() { | ||
| urls += url.String() + "\n" | ||
| } | ||
| errs = append(errs, errors.Wrapf(err, "failed to set up %q", urls)) |
There was a problem hiding this comment.
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.)
internal/gps/source_manager.go
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
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."
|
Thanks @sdboyer, fixed the comment & error string |
internal/gps/maybe_source.go
Outdated
| for _, url := range mb.possibleURLs() { | ||
| urls += url.String() + "\n" | ||
| } | ||
| errs = append(errs, errors.Wrapf(err, "failed to set sources from the following URLs:\n%s", urls)) |
There was a problem hiding this comment.
🤦♂️
i fatfingered that. i meant "failed to set up sources from the following URLs:\n" - sorry!
|
Thanks @sdboyer - fixed the typo |
and an error msg in `maybeSources.try()`
michael-go
force-pushed
the
source-manager-expose-deduced-sources
branch
from
450d24f
to
348b869
Compare
darkowlzz
changed the title
What does this do / why do we need it?
This exposes all possible source URLs that are deduced for a given import path.
This PR was created to accommodate #1149 (
gvtimporter), following @sdboyer 's advice.gvtallows the user to specify an alternate url (e.g. fork) for a vendored import, via theRepositoryfield in the manifest. However, all packages that are listed in the manifest have theRepositoryfield even if it points to the original source URL.gpsalready has a complex logic of deducing the source URLs given an import, however the actual URLs are not exposed to the callers of SourceManager. The new exportedSourceURLsForPath()will allow the importers to understand if the a URL is a not a default one, and only in such a case, pass it as a constraint.This change required a slight modification to
gps's internalmaybeSourceinterface and some refactoring to the various implementations: thegetURL() stringmethod (which wasn't called anywhere) was changed topossibleURLs() []*url.URL. In addition a small change was done to thegopkg.indeducer.