Fix cache failing to resolve dependencies with name collision

Source/CarthageKit/Dependency.swift

@@ -23,7 +23,26 @@ public enum Dependency {
 			return url.name ?? url.urlString
 
 		case let .binary(url):
-			return url.lastPathComponent.stripping(suffix: ".json")
+			let name = url.lastPathComponent.stripping(suffix: ".json")
+			// lastPathComponent gives empty string when no path
+			if name == "" {
+				return url.host ?? "unknown"
+			}
+			return name
+		}
+	}
+
+	public var cacheName: String {
+		switch self {
+		case let .gitHub(.dotCom, repo):
+			return "\(repo.name)_\(repo.owner)"
+		case let .gitHub(.enterprise(url), repo):
+			return "\(url.absoluteString.cacheSafeName)_\(repo.name)_\(repo.owner)"
+		case let .git(url):
+			// Replace all non a-z0-9 chars with _
+			return url.normalizedURLString.cacheSafeName
+		case let .binary(url):
+			return url.absoluteString.cacheSafeName
 		}
 	}
 

Source/CarthageKit/FrameworkExtensions.swift

@@ -28,6 +28,11 @@ extension String {
 			return self
 		}
 	}
+
+	// Replace all non a-z0-9 chars with _
+	internal var cacheSafeName: String {
+		return self.replacingOccurrences(of: "://", with: "_").components(separatedBy: CharacterSet.alphanumerics.inverted).joined(separator: "_")
+	}
 }
 
 extension Signal {

Source/CarthageKit/GitURL.swift

@@ -47,7 +47,7 @@ public struct GitURL {
 
 	/// The name of the repository, if it can be inferred from the URL.
 	public var name: String? {
-		let components = urlString.split(omittingEmptySubsequences: true) { $0 == "/" }
+		let components = normalizedURLString.split(omittingEmptySubsequences: true) { $0 == "/" }
 
 		return components
 			.last

Source/CarthageKit/Project.swift

@@ -1254,8 +1254,26 @@ private func BCSymbolMapsForFramework(_ frameworkURL: URL, inDirectoryURL direct
 
 /// Returns the file URL at which the given project's repository will be
 /// located.
-private func repositoryFileURL(for dependency: Dependency, baseURL: URL = Constants.Dependency.repositoriesURL) -> URL {
-	return baseURL.appendingPathComponent(dependency.name, isDirectory: true)
+internal func repositoryFileURL(for dependency: Dependency, baseURL: URL = Constants.Dependency.repositoriesURL) -> URL {
+	let url = baseURL.appendingPathComponent(dependency.cacheName, isDirectory: true)
+	let fileManager = FileManager.default
+	// If the new dir exists, use it
+	if fileManager.fileExists(atPath: url.path) {
+		return url
+	}
+	// The old cache was based on dependency name, and we want to migrate to the new cache name
+	let oldUrl = baseURL.appendingPathComponent(dependency.name, isDirectory: true)
+	if !fileManager.fileExists(atPath: oldUrl.path) {
+		return url
+	}
+
+	do {
+		try fileManager.moveItem(at: oldUrl, to: url)
+	} catch {
+		NSLog("Warning: Failed to move old cache dir: \(error.localizedDescription). (\(error))")
+	}
+
+	return url
 }
 
 /// Returns the string representing a relative path from a dependency back to the root

Tests/CarthageKitTests/DependencySpec.swift

@@ -72,6 +72,12 @@ class DependencySpec: QuickSpec {
 
 					expect(dependency.name) == "myproject"
 				}
+
+				it("should be the domain when there is no path") {
+					let dependency = Dependency.git(GitURL("ssh://server.com"))
+
+					expect(dependency.name) == "server.com"
+				}
 
 				it("should not include the trailing git suffix") {
 					let dependency = Dependency.git(GitURL("ssh://server.com/myproject.git"))
@@ -84,6 +90,18 @@ class DependencySpec: QuickSpec {
 
 					expect(dependency.name) == "whatisthisurleven"
 				}
+
+				it("should be the last component of scp url with a path") {
+					let dependency = Dependency.git(GitURL("git@github.com:antitypical/Result.git"))
+
+					expect(dependency.name) == "Result"
+				}
+
+				it("should be the last component of scp url") {
+					let dependency = Dependency.git(GitURL("git@github.com:Result.git"))
+
+					expect(dependency.name) == "Result"
+				}
 			}
 
 			context("binary") {
@@ -92,6 +110,12 @@ class DependencySpec: QuickSpec {
 
 					expect(dependency.name) == "myproject"
 				}
+
+				it("should be the domain when there is no path") {
+					let dependency = Dependency.git(GitURL("https://server.com"))
+
+					expect(dependency.name) == "server.com"
+				}
 
 				it("should not include the trailing git suffix") {
 					let dependency = Dependency.binary(URL(string: "https://server.com/myproject.json")!)
@@ -100,6 +124,114 @@ class DependencySpec: QuickSpec {
 				}
 			}
 		}
+
+		describe("cacheName") {
+			context ("github") {
+				it("should equal owner_name of a github.com repo") {
+					let dependency = Dependency.gitHub(.dotCom, Repository(owner: "owner", name: "name"))
+
+					expect(dependency.cacheName) == "name_owner"
+				}
+
+				it("should equal url_owner_name of an enterprise github repo") {
+					let enterpriseRepo = Repository(
+						owner: "owner",
+						name: "name")
+
+					let dependency = Dependency.gitHub(.enterprise(url: URL(string: "http://server.com")!), enterpriseRepo)
+
+					expect(dependency.cacheName) == "http_server_com_name_owner"
+				}
+			}
+
+			context("git") {
+				it("should be the last component of the URL") {
+					let dependency = Dependency.git(GitURL("ssh://server.com/myproject"))
+
+					expect(dependency.cacheName) == "server_com_myproject"
+				}
+
+				it("should not include the trailing git suffix") {
+					let dependency = Dependency.git(GitURL("ssh://server.com/myproject.git"))
+
+					expect(dependency.cacheName) == "server_com_myproject"
+				}
+
+				it("should be the entire URL string if there is no last component") {
+					let dependency = Dependency.git(GitURL("whatisthisurleven"))
+
+					expect(dependency.cacheName) == "whatisthisurleven"
+				}
+
+				it("should be the owner_name when there are 3 components of the path") {
+					let dependency = Dependency.git(GitURL("path/to/project"))
+
+					expect(dependency.cacheName) == "path_to_project"
+				}
+
+				it("should be the name when there is one components of the URL") {
+					let dependency = Dependency.git(GitURL("path/project"))
+
+					expect(dependency.cacheName) == "path_project"
+				}
+
+				it("should be the owner_name when there are 2 components of the URL") {
+					let dependency = Dependency.git(GitURL("ssh://server.com/myname/myproject"))
+
+					expect(dependency.cacheName) == "server_com_myname_myproject"
+				}
+
+				it("should be the owner_name when there are 2 components of the URL and still no trailing git suffix") {
+					let dependency = Dependency.git(GitURL("ssh://server.com/myname/myproject.git"))
+
+					expect(dependency.cacheName) == "server_com_myname_myproject"
+				}
+			}
+
+			context("binary") {
+				it("should be the last component of the URL") {
+					let dependency = Dependency.binary(URL(string: "https://server.com/myproject")!)
+
+					expect(dependency.cacheName) == "https_server_com_myproject"
+				}
+
+				it("should be the domain when there is no path") {
+					let dependency = Dependency.binary(URL(string: "https://server.com")!)
+
+					expect(dependency.cacheName) == "https_server_com"
+				}
+
+				it("should not include the trailing json suffix") {
+					let dependency = Dependency.binary(URL(string: "https://server.com/myproject.json")!)
+
+					expect(dependency.cacheName) == "https_server_com_myproject_json"
+				}
+
+				it("should be owner_name when there are multiple components of the path") {
+					let dependency = Dependency.binary(URL(string: "path/to/project")!)
+
+					expect(dependency.cacheName) == "path_to_project"
+				}
+
+				it("should be owner_name when there are 2 components of the path") {
+					let dependency = Dependency.binary(URL(string: "path/project")!)
+
+					expect(dependency.cacheName) == "path_project"
+				}
+
+				it("should be owner_name when there are 2 components of the URL") {
+					let dependency = Dependency.binary(URL(string: "https://server.com/myname/myproject")!)
+
+					expect(dependency.cacheName) == "https_server_com_myname_myproject"
+				}
+
+				it("should be owner_name when there are 2 components of the URL and still no trailing json suffix") {
+					let dependency = Dependency.binary(URL(string: "https://server.com/myname/myproject.json")!)
+
+					expect(dependency.cacheName) == "https_server_com_myname_myproject_json"
+				}
+			}
+		}
 
 		describe("from") {
 			context("github") {

Tests/CarthageKitTests/ProjectSpec.swift

@@ -505,6 +505,39 @@ class ProjectSpec: QuickSpec {
 				expect(actualPlatform) == .iOS
 			}
 		}
+
+		describe("repositoryFileURL") {
+			let dependency = Dependency.gitHub(.dotCom, Repository(owner: "owner", name: "name"))
+			let temporaryPath = (NSTemporaryDirectory() as NSString).appendingPathComponent(ProcessInfo.processInfo.globallyUniqueString)
+			let temporaryURL = URL(fileURLWithPath: temporaryPath, isDirectory: true)
+
+			beforeEach {
+				expect { try FileManager.default.createDirectory(atPath: temporaryURL.path, withIntermediateDirectories: true) }.notTo(throwError())
+			}
+
+			afterEach {
+				_ = try? FileManager.default.removeItem(at: temporaryURL)
+			}
+
+			it("should append cacheName to baseURL") {
+				let url = CarthageKit.repositoryFileURL(for: dependency, baseURL: temporaryURL)
+
+				expect(url) == temporaryURL.appendingPathComponent(dependency.cacheName, isDirectory: true)
+			}
+
+			it("should move old cache to new cache") {
+				let fileManager = FileManager.default
+
+				let oldCacheUrl = temporaryURL.appendingPathComponent(dependency.name, isDirectory: true)
+				expect { try FileManager.default.createDirectory(atPath: oldCacheUrl.path, withIntermediateDirectories: true) }.notTo(throwError())
+
+				let url = CarthageKit.repositoryFileURL(for: dependency, baseURL: temporaryURL)
+
+				expect(url) == temporaryURL.appendingPathComponent(dependency.cacheName, isDirectory: true)
+				expect(fileManager.fileExists(atPath: oldCacheUrl.path)) == false
+				expect(fileManager.fileExists(atPath: url.path)) == true
+			}
+		}
 	}
 }