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,29 +442,19 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         completion: @escaping (Result<EvaluationResult, Error>) -> Void
     ) {
         do {
-            if localFileSystem.isFile(manifestPath) {
+            try withTemporaryFile(suffix: ".swift") { tempFile, cleanupTempFile in
+                try localFileSystem.writeFileContents(tempFile.path, bytes: ByteString(manifestContents))
                 self.evaluateManifest(
-                    at: manifestPath,
+                    at: tempFile.path,
+                    originalPath: manifestPath,
                     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))
-                    self.evaluateManifest(
-                        at: tempFile.path,
-                        packageIdentity: packageIdentity,
-                        toolsVersion: toolsVersion,
-                        delegateQueue: delegateQueue,
-                        callbackQueue: callbackQueue
-                    ) { result in
-                        dispatchPrecondition(condition: .onQueue(callbackQueue))
-                        cleanupTempFile(tempFile)
-                        completion(result)
-                    }
+                    delegateQueue: delegateQueue,
+                    callbackQueue: callbackQueue
+                ) { result in
+                    dispatchPrecondition(condition: .onQueue(callbackQueue))
+                    cleanupTempFile(tempFile)
+                    completion(result)
                 }
             }
         } catch {
@@ -477,6 +467,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     /// Helper method for evaluating the manifest.
     func evaluateManifest(
         at manifestPath: AbsolutePath,
+        originalPath: AbsolutePath,
         packageIdentity: PackageIdentity,
         toolsVersion: ToolsVersion,
         delegateQueue: DispatchQueue,
@@ -597,7 +588,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                         let compilerResult : ProcessResult
                         do {
                             compilerResult = try result.get()
-                            evaluationResult.compilerOutput = try (compilerResult.utf8Output() + compilerResult.utf8stderrOutput()).spm_chuzzle()
+                            evaluationResult.compilerOutput = try (compilerResult.utf8Output() + compilerResult.utf8stderrOutput()).spm_chuzzle()?.replacingOccurrences(of: manifestPath.pathString, with: originalPath.pathString)
                         } catch {
                             return completion(.failure(error))
                         }

Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift

@@ -589,7 +589,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")
             }
@@ -649,7 +649,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")
             }