Skip to content

Commit 3b5474a

Browse files
[6.0] Fix cache not cleaned up if download fails (#7671)
Cherry-pick of #7663. **Explanation**: If a server returns an error when trying to download a binaryTarget an invalid file remains in the artifacts cache. **Scope**: Isolated to binary targets cache. **Risk**: Low, scope is isolated, and the change has a corresponding test. **Testing**: Automated with an existing test case refined. **Issue**: rdar://123897276 **Reviewer**: @MaxDesiatov --------- Co-authored-by: Philipp Wallrich <[email protected]>
1 parent c56e4fc commit 3b5474a

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
@@ -7374,6 +7374,7 @@ final class WorkspaceTests: XCTestCase {
73747374
let fs = InMemoryFileSystem()
73757375
let sandbox = AbsolutePath("/tmp/ws/")
73767376
try fs.createDirectory(sandbox, recursive: true)
7377+
let artifactUrl = "https://a.com/a.zip"
73777378

73787379
let httpClient = LegacyHTTPClient(handler: { request, _, completion in
73797380
do {
@@ -7404,7 +7405,7 @@ final class WorkspaceTests: XCTestCase {
74047405
MockTarget(
74057406
name: "A1",
74067407
type: .binary,
7407-
url: "https://a.com/a.zip",
7408+
url: artifactUrl,
74087409
checksum: "a1"
74097410
),
74107411
]
@@ -7429,6 +7430,17 @@ final class WorkspaceTests: XCTestCase {
74297430
// make sure artifact downloaded is deleted
74307431
XCTAssertTrue(fs.isDirectory(AbsolutePath("/tmp/ws/.build/artifacts/root")))
74317432
XCTAssertFalse(fs.exists(AbsolutePath("/tmp/ws/.build/artifacts/root/a.zip")))
7433+
7434+
// make sure the cached artifact is also deleted
7435+
let artifactCacheKey = artifactUrl.spm_mangledToC99ExtendedIdentifier()
7436+
guard let cachePath = workspace.workspaceLocation?
7437+
.sharedBinaryArtifactsCacheDirectory?
7438+
.appending(artifactCacheKey) else {
7439+
XCTFail("Required workspace location wasn't found")
7440+
return
7441+
}
7442+
7443+
XCTAssertFalse(fs.exists(cachePath))
74327444
}
74337445

74347446
func testArtifactDownloaderOrArchiverError() throws {

0 commit comments

Comments
 (0)