Skip to content

Commit 4b9f96f

Browse files
authored
Fix: cache not cleaned up if download fails (#7663)
If a server returns an error when trying to download a binaryTarget an invalid file remains in the artifacts cache. ### Motivation: If the download of a binaryTarget fails a cache file still remains in the artifact cache. The file only contains the server's response which is usually the status code and the error message. Example: ``` $ ls ~/Library/Caches/org.swift.swiftpm/artifacts . .. https___test_example_local_foo_zip $ cat ~/Library/Caches/org.swift.swiftpm/artifacts/https___test_example_local_foo_zip 404 page not found ``` Worse, all following `resolve` calls therefore fail because the cached file is used but is an invalid archive. ### Modifications: If the download fails the cache file in the artifacts cache is deleted. ### Result: As expected, the artifacts folder is empty. Example: ``` $ ls ~/Library/Caches/org.swift.swiftpm/artifacts . .. ```
1 parent 8a4364c commit 4b9f96f

File tree

3 files changed

+20
-1
lines changed

3 files changed

+20
-1
lines changed

Sources/SPMTestSupport/MockWorkspace.swift

+4
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ public final class MockWorkspace {
136136
return self.sandbox.appending(components: ".build", "artifacts")
137137
}
138138

139+
public var workspaceLocation: Workspace.Location? {
140+
return self._workspace?.location
141+
}
142+
139143
public func pathToRoot(withName name: String) throws -> AbsolutePath {
140144
return try AbsolutePath(validating: name, relativeTo: self.rootsDir)
141145
}

Sources/Workspace/Workspace+BinaryArtifacts.swift

+3
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,9 @@ extension Workspace {
618618
progress: progress,
619619
completion: { result in
620620
self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: false)
621+
if case .failure = result {
622+
try? self.fileSystem.removeFileTree(cachedArtifactPath)
623+
}
621624
completion(result.flatMap {
622625
Result.init(catching: {
623626
// copy from cache to destination

Tests/WorkspaceTests/WorkspaceTests.swift

+13-1
Original file line numberDiff line numberDiff line change
@@ -7416,6 +7416,7 @@ final class WorkspaceTests: XCTestCase {
74167416
let fs = InMemoryFileSystem()
74177417
let sandbox = AbsolutePath("/tmp/ws/")
74187418
try fs.createDirectory(sandbox, recursive: true)
7419+
let artifactUrl = "https://a.com/a.zip"
74197420

74207421
let httpClient = LegacyHTTPClient(handler: { request, _, completion in
74217422
do {
@@ -7446,7 +7447,7 @@ final class WorkspaceTests: XCTestCase {
74467447
MockTarget(
74477448
name: "A1",
74487449
type: .binary,
7449-
url: "https://a.com/a.zip",
7450+
url: artifactUrl,
74507451
checksum: "a1"
74517452
),
74527453
]
@@ -7471,6 +7472,17 @@ final class WorkspaceTests: XCTestCase {
74717472
// make sure artifact downloaded is deleted
74727473
XCTAssertTrue(fs.isDirectory(AbsolutePath("/tmp/ws/.build/artifacts/root")))
74737474
XCTAssertFalse(fs.exists(AbsolutePath("/tmp/ws/.build/artifacts/root/a.zip")))
7475+
7476+
// make sure the cached artifact is also deleted
7477+
let artifactCacheKey = artifactUrl.spm_mangledToC99ExtendedIdentifier()
7478+
guard let cachePath = workspace.workspaceLocation?
7479+
.sharedBinaryArtifactsCacheDirectory?
7480+
.appending(artifactCacheKey) else {
7481+
XCTFail("Required workspace location wasn't found")
7482+
return
7483+
}
7484+
7485+
XCTAssertFalse(fs.exists(cachePath))
74747486
}
74757487

74767488
func testArtifactDownloaderOrArchiverError() throws {

0 commit comments

Comments
 (0)