Improved the error message when using a local package dependency as 'url:' instead of 'path:' (#3236)

Improved the error message when using a local package dependency as 'url:' instead of 'path:'

rdar://69272273

Co-authored-by: Miguel Perez <[email protected]>

Author: miggs597

Fixtures/Miscellaneous/LocalPackageAsURL/Bar/Package.swift

@@ -0,0 +1,12 @@
+// swift-tools-version:5.1
+import PackageDescription
+
+let package = Package(
+    name: "Bar",
+    dependencies: [
+        .package(url: "../Foo", from: "1.0.0"),
+    ],
+    targets: [
+        .target(name: "Bar", dependencies: ["Foo"], path: "./"),
+    ]
+)

Fixtures/Miscellaneous/LocalPackageAsURL/Bar/main.swift

@@ -0,0 +1,4 @@
+import Foo
+
+foo()
+print("here")

Fixtures/Miscellaneous/LocalPackageAsURL/Foo/Foo.swift

@@ -0,0 +1,3 @@
+public func foo() {
+	{}()
+}

Fixtures/Miscellaneous/LocalPackageAsURL/Foo/Package.swift

@@ -0,0 +1,12 @@
+// swift-tools-version:4.2
+import PackageDescription
+
+let package = Package(
+    name: "Foo",
+    products: [
+        .library(name: "Foo", targets: ["Foo"]),
+    ],
+    targets: [
+        .target(name: "Foo", path: "./"),
+    ]
+)

Package.swift

@@ -137,7 +137,7 @@ let package = Package(
         .target(
             /** Package model conventions and loading support */
             name: "PackageLoading",
-            dependencies: ["SwiftToolsSupport-auto", "Basics", "PackageModel"]),
+            dependencies: ["SwiftToolsSupport-auto", "Basics", "PackageModel", "SourceControl"]),
 
         // MARK: Package Dependency Resolution
 

Sources/PackageLoading/PackageDescription4Loader.swift

@@ -13,6 +13,7 @@ import TSCBasic
 import TSCUtility
 import PackageModel
 import Foundation
+import SourceControl
 
 enum ManifestJSONParser {
      struct Result {
@@ -361,13 +362,21 @@ extension PackageDependencyDescription {
             self = .local(identity: identity,
                           name: name,
                           path: path,
-                           productFilter: .everything)
+                          productFilter: .everything)
         // a package in a git location, may be a remote URL or on disk
         // TODO: consider refining the behavior + validation when the package is on disk
         // TODO: refactor this when adding registry support
         default:
             // location mapping (aka mirrors)
             let location = identityResolver.resolveLocation(from: location)
+            
+            if let validPath = try? AbsolutePath(validating: location), fileSystem.exists(validPath) {
+                let gitRepoProvider = GitRepositoryProvider()
+                guard gitRepoProvider.isValidDirectory(location) else {
+                    throw StringError("Cannot clone from local directory \(location)\nPlease git init or use \"path:\" for \(location)")
+                }
+            }
+            
             // in the future this will check with the registries for the identity of the URL
             let identity = identityResolver.resolveIdentity(for: location)
             let requirement = try Requirement(v4: requirementJSON)

Sources/SPMTestSupport/misc.swift

@@ -33,6 +33,7 @@ import class Foundation.Bundle
 /// the name were a relative path.
 public func fixture(
     name: String,
+    createGitRepo: Bool = true,
     file: StaticString = #file,
     line: UInt = #line,
     body: (AbsolutePath) throws -> Void
@@ -76,7 +77,9 @@ public func fixture(
                     guard localFileSystem.isDirectory(srcDir) else { continue }
                     let dstDir = tmpDirPath.appending(component: fileName)
                     try systemQuietly("cp", "-R", "-H", srcDir.pathString, dstDir.pathString)
-                    initGitRepo(dstDir, tag: "1.2.3", addFile: false)
+                    if createGitRepo {
+                        initGitRepo(dstDir, tag: "1.2.3", addFile: false)
+                    }
                 }
 
                 // Invoke the block, passing it the path of the copied fixture.

Sources/SourceControl/GitRepository.swift

@@ -81,12 +81,25 @@ public struct GitRepositoryProvider: RepositoryProvider {
         // expected cost of iterative updates on a full clone is less than on a
         // shallow clone.
         precondition(!localFileSystem.exists(path))
+
         // FIXME: Ideally we should pass `--progress` here and report status regularly.  We currently don't have callbacks for that.
         try self.callGit("clone", "--mirror", repository.url, path.pathString,
                          repository: repository,
                          failureMessage: "Failed to clone repository \(repository.url)")
     }
-
+    
+    public func isValidDirectory(_ directory: String) -> Bool {
+        // Provides better feedback when mistakingly using url: for a dependency that
+        // is a local package. Still allows for using url with a local package that has
+        // also been initialized by git
+        do {
+            try self.callGit("-C", directory, "rev-parse", "--git-dir", repository: RepositorySpecifier(url: directory))
+            return true
+        } catch {
+            return false
+        }
+    }
+    
     public func copy(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws {
         try localFileSystem.copy(from: sourcePath, to: destinationPath)
     }

Tests/FunctionalTests/MiscellaneousTests.swift

@@ -421,7 +421,23 @@ class MiscellaneousTestCase: XCTestCase {
             XCTAssert(output.contains("does not exist"), "Error from git was not propogated to process output: \(output)")
         }
     }
+    
+    func testLocalPackageUsedAsURL() throws {
+        fixture(name: "Miscellaneous/LocalPackageAsURL", createGitRepo: false) { prefix in
+            // This fixture has a setup that is trying to use a local package
+            // as a url that hasn't been initialized as a repo
+
+            // Launch swift-build.
+            let app = prefix.appending(component: "Bar")
+
+            let result = try SwiftPMProduct.SwiftBuild.executeProcess([], packagePath: app)
 
+            XCTAssert(result.exitStatus != .terminated(code: 0))
+            let output = try result.utf8stderrOutput()
+            XCTAssert(output.contains("Cannot clone from local directory"), "Didn't find expected output: \(output)")
+        }
+    }
+    
     func testUnicode() {
         #if !os(Linux) && !os(Android) // TODO: - Linux has trouble with this and needs investigation.
         fixture(name: "Miscellaneous/Unicode") { prefix in