Fix test failure by using atomic write instead of file locks

Author: mattt

Sources/Commands/SwiftPackageRegistryTool.swift

@@ -102,9 +102,9 @@ public struct SwiftPackageRegistryTool: ParsableCommand {
 
             let configuration = try swiftTool.getRegistriesConfig()
             if global {
-                try configuration.applyShared(handler: set)
+                try configuration.updateShared(with: set)
             } else {
-                try configuration.applyLocal(handler: set)
+                try configuration.updateLocal(with: set)
             }
 
             // TODO: Add login and password to .netrc
@@ -141,9 +141,9 @@ public struct SwiftPackageRegistryTool: ParsableCommand {
 
             let configuration = try swiftTool.getRegistriesConfig()
             if global {
-                try configuration.applyShared(handler: unset)
+                try configuration.updateShared(with: unset)
             } else {
-                try configuration.applyLocal(handler: unset)
+                try configuration.updateLocal(with: unset)
             }
         }
     }

Sources/Workspace/WorkspaceConfiguration.swift

@@ -399,25 +399,25 @@ extension Workspace.Configuration {
             sharedRegistriesFile: AbsolutePath?,
             fileSystem: FileSystem
         ) throws {
-            self.localRegistries = .init(path: localRegistriesFile, fileSystem: fileSystem, deleteWhenEmpty: true)
-            self.sharedRegistries = sharedRegistriesFile.map { .init(path: $0, fileSystem: fileSystem, deleteWhenEmpty: false) }
+            self.localRegistries = .init(path: localRegistriesFile, fileSystem: fileSystem)
+            self.sharedRegistries = sharedRegistriesFile.map { .init(path: $0, fileSystem: fileSystem) }
             self.fileSystem = fileSystem
             try self.computeRegistries()
         }
 
         @discardableResult
-        public func applyLocal(handler: (inout RegistryConfiguration) throws -> Void) throws -> RegistryConfiguration {
-            try self.localRegistries.apply(handler: handler)
+        public func updateLocal(with handler: (inout RegistryConfiguration) throws -> Void) throws -> RegistryConfiguration {
+            try self.localRegistries.update(with: handler)
             try self.computeRegistries()
             return self.configuration
         }
 
         @discardableResult
-        public func applyShared(handler: (inout RegistryConfiguration) throws -> Void) throws -> RegistryConfiguration {
+        public func updateShared(with handler: (inout RegistryConfiguration) throws -> Void) throws -> RegistryConfiguration {
             guard let sharedRegistries = self.sharedRegistries else {
                 throw InternalError("shared registries not configured")
             }
-            try sharedRegistries.apply(handler: handler)
+            try sharedRegistries.update(with: handler)
             try self.computeRegistries()
             return self.configuration
         }
@@ -428,11 +428,11 @@ extension Workspace.Configuration {
             try self.lock.withLock {
                 var configuration = RegistryConfiguration()
 
-                if let sharedConfiguration = try sharedRegistries?.get() {
+                if let sharedConfiguration = try sharedRegistries?.load() {
                     configuration.merge(sharedConfiguration)
                 }
 
-                let localConfiguration = try localRegistries.get()
+                let localConfiguration = try localRegistries.load()
                 configuration.merge(localConfiguration)
 
                 self._configuration = configuration
@@ -445,37 +445,13 @@ extension Workspace.Configuration {
     private struct RegistriesStorage {
         private let path: AbsolutePath
         private let fileSystem: FileSystem
-        private let deleteWhenEmpty: Bool
 
-        public init(path: AbsolutePath, fileSystem: FileSystem, deleteWhenEmpty: Bool) {
+        public init(path: AbsolutePath, fileSystem: FileSystem) {
             self.path = path
             self.fileSystem = fileSystem
-            self.deleteWhenEmpty = deleteWhenEmpty
-        }
-
-        public func get() throws -> RegistryConfiguration {
-            return try self.fileSystem.withLock(on: self.path.parentDirectory, type: .shared) {
-                return try Self.load(self.path, fileSystem: self.fileSystem)
-            }
-        }
-
-        @discardableResult
-        public func apply(handler: (inout RegistryConfiguration) throws -> Void) throws -> RegistryConfiguration {
-            if !self.fileSystem.exists(self.path.parentDirectory) {
-                try self.fileSystem.createDirectory(self.path.parentDirectory, recursive: true)
-            }
-            return try self.fileSystem.withLock(on: self.path.parentDirectory, type: .exclusive) {
-                let configuration = try Self.load(self.path, fileSystem: self.fileSystem)
-                var updatedConfiguration = configuration
-                try handler(&updatedConfiguration)
-                if updatedConfiguration != configuration {
-                    try Self.save(updatedConfiguration, to: self.path, fileSystem: self.fileSystem, deleteWhenEmpty: self.deleteWhenEmpty)
-                }
-                return updatedConfiguration
-            }
         }
 
-        private static func load(_ path: AbsolutePath, fileSystem: FileSystem) throws -> RegistryConfiguration {
+        public func load() throws -> RegistryConfiguration {
             guard fileSystem.exists(path) else {
                 return RegistryConfiguration()
             }
@@ -485,23 +461,26 @@ extension Workspace.Configuration {
             return try decoder.decode(RegistryConfiguration.self, from: data)
         }
 
-        private static func save(_ configuration: RegistryConfiguration, to path: AbsolutePath, fileSystem: FileSystem, deleteWhenEmpty: Bool) throws {
-            if configuration.isEmpty {
-                if deleteWhenEmpty && fileSystem.exists(path)  {
-                    // deleteWhenEmpty is a backward compatibility mode
-                    return try fileSystem.removeFileTree(path)
-                } else if !fileSystem.exists(path)  {
-                    // nothing to do
-                    return
-                }
-            }
-
+        public func save(_ configuration: RegistryConfiguration) throws {
             let encoder = JSONEncoder.makeWithDefaults()
             let data = try encoder.encode(configuration)
+
             if !fileSystem.exists(path.parentDirectory) {
                 try fileSystem.createDirectory(path.parentDirectory, recursive: true)
             }
-            try fileSystem.writeFileContents(path, data: data)
+            try fileSystem.writeFileContents(path, bytes: ByteString(data), atomically: true)
+        }
+
+        @discardableResult
+        public func update(with handler: (inout RegistryConfiguration) throws -> Void) throws -> RegistryConfiguration {
+            let configuration = try load()
+            var updatedConfiguration = configuration
+            try handler(&updatedConfiguration)
+            if updatedConfiguration != configuration {
+                try save(updatedConfiguration)
+            }
+
+            return updatedConfiguration
         }
     }
 }