Treat ProjectRoot as case-insensitive in srcCoord

Author: sdboyer

internal/gps/manager_test.go

@@ -419,45 +419,55 @@ func TestFSCaseSensitivityConvergesSources(t *testing.T) {
 		t.Skip("Skipping slow test in short mode")
 	}
 
-	sm, clean := mkNaiveSM(t)
-	defer clean()
+	f := func(name string, pi1, pi2 ProjectIdentifier) {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			sm, clean := mkNaiveSM(t)
+			defer clean()
 
-	pi1 := mkPI("github.com/sdboyer/deptest").normalize()
-	sm.SyncSourceFor(pi1)
-	sg1, err := sm.srcCoord.getSourceGatewayFor(context.Background(), pi1)
-	if err != nil {
-		t.Fatal(err)
-	}
+			sm.SyncSourceFor(pi1)
+			sg1, err := sm.srcCoord.getSourceGatewayFor(context.Background(), pi1)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	pi2 := mkPI("github.com/Sdboyer/deptest").normalize()
-	sm.SyncSourceFor(pi2)
-	sg2, err := sm.srcCoord.getSourceGatewayFor(context.Background(), pi2)
-	if err != nil {
-		t.Fatal(err)
-	}
+			sm.SyncSourceFor(pi2)
+			sg2, err := sm.srcCoord.getSourceGatewayFor(context.Background(), pi2)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	path1 := sg1.src.(*gitSource).repo.LocalPath()
-	t.Log("path1:", path1)
-	stat1, err := os.Stat(path1)
-	if err != nil {
-		t.Fatal(err)
-	}
-	path2 := sg2.src.(*gitSource).repo.LocalPath()
-	t.Log("path2:", path2)
-	stat2, err := os.Stat(path2)
-	if err != nil {
-		t.Fatal(err)
-	}
+			path1 := sg1.src.(*gitSource).repo.LocalPath()
+			t.Log("path1:", path1)
+			stat1, err := os.Stat(path1)
+			if err != nil {
+				t.Fatal(err)
+			}
+			path2 := sg2.src.(*gitSource).repo.LocalPath()
+			t.Log("path2:", path2)
+			stat2, err := os.Stat(path2)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	same, count := os.SameFile(stat1, stat2), len(sm.srcCoord.srcs)
-	if same && count != 1 {
-		t.Log("are same, count", count)
-		t.Fatal("on case-insensitive filesystem, case-varying sources should have been folded together but were not")
-	}
-	if !same && count != 2 {
-		t.Log("not same, count", count)
-		t.Fatal("on case-sensitive filesystem, case-varying sources should not have been folded together, but were")
+			same, count := os.SameFile(stat1, stat2), len(sm.srcCoord.srcs)
+			if same && count != 1 {
+				t.Log("are same, count", count)
+				t.Fatal("on case-insensitive filesystem, case-varying sources should have been folded together but were not")
+			}
+			if !same && count != 2 {
+				t.Log("not same, count", count)
+				t.Fatal("on case-sensitive filesystem, case-varying sources should not have been folded together, but were")
+			}
+		})
 	}
+
+	folded := mkPI("github.com/sdboyer/deptest").normalize()
+	casevar1 := mkPI("github.com/Sdboyer/deptest").normalize()
+	casevar2 := mkPI("github.com/SdboyeR/deptest").normalize()
+	f("folded first", folded, casevar1)
+	f("folded second", casevar1, folded)
+	f("both unfolded", casevar1, casevar2)
 }
 
 // Regression test for #32

internal/gps/source.go

@@ -91,39 +91,58 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	}
 	sc.srcmut.RUnlock()
 
+	// Without a direct match, we must fold the input name to a generally
+	// stable, caseless variant and primarily work from that. This ensures that
+	// on case-insensitive filesystems, we do not end up with multiple
+	// sourceGateways for paths that vary only by case. We perform folding
+	// unconditionally, independent of whether the underlying fs is
+	// case-sensitive, in order to ensure uniform behavior.
+	//
+	// This has significant implications. It is effectively deciding that the
+	// ProjectRoot portion of import paths are case-insensitive, which is by no
+	// means an invariant maintained by all hosting systems. If this presents a
+	// problem in practice, then we can explore expanding the deduction system
+	// to include case-sensitivity-for-roots metadata and treat it on a
+	// host-by-host basis. Such cases would still be rejected by the Go
+	// toolchain's compiler, though, and case-sensitivity in root names is
+	// likely to be at least frowned on if not disallowed by most hosting
+	// systems. So we follow this path, which is both a vastly simpler solution
+	// and one that seems quite likely to work in practice.
+	foldedNormalName := toFold(normalizedName)
+
 	// No gateway exists for this path yet; set up a proto, being careful to fold
-	// together simultaneous attempts on the same path.
+	// together simultaneous attempts on the same case-folded path.
 	sc.psrcmut.Lock()
-	if chans, has := sc.protoSrcs[normalizedName]; has {
+	if chans, has := sc.protoSrcs[foldedNormalName]; has {
 		// Another goroutine is already working on this normalizedName. Fold
 		// in with that work by attaching our return channels to the list.
 		rc := srcReturnChans{
 			ret: make(chan *sourceGateway, 1),
 			err: make(chan error, 1),
 		}
-		sc.protoSrcs[normalizedName] = append(chans, rc)
+		sc.protoSrcs[foldedNormalName] = append(chans, rc)
 		sc.psrcmut.Unlock()
 		return rc.awaitReturn()
 	}
 
-	sc.protoSrcs[normalizedName] = []srcReturnChans{}
+	sc.protoSrcs[foldedNormalName] = []srcReturnChans{}
 	sc.psrcmut.Unlock()
 
 	doReturn := func(sg *sourceGateway, err error) {
 		sc.psrcmut.Lock()
 		if sg != nil {
-			for _, rc := range sc.protoSrcs[normalizedName] {
+			for _, rc := range sc.protoSrcs[foldedNormalName] {
 				rc.ret <- sg
 			}
 		} else if err != nil {
-			for _, rc := range sc.protoSrcs[normalizedName] {
+			for _, rc := range sc.protoSrcs[foldedNormalName] {
 				rc.err <- err
 			}
 		} else {
 			panic("sg and err both nil")
 		}
 
-		delete(sc.protoSrcs, normalizedName)
+		delete(sc.protoSrcs, foldedNormalName)
 		sc.psrcmut.Unlock()
 	}
 
@@ -142,7 +161,7 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	// and bailing out if we find an entry.
 	var srcGate *sourceGateway
 	sc.srcmut.RLock()
-	if url, has := sc.nameToURL[normalizedName]; has {
+	if url, has := sc.nameToURL[foldedNormalName]; has {
 		if srcGate, has := sc.srcs[url]; has {
 			sc.srcmut.RUnlock()
 			doReturn(srcGate, nil)
@@ -155,10 +174,10 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	srcGate = newSourceGateway(pd.mb, sc.supervisor, sc.cachedir)
 
 	// The normalized name is usually different from the source URL- e.g.
-	// github.com/golang/dep/internal/gps vs. https://github.com/golang/dep/internal/gps. But it's
-	// possible to arrive here with a full URL as the normalized name - and
-	// both paths *must* lead to the same sourceGateway instance in order to
-	// ensure disk access is correctly managed.
+	// github.com/sdboyer/gps vs. https://github.com/sdboyer/gps. But it's
+	// possible to arrive here with a full URL as the normalized name - and both
+	// paths *must* lead to the same sourceGateway instance in order to ensure
+	// disk access is correctly managed.
 	//
 	// Therefore, we now must query the sourceGateway to get the actual
 	// sourceURL it's operating on, and ensure it's *also* registered at
@@ -176,6 +195,11 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	defer sc.srcmut.Unlock()
 	// Record the name -> URL mapping, even if it's a self-mapping.
 	sc.nameToURL[normalizedName] = url
+	// Make sure we have both the folded and unfolded names recorded in the map,
+	// should they differ.
+	if normalizedName != foldedNormalName {
+		sc.nameToURL[foldedNormalName] = url
+	}
 
 	if sa, has := sc.srcs[url]; has {
 		// URL already had an entry in the main map; use that as the result.