fix(gps): support source urls that point directly to go.googlesource.com

[avidal]

This fixes a bug which can be reproduced simply:

$ dep ensure golang.org/x/crypto:go.googlesource.com/crypto
list versions for golang.org/x/crypto(https://go.googlesource.com/crypto): unable to deduce repository and source type for "https://go.googlesource.com/crypto": unable to read metadata: go-import metadata not found

The problem comes up because there was no deducer for
go.googlesource.com, and go.googlesource.com itself does not have any
metatags that can be used to deduce the vcs.

Generally speaking, this bug would not come up because most users would
target golang.org/x/crypto, which does have all of the required
metadata, but it's a bug all the same.

I encountered the bug (and @sdboyer confirmed it) while working on #818.

[ibrasho]

I would change goSource to googlesource for consistency with the other sources.

Otherwise, LGTM.

[avidal]

@ibrasho Yeah I had that at first. Not sure why I changed it.

[sdboyer]

I think we actually need something else here, because there's a bug in the way we handle those source URLs.

Deduction should only really be happening on the import path. Which means the problem here goes back to the same essential problems of #411, where source URLs were being subjected to import path-shaped validation. These paths need to be separated further.

When an alternate source is specified, the system should basically just assume that it's pointing directly at a repository. What's a little hinky here is that we still have to figure out the VCS type, which...well, probably the only easy thing there is to assume it's the same as what was deduced from the import path.

We could merge this, but it should be with a note explicitly saying that it's temporary, and that the real fix is fixing #411. However, I'd prefer that we fix the real problem here, which is better teasing apart how these inputs are validated.

[avidal]

That makes sense, I think. What are your thoughts on deducing just the VCS from a source if it's not otherwise specified? So in cases like go.googlesource.com gps can have code that knows those are git repositories?

Alternatively, it could act like pip, where if you are specifying the source the source must be fully qualified (ie, <vcs>(+<protocol>)?://, such as git+https://go.googlesource.com or git+ssh:// or git://); this could also allow for file:/// as a source.

edit: Although I suppose the alternative option may result in a backwards incompatible change to the manifest and lock files, since it would require a fully specified source; but there is a migration path because any source specified in an existing manifest/lock would be able to be deduced using the original code and then migrated to a fully qualified version.

[sdboyer]

Alternatively, it could act like pip,

iiinteresting.

this could also allow for file:/// as a source

this is kind of a can of worms; i absolutely believe that we need some kind of solution for arbitrary local code paths, but it has the potential to break lockfile portability in nasty, unpredictable ways. ("lockfile portability" is a concept that i need to do a standalone writeup about.)

the rest is good, though - let's just put a pin in this part for now.

edit: Although I suppose the alternative option may result in a backwards incompatible change

i had the same thought - to require a vcs as part of the scheme would be a backwards incompatible change, and is thus off the table to do in a way that invalidates old, vcs-less schemes. But, the only cases where we wouldn't be able to infer the vcs type from the import path are cases where this would have been failing already.

this suggests to me that we could:

1. add support for schemes which specify the vcs type
2. issue warnings when the current project's manifest has source schemes that lack vcs type
3. error immediately if the current project's manifest has a source schemes without vcs types paired with an un-deducible import path

when it finally comes time to jump into the toolchain, we may allow mild BC breaks like this. if so, we can drop support for vcs-less schemes then.

[avidal]

I agree re: file:///, that won't hold well in a lock file for obvious reasons; so I'm more than comfortable skipping it.

Otherwise, it seems like there's enough possibility here to take a stab at an implementation. Do you mind if I take it on? Should I close this PR and start fresh with one aimed at closing #411?

[sdboyer]

@avidal that'd be great! fair warning, this will be pretty involved, and touch a number of things 😄

[sdboyer]

(so, don't hesitate to put up a WIP PR and ask plenty of questions as needed)

[avidal]

🖖 Closing this and opening a WIP PR when I get a chance.