Remove slightly dangerous optimization

If there happens to be a file at the location passed into the `ManifestLoader, we were using that to compile the manifest, instead of the actual contents. This is dangerous, because the public API of `ManifestLoader` accepts a custom filesystem parameter that is not taken into account at all here, so the file being used might not be the right one, it just has the same path on the local filesystem.

We could have done some more work to preserve the optimization in the case where the used filesystem is the local one and the correct file is being used, but that doesn't seem worth it since the vast majority of loads are being done for remote dependencies (this we may load multiple times as we are resolving) which are never using the local filesystem.

Author: neonichu

Sources/PackageLoading/ManifestLoader.swift

@@ -442,27 +442,24 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         completion: @escaping (Result<EvaluationResult, Error>) -> Void
     ) throws {
         do {
-            if localFileSystem.isFile(manifestPath) {
+            try withTemporaryFile(suffix: ".swift") { tempFile, cleanupTempFile in
+                try localFileSystem.writeFileContents(tempFile.path, bytes: ByteString(manifestContents))
                 try self.evaluateManifest(
-                    at: manifestPath,
+                    at: tempFile.path,
                     packageIdentity: packageIdentity,
                     toolsVersion: toolsVersion,
-                    delegateQueue:  delegateQueue,
-                    callbackQueue: callbackQueue,
-                    completion: completion
-                )
-            } else {
-                try withTemporaryFile(suffix: ".swift") { tempFile, cleanupTempFile in
-                    try localFileSystem.writeFileContents(tempFile.path, bytes: ByteString(manifestContents))
-                    try self.evaluateManifest(
-                        at: tempFile.path,
-                        packageIdentity: packageIdentity,
-                        toolsVersion: toolsVersion,
-                        delegateQueue: delegateQueue,
-                        callbackQueue: callbackQueue
-                    ) { result in
-                        dispatchPrecondition(condition: .onQueue(callbackQueue))
-                        cleanupTempFile(tempFile)
+                    delegateQueue: delegateQueue,
+                    callbackQueue: callbackQueue
+                ) { result in
+                    dispatchPrecondition(condition: .onQueue(callbackQueue))
+                    cleanupTempFile(tempFile)
+
+                    switch result {
+                    case .success(var evaluationResult):
+                        // Replace the temporary path with the orgiginal one in compiler output.
+                        evaluationResult.compilerOutput = evaluationResult.compilerOutput?.replacingOccurrences(of: tempFile.path.pathString, with: manifestPath.pathString)
+                        completion(.success(evaluationResult))
+                    case .failure:
                         completion(result)
                     }
                 }

Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift

@@ -609,7 +609,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
 
                 XCTAssertNoDiagnostics(observability.diagnostics)
                 XCTAssertEqual(try delegate.loaded(timeout: .now() + 1), [manifestPath])
-                XCTAssertEqual(try delegate.parsed(timeout: .now() + 1), expectCached ? [] : [manifestPath])
+                XCTAssertEqual(try delegate.parsed(timeout: .now() + 1).count, expectCached ? 0 : 1)
                 XCTAssertEqual(manifest.displayName, "Trivial")
                 XCTAssertEqual(manifest.targets[0].name, "foo")
             }
@@ -669,7 +669,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
 
                 XCTAssertNoDiagnostics(observability.diagnostics)
                 XCTAssertEqual(try delegate.loaded(timeout: .now() + 1), [manifestPath])
-                XCTAssertEqual(try delegate.parsed(timeout: .now() + 1), expectCached ? [] : [manifestPath])
+                XCTAssertEqual(try delegate.parsed(timeout: .now() + 1).count, expectCached ? 0 : 1)
                 XCTAssertEqual(manifest.displayName, "Trivial")
                 XCTAssertEqual(manifest.targets[0].name, "foo")
             }