[5.3] Improve the diagnostics that are emitted when a package dependency specifies a non-existent branch or commit

This improves the handling of errors thrown by Git, and is quite safe because it does not affect the successful code path.  It checks for the common case of a missing branch name, and as a bonus, adds a specific check to provide a suggestion related to the `master` -> `main` renaming (if `master` is requested but doesn't exist, but if `main` does exist).

Note that tags go through different processing, since they are the inputs to the semantic versioning and missing tags are reported differently.  There is no way in SwiftPM to define an explicit dependency on a non-semver tag, which is why this code only needs to handle branches and commits.

A similar fix is already in the main branch (though the exact diff is not the same in order to keep the 5.3 patch minimal)

rdar://69413377

Author: abertelrud

Sources/PackageGraph/RepositoryPackageContainerProvider.swift

@@ -261,6 +261,9 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
 
         /// The actual error that occurred.
         public let underlyingError: Swift.Error
+        
+        /// Optional suggestion for how to resolve the error.
+        public let suggestion: String?
     }
 
     /// This is used to remember if tools version of a particular version is
@@ -371,7 +374,7 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
             }.1
         } catch {
             throw GetDependenciesErrorWrapper(
-                containerIdentifier: identifier.repository.url, reference: version.description, underlyingError: error)
+                containerIdentifier: identifier.repository.url, reference: version.description, underlyingError: error, suggestion: nil)
         }
     }
 
@@ -383,8 +386,28 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
                 return try getDependencies(at: revision)
             }.1
         } catch {
-            throw GetDependenciesErrorWrapper(
-                containerIdentifier: identifier.repository.url, reference: revision, underlyingError: error)
+            // Examine the error to see if we can come up with a more informative and actionable error message.  We know that the revision is expected to be a branch name or a hash (tags are handled through a different code path).
+            if let gitInvocationError = error as? ProcessResult.Error, gitInvocationError.description.contains("Needed a single revision") {
+                // It was a Git process invocation error.  Take a look at the repository to see if we can come up with a reasonable diagnostic.
+                if let rev = try? repository.resolveRevision(identifier: revision), repository.exists(revision: rev) {
+                    // Revision does exist, so something else must be wrong.
+                    throw GetDependenciesErrorWrapper(
+                        containerIdentifier: identifier.repository.url, reference: revision, underlyingError: error, suggestion: nil)
+                }
+                else {
+                    // Revision does not exist, so we customize the error.
+                    let sha1RegEx = try! RegEx(pattern: #"\A[:xdigit:]{40}\Z"#)
+                    let isBranchRev = sha1RegEx.matchGroups(in: revision).compactMap{ $0 }.isEmpty
+                    let errorMessage = "could not find " + (isBranchRev ? "a branch named ‘\(revision)’" : "the commit \(revision)")
+                    let mainBranchExists = (try? repository.resolveRevision(identifier: "main")) != nil
+                    let suggestion = (revision == "master" && mainBranchExists) ? "did you mean ‘main’?" : nil
+                    throw GetDependenciesErrorWrapper(
+                        containerIdentifier: identifier.repository.url, reference: revision,
+                        underlyingError: StringError(errorMessage), suggestion: suggestion)
+                }
+            }
+            // If we get this far without having thrown an error, we wrap and throw the underlying error.
+            throw GetDependenciesErrorWrapper(containerIdentifier: identifier.repository.url, reference: revision, underlyingError: error, suggestion: nil)
         }
     }
 

Sources/Workspace/Workspace.swift

@@ -2007,7 +2007,8 @@ extension Workspace {
             // Emit proper error if we were not able to parse some manifest during dependency resolution.
             case let error as RepositoryPackageContainer.GetDependenciesErrorWrapper:
                 let location = PackageLocation.Remote(url: error.containerIdentifier, reference: error.reference)
-                diagnostics.emit(error.underlyingError, location: location)
+                let suggestion = error.suggestion.flatMap{ " (\($0))" }
+                diagnostics.emit(StringError("\(error.underlyingError)\(suggestion ?? "")"), location: location)
 
             default:
                 diagnostics.emit(error)

Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift

@@ -11,6 +11,7 @@
 import XCTest
 
 import TSCBasic
+import TSCUtility
 import PackageLoading
 import PackageModel
 import PackageGraph
@@ -478,4 +479,62 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
             )
         }
     }
+    
+    func testMissingBranchDiagnostics() {
+        mktmpdir { tmpDir in
+            // Create a repository.
+            let packageDir = tmpDir.appending(component: "SomePackage")
+            try localFileSystem.createDirectory(packageDir)
+            initGitRepo(packageDir)
+            let packageRepo = GitRepository(path: packageDir)
+            
+            // Create a package manifest in it (it only needs the `swift-tools-version` part, because we'll supply the manifest later).
+            let manifestFile = packageDir.appending(component: "Package.swift")
+            try localFileSystem.writeFileContents(manifestFile, bytes: ByteString("// swift-tools-version:4.2"))
+            
+            // Commit it and tag it.
+            try packageRepo.stage(file: "Package.swift")
+            try packageRepo.commit(message: "Initial")
+            try packageRepo.tag(name: "1.0.0")
+            
+            // Rename the `master` branch to `main`.
+            try systemQuietly([Git.tool, "-C", packageDir.pathString, "branch", "-m", "main"])
+
+            // Create a repository manager for it.
+            let repoProvider = GitRepositoryProvider()
+            let repositoryManager = RepositoryManager(path: packageDir, provider: repoProvider, delegate: nil, fileSystem: localFileSystem)
+            
+            // Create a container provider, configured with a mock manifest loader that will return the package manifest.
+            let manifest = Manifest.createV4Manifest(
+                name: packageDir.basename,
+                path: packageDir.pathString,
+                url: packageDir.pathString,
+                packageKind: .root,
+                targets: [
+                    TargetDescription(name: packageDir.basename, path: packageDir.pathString)
+                ]
+            )
+            let containerProvider = RepositoryPackageContainerProvider(repositoryManager: repositoryManager, manifestLoader: MockManifestLoader(manifests: [.init(url: packageDir.pathString, version: nil) : manifest]))
+            
+            // Get a hold of the container for the test package.
+            let packageRef = PackageReference(identity: "somepackage", path: packageDir.pathString)
+            let container = try await { containerProvider.getContainer(for: packageRef, completion: $0) } as! RepositoryPackageContainer
+            
+            // Simulate accessing a fictitious dependency on the `master` branch, and check that we get back the expected error.
+            do { _ = try container.getDependencies(at: "master") }
+            catch let error as RepositoryPackageContainer.GetDependenciesErrorWrapper {
+                // We expect to get an error message about `master` with a suggestion to use `main`.
+                XCTAssertEqual("\(error.underlyingError)", "could not find a branch named ‘master’")
+                XCTAssertEqual(error.suggestion, "did you mean ‘main’?")
+            }
+            
+            // Simulate accessing a fictitious dependency on some random commit that doesn't exist, and check that we get back the expected error.
+            do { _ = try container.getDependencies(at: "535f4cb5b4a0872fa691473e82d7b27b9894df00") }
+            catch let error as RepositoryPackageContainer.GetDependenciesErrorWrapper {
+                // We expect to get an error message about the commit SHA, but without a suggestion.
+                XCTAssertMatch("\(error.underlyingError)", "could not find the commit 535f4cb5b4a0872fa691473e82d7b27b9894df00")
+                XCTAssertEqual(error.suggestion, nil)
+            }
+        }
+    }
 }