Merge pull request #627 from abertelrud/SR-2261

Improve the error message that's shown when trying to access a package by a broken local path

Author: abertelrud

Sources/Get/Error.swift

@@ -10,6 +10,7 @@
 
 import Utility
 
+/// Errors that can result from trying to fetch a package from a repository.
 enum Error: Swift.Error {
 
     typealias ClonePath = String
@@ -21,6 +22,10 @@ enum Error: Swift.Error {
     case updateRequired(ClonePath)
     case unversioned(ClonePath)
     case invalidDependencyGraphMissingTag(package: String, requestedTag: String, existingTags: String)
+    /// A package is referenced using a `file` URL (i.e. a local file system reference), but there is no package at that path.
+    case missingLocalFileURL(URL)
+    /// A package is referenced using a `file` URL (i.e. a local file system reference), but the file system entity at that path isn't a cloned repository (a situation that is not currently supported).
+    case nonRepoLocalFileURL(URL)
 }
 
 extension Error: CustomStringConvertible {
@@ -38,6 +43,10 @@ extension Error: CustomStringConvertible {
             return "No version tag found in (\(package)) package. Add a version tag with \"git tag\" command. Example: \"git tag 0.1.0\""
         case .noManifest(let clonePath, let version):
             return "The package at `\(clonePath)' has no Package.swift for the specific version: \(version)"
+        case .missingLocalFileURL(let url):
+            return "No package at path \(url)"
+        case .nonRepoLocalFileURL(let url):
+            return "Directory at path \(url) is not a Git repository"
         }
     }
 }

Sources/Get/PackagesDirectory.swift

@@ -123,6 +123,19 @@ extension PackagesDirectory: Fetcher {
         let manifestParser = { try self.manifestLoader.load(packagePath: $0, baseURL: $1, version: $2) }
         let basename = url.components(separatedBy: "/").last!
         let dstdir = packagesPath.appending(component: basename)
+        // If it's a local URL, do some preliminary checking.  The reason we don't just give it a try and then do forensics if it fails is to avoid spurious successes (not sure if that can happen.
+        // FIXME: We should also support `file` URLs in addition to "no scheme" URLs to mean a file path.
+        if URL.scheme(url) == nil {
+            // It's a local path -- by the time we get here we currently always have an absolute path (this is spelled out it ManifestLoader's `loadFile(path:,baseURL:,version:,fileSystem:)` method, albeit with a note that it should doesn't belong in that method).
+            let path = AbsolutePath(url)
+            guard localFileSystem.exists(path) else {
+                throw Error.missingLocalFileURL(url)
+            }
+            // FIXME: We should not be baking in the assumption about ".git" at this point.
+            guard localFileSystem.isDirectory(path.appending(component: ".git")) else {
+                throw Error.nonRepoLocalFileURL(url)
+            }
+        }
         if let repo = Git.Repo(path: dstdir), repo.origin == url {
             //TODO need to canonicalize the URL need URL struct
             return try RawClone(path: dstdir, manifestParser: manifestParser)