Skip to content

Commit 6c49273

Browse files
committed
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.
1 parent 813fd7a commit 6c49273

File tree

1 file changed

+9
-20
lines changed

1 file changed

+9
-20
lines changed

Sources/PackageLoading/ManifestLoader.swift

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -634,29 +634,18 @@ public final class ManifestLoader: ManifestLoaderProtocol {
634634
completion: @escaping (Result<EvaluationResult, Error>) -> Void
635635
) {
636636
do {
637-
if localFileSystem.isFile(manifestPath) {
637+
try withTemporaryFile(suffix: ".swift") { tempFile, cleanupTempFile in
638+
try localFileSystem.writeFileContents(tempFile.path, bytes: ByteString(manifestContents))
638639
self.evaluateManifest(
639-
at: manifestPath,
640+
at: tempFile.path,
640641
packageIdentity: packageIdentity,
641642
toolsVersion: toolsVersion,
642-
delegateQueue: delegateQueue,
643-
callbackQueue: callbackQueue,
644-
completion: completion
645-
)
646-
} else {
647-
try withTemporaryFile(suffix: ".swift") { tempFile, cleanupTempFile in
648-
try localFileSystem.writeFileContents(tempFile.path, bytes: ByteString(manifestContents))
649-
self.evaluateManifest(
650-
at: tempFile.path,
651-
packageIdentity: packageIdentity,
652-
toolsVersion: toolsVersion,
653-
delegateQueue: delegateQueue,
654-
callbackQueue: callbackQueue
655-
) { result in
656-
dispatchPrecondition(condition: .onQueue(callbackQueue))
657-
cleanupTempFile(tempFile)
658-
completion(result)
659-
}
643+
delegateQueue: delegateQueue,
644+
callbackQueue: callbackQueue
645+
) { result in
646+
dispatchPrecondition(condition: .onQueue(callbackQueue))
647+
cleanupTempFile(tempFile)
648+
completion(result)
660649
}
661650
}
662651
} catch {

0 commit comments

Comments
 (0)