Merge pull request #1132 from sdboyer/gopkgin-separation

Properly separate sources for different gopkg.in versions & github

internal/gps/manager_test.go

@@ -348,6 +348,66 @@ func TestMgrMethodsFailWithBadPath(t *testing.T) {
 	}
 }
 
+type sourceCreationTestFixture struct {
+	roots              []ProjectIdentifier
+	urlcount, srccount int
+}
+
+func (f sourceCreationTestFixture) run(t *testing.T) {
+	t.Parallel()
+	sm, clean := mkNaiveSM(t)
+	defer clean()
+
+	for _, pi := range f.roots {
+		_, err := sm.SourceExists(pi)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if len(sm.srcCoord.nameToURL) != f.urlcount {
+		t.Errorf("want %v names in the name->url map, but got %v. contents: \n%v", f.urlcount, len(sm.srcCoord.nameToURL), sm.srcCoord.nameToURL)
+	}
+
+	if len(sm.srcCoord.srcs) != f.srccount {
+		t.Errorf("want %v gateways in the sources map, but got %v", f.srccount, len(sm.srcCoord.srcs))
+	}
+}
+
+// This test is primarily about making sure that the logic around folding
+// together different ways of referencing the same underlying resource - whether
+// that be intentionally folding them, or intentionally keeping them separate -
+// work as intended.
+func TestSourceCreationCounts(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping slow test in short mode")
+	}
+
+	fixtures := map[string]sourceCreationTestFixture{
+		"gopkgin uniqueness": {
+			roots: []ProjectIdentifier{
+				mkPI("gopkg.in/sdboyer/gpkt.v1"),
+				mkPI("gopkg.in/sdboyer/gpkt.v2"),
+				mkPI("gopkg.in/sdboyer/gpkt.v3"),
+			},
+			urlcount: 6,
+			srccount: 3,
+		},
+		"gopkgin separation from github": {
+			roots: []ProjectIdentifier{
+				mkPI("gopkg.in/sdboyer/gpkt.v1"),
+				mkPI("github.com/sdboyer/gpkt"),
+			},
+			urlcount: 4,
+			srccount: 2,
+		},
+	}
+
+	for name, fix := range fixtures {
+		t.Run(name, fix.run)
+	}
+}
+
 func TestGetSources(t *testing.T) {
 	// This test is a tad slow, skip it on -short
 	if testing.Short() {

internal/gps/maybe_source.go

@@ -142,7 +142,8 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo
 	// We don't actually need a fully consistent transform into the on-disk path
 	// - just something that's unique to the particular gopkg.in domain context.
 	// So, it's OK to just dumb-join the scheme with the path.
-	path := sourceCachePath(cachedir, m.url.Scheme+"/"+m.opath)
+	aliasURL := m.url.Scheme + "://" + m.opath
+	path := sourceCachePath(cachedir, aliasURL)
 	ustr := m.url.String()
 
 	r, err := newCtxRepo(vcs.Git, ustr, path)
@@ -158,6 +159,7 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo
 		},
 		major:    m.major,
 		unstable: m.unstable,
+		aliasURL: aliasURL,
 	}
 
 	var vl []PairedVersion

internal/gps/source.go

@@ -168,8 +168,12 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	// integrate it back into the main map.
 	sc.srcmut.Lock()
 	defer sc.srcmut.Unlock()
-	// Record the name -> URL mapping, even if it's a self-mapping.
+	// Record the name -> URL mapping, making sure that we also get the
+	// self-mapping.
 	sc.nameToURL[normalizedName] = url
+	if url != normalizedName {
+		sc.nameToURL[url] = url
+	}
 
 	if sa, has := sc.srcs[url]; has {
 		// URL already had an entry in the main map; use that as the result.

internal/gps/vcs_source.go

@@ -289,6 +289,13 @@ type gopkginSource struct {
 	gitSource
 	major    uint64
 	unstable bool
+	// The aliased URL we report as being the one we talk to, even though we're
+	// actually talking directly to GitHub.
+	aliasURL string
+}
+
+func (s *gopkginSource) upstreamURL() string {
+	return s.aliasURL
 }
 
 func (s *gopkginSource) listVersions(ctx context.Context) ([]PairedVersion, error) {

internal/gps/vcs_source_test.go

@@ -151,8 +151,8 @@ func testGopkginSourceInteractions(t *testing.T) {
 	}()
 
 	tfunc := func(opath, n string, major uint64, evl []Version) {
-		un := "https://" + n
-		u, err := url.Parse(un)
+		un := "https://" + opath
+		u, err := url.Parse("https://" + n)
 		if err != nil {
 			t.Errorf("URL was bad, lolwut? errtext: %s", err)
 			return