Skip to content

Commit eee97d7

Browse files
authored
make local binary archives extraction more resilient (#4335)
motivation: in some case duplicate artifacts or parallel processes may race the extraction of binary artifacts changes: add a file system lock around archive extraction of local archives, similar to what we do for remote archives rdar://92622234
1 parent f3315a4 commit eee97d7

File tree

1 file changed

+24
-22
lines changed

1 file changed

+24
-22
lines changed

Sources/Workspace/Workspace+BinaryArtifacts.swift

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,12 @@ extension Workspace {
165165

166166
// finally download zip files, if any
167167
for artifact in zipArtifacts.get() {
168-
let parentDirectory = artifactsDirectory.appending(component: artifact.packageRef.identity.description)
169-
guard observabilityScope.trap ({ try fileSystem.createDirectory(parentDirectory, recursive: true) }) else {
168+
let destinationDirectory = artifactsDirectory.appending(component: artifact.packageRef.identity.description)
169+
guard observabilityScope.trap ({ try fileSystem.createDirectory(destinationDirectory, recursive: true) }) else {
170170
continue
171171
}
172172

173-
let archivePath = parentDirectory.appending(component: artifact.url.lastPathComponent)
173+
let archivePath = destinationDirectory.appending(component: artifact.url.lastPathComponent)
174174
if self.fileSystem.exists(archivePath) {
175175
guard observabilityScope.trap ({ try self.fileSystem.removeFileTree(archivePath) }) else {
176176
continue
@@ -241,7 +241,7 @@ extension Workspace {
241241
case .success:
242242
var artifactPath: AbsolutePath? = nil
243243
observabilityScope.trap {
244-
try self.fileSystem.withLock(on: parentDirectory, type: .exclusive) {
244+
try self.fileSystem.withLock(on: destinationDirectory, type: .exclusive) {
245245
// strip first level component if needed
246246
if try self.fileSystem.shouldStripFirstLevel(archiveDirectory: tempExtractionDirectory, acceptableExtensions: BinaryTarget.Kind.allCases.map({ $0.fileExtension })) {
247247
observabilityScope.emit(debug: "stripping first level component from \(tempExtractionDirectory)")
@@ -253,7 +253,7 @@ extension Workspace {
253253
// copy from temp location to actual location
254254
for file in content {
255255
let source = tempExtractionDirectory.appending(component: file)
256-
let destination = parentDirectory.appending(component: file)
256+
let destination = destinationDirectory.appending(component: file)
257257
if self.fileSystem.exists(destination) {
258258
try self.fileSystem.removeFileTree(destination)
259259
}
@@ -332,24 +332,26 @@ extension Workspace {
332332
case .success:
333333
observabilityScope.trap { () -> Void in
334334
var artifactPath: AbsolutePath? = nil
335-
// strip first level component if needed
336-
if try self.fileSystem.shouldStripFirstLevel(archiveDirectory: tempExtractionDirectory, acceptableExtensions: BinaryTarget.Kind.allCases.map({ $0.fileExtension })) {
337-
observabilityScope.emit(debug: "stripping first level component from \(tempExtractionDirectory)")
338-
try self.fileSystem.stripFirstLevel(of: tempExtractionDirectory)
339-
} else {
340-
observabilityScope.emit(debug: "no first level component stripping needed for \(tempExtractionDirectory)")
341-
}
342-
let content = try self.fileSystem.getDirectoryContents(tempExtractionDirectory)
343-
// copy from temp location to actual location
344-
for file in content {
345-
let source = tempExtractionDirectory.appending(component: file)
346-
let destination = destinationDirectory.appending(component: file)
347-
if self.fileSystem.exists(destination) {
348-
try self.fileSystem.removeFileTree(destination)
335+
try self.fileSystem.withLock(on: destinationDirectory, type: .exclusive) {
336+
// strip first level component if needed
337+
if try self.fileSystem.shouldStripFirstLevel(archiveDirectory: tempExtractionDirectory, acceptableExtensions: BinaryTarget.Kind.allCases.map({ $0.fileExtension })) {
338+
observabilityScope.emit(debug: "stripping first level component from \(tempExtractionDirectory)")
339+
try self.fileSystem.stripFirstLevel(of: tempExtractionDirectory)
340+
} else {
341+
observabilityScope.emit(debug: "no first level component stripping needed for \(tempExtractionDirectory)")
349342
}
350-
try self.fileSystem.copy(from: source, to: destination)
351-
if artifact.isMatchingDirectory(destination) {
352-
artifactPath = destination
343+
let content = try self.fileSystem.getDirectoryContents(tempExtractionDirectory)
344+
// copy from temp location to actual location
345+
for file in content {
346+
let source = tempExtractionDirectory.appending(component: file)
347+
let destination = destinationDirectory.appending(component: file)
348+
if self.fileSystem.exists(destination) {
349+
try self.fileSystem.removeFileTree(destination)
350+
}
351+
try self.fileSystem.copy(from: source, to: destination)
352+
if artifact.isMatchingDirectory(destination) {
353+
artifactPath = destination
354+
}
353355
}
354356
}
355357

0 commit comments

Comments
 (0)