Remove bogus SCM validation

As a follow-up to #7037, we decided to disable this bogus validation entirely since it doesn't add value compared to git's own output (it actually decreases the value since e.g. it omits the repository in question), it was emitted multiple times (probably still a bug for other validations that needs to be fixed) and it was unconditional, so e.g. it could cause SwiftPM to fail in cases where the branch/revision wouldn't be used anyway.

I also opted to remove the existing test here since it doesn't seem to be very useful for us to test git itself. We will now always just use git and the user will get the same behavior as when they're using an invalid branch with git itself.

rdar://117442643

Author: neonichu

Fixtures/Miscellaneous/InvalidRefs/InvalidBranch/Package.swift

@@ -217,23 +217,6 @@ public struct ManifestValidator {
 
     func validateSourceControlDependency(_ dependency: PackageDependency.SourceControl) -> [Basics.Diagnostic] {
         var diagnostics = [Basics.Diagnostic]()
-
-        // validate source control ref
-        switch dependency.requirement {
-        case .branch(let name):
-            // FIXME: removed this validation because it is applied unconditionally, rdar://117442643 tracks restoring it once we can do it right
-            break
-        case .revision(let revision):
-            do {
-                if try !self.sourceControlValidator.isValidRefFormat(revision) {
-                    diagnostics.append(.invalidSourceControlRevision(revision))
-                }
-            } catch {
-                diagnostics.append(.invalidSourceControlRevision(revision, underlyingError: error))
-            }
-        default:
-            break
-        }
         // if a location is on file system, validate it is in fact a git repo
         // there is a case to be made to throw early (here) if the path does not exists
         // but many of our tests assume they can pass a non existent path
@@ -254,7 +237,6 @@ public struct ManifestValidator {
 }
 
 public protocol ManifestSourceControlValidator {
-    func isValidRefFormat(_ revision: String) throws -> Bool
     func isValidDirectory(_ path: AbsolutePath) throws -> Bool
 }
 
@@ -319,14 +301,6 @@ extension Basics.Diagnostic {
         }
     }
 
-    static func invalidSourceControlBranchName(_ name: String, underlyingError: Error? = nil) -> Self {
-        .error("invalid branch name: '\(name)'\(errorSuffix(underlyingError))")
-    }
-
-    static func invalidSourceControlRevision(_ revision: String, underlyingError: Error? = nil) -> Self {
-        .error("invalid revision: '\(revision)'\(errorSuffix(underlyingError))")
-    }
-
     static func invalidSourceControlDirectory(_ path: AbsolutePath, underlyingError: Error? = nil) -> Self {
         .error("cannot clone from local directory \(path)\nPlease git init or use \"path:\" for \(path)\(errorSuffix(underlyingError))")
     }

Fixtures/Miscellaneous/InvalidRefs/InvalidRevision/Package.swift

@@ -486,10 +486,6 @@ public final class InMemoryGitRepositoryProvider: RepositoryProvider {
         return true
     }
 
-    public func isValidRefFormat(_ ref: String) throws -> Bool {
-        return true
-    }
-
     public func cancel(deadline: DispatchTime) throws {
         // noop
     }

Sources/PackageLoading/ManifestLoader+Validation.swift

@@ -210,12 +210,6 @@ public struct GitRepositoryProvider: RepositoryProvider, Cancellable {
         return result == ".git" || result == "." || result == directory.pathString
     }
 
-    /// Returns true if the git reference name is well formed.
-    public func isValidRefFormat(_ ref: String) throws -> Bool {
-        _ = try self.git.run(["check-ref-format", "--allow-onelevel", ref])
-        return true
-    }
-
     public func copy(from sourcePath: Basics.AbsolutePath, to destinationPath: Basics.AbsolutePath) throws {
         try localFileSystem.copy(from: sourcePath, to: destinationPath)
     }

Sources/SPMTestSupport/InMemoryGitRepository.swift

@@ -144,9 +144,6 @@ public protocol RepositoryProvider: Cancellable {
 
     /// Returns true if the directory is valid git location.
     func isValidDirectory(_ directory: AbsolutePath) throws -> Bool
-
-    /// Returns true if the git reference name is well formed.
-    func isValidRefFormat(_ ref: String) throws -> Bool
 }
 
 /// Abstract repository operations.

Sources/SourceControl/GitRepository.swift

@@ -426,11 +426,6 @@ public class RepositoryManager: Cancellable {
         try self.provider.isValidDirectory(directory)
     }
 
-    /// Returns true if the git reference name is well formed.
-    public func isValidRefFormat(_ ref: String) throws -> Bool {
-        try self.provider.isValidRefFormat(ref)
-    }
-
     /// Reset the repository manager.
     ///
     /// Note: This also removes the cloned repositories from the disk.

Sources/SourceControl/Repository.swift

@@ -337,29 +337,6 @@ class MiscellaneousTestCase: XCTestCase {
         }
     }
 
-    func testInvalidRefsValidation() throws {
-        try XCTSkipIf(true, "skipping since we disabled the validation, see rdar://117442643")
-
-        try fixture(name: "Miscellaneous/InvalidRefs", createGitRepo: false) { fixturePath in
-            do {
-                XCTAssertThrowsError(try SwiftPM.Build.execute(packagePath: fixturePath.appending("InvalidBranch"))) { error in
-                    guard case SwiftPMError.executionFailure(_, _, let stderr) = error else {
-                        return XCTFail("invalid error \(error)")
-                    }
-                    XCTAssert(stderr.contains("invalid branch name: "), "Didn't find expected output: \(stderr)")
-                }
-            }
-            do {
-                XCTAssertThrowsError(try SwiftPM.Build.execute(packagePath: fixturePath.appending("InvalidRevision"))) { error in
-                    guard case SwiftPMError.executionFailure(_, _, let stderr) = error else {
-                        return XCTFail("invalid error \(error)")
-                    }
-                    XCTAssert(stderr.contains("invalid revision: "), "Didn't find expected output: \(stderr)")
-                }
-            }
-        }
-    }
-
     func testUnicode() throws {
         #if !os(Linux) && !os(Android) // TODO: - Linux has trouble with this and needs investigation.
         try fixture(name: "Miscellaneous/Unicode") { fixturePath in

Sources/SourceControl/RepositoryManager.swift

@@ -190,10 +190,6 @@ final class ManifestTestDelegate: ManifestLoaderDelegate {
 }
 
 fileprivate struct NOOPManifestSourceControlValidator: ManifestSourceControlValidator {
-    func isValidRefFormat(_ revision: String) throws -> Bool {
-        true
-    }
-
     func isValidDirectory(_ path: AbsolutePath) throws -> Bool {
         true
     }

Tests/FunctionalTests/MiscellaneousTests.swift

@@ -561,10 +561,6 @@ class RepositoryManagerTests: XCTestCase {
                 fatalError("should not be called")
             }
 
-            func isValidRefFormat(_ ref: String) throws -> Bool {
-                fatalError("should not be called")
-            }
-
             func cancel(deadline: DispatchTime) throws {
                 print("cancel")
             }
@@ -644,10 +640,6 @@ private class DummyRepository: Repository {
         fatalError("unexpected API call")
     }
 
-    func isValidRefFormat(_ ref: String) throws -> Bool {
-        fatalError("unexpected API call")
-    }
-
     func fetch() throws {
         self.provider.increaseFetchCount()
     }
@@ -728,10 +720,6 @@ private class DummyRepositoryProvider: RepositoryProvider {
         return true
     }
 
-    func isValidRefFormat(_ ref: String) throws -> Bool {
-        return true
-    }
-
     func cancel(deadline: DispatchTime) throws {
         // noop
     }

Tests/PackageLoadingTests/PDLoadingTests.swift

@@ -150,10 +150,6 @@ private class MockRepositories: RepositoryProvider {
         return true
     }
 
-    func isValidRefFormat(_ ref: String) throws -> Bool {
-        return true
-    }
-
     func cancel(deadline: DispatchTime) throws {
         // noop
     }