From 88f1b098e3a68ceace30fcd7a8fd0202b1f7346d Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 7 May 2025 14:48:10 -0400 Subject: [PATCH 1/9] ProcessID enhacement --- Sources/Basics/Concurrency/PID.swift | 81 ++++++++++++++++++++ Sources/CoreCommands/SwiftCommandState.swift | 32 +++++--- 2 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 Sources/Basics/Concurrency/PID.swift diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift new file mode 100644 index 00000000000..6e40dc44768 --- /dev/null +++ b/Sources/Basics/Concurrency/PID.swift @@ -0,0 +1,81 @@ +// +// PID.swift +// SwiftPM +// +// Created by John Bute on 2025-04-24. +// + +import Foundation + +public protocol pidFileManipulator { + var scratchDirectory: AbsolutePath {get set} + var pidFilePath: AbsolutePath { get } + + init(scratchDirectory: AbsolutePath) + + + func readPID(from path: AbsolutePath) -> Int32? + func deletePIDFile(file: URL) throws + func writePID(pid: pid_t, to: URL, atomically: Bool, encoding: String.Encoding) throws + func getCurrentPID() -> Int32 +} + + + +public struct pidFile: pidFileManipulator { + + public var scratchDirectory: AbsolutePath + + public init(scratchDirectory: AbsolutePath) { + self.scratchDirectory = scratchDirectory + } + + /// Return the path of the PackageManager.lock.pid file where the PID is located + public var pidFilePath: AbsolutePath { + return self.scratchDirectory.appending(component: "PackageManager.lock.pid") + } + + /// Read the pid file + public func readPID(from path: AbsolutePath) -> Int32? { + // Check if the file exists + let filePath = path.pathString + if !FileManager.default.fileExists(atPath: filePath) { + print("File does not exist at path: \(filePath)") + return nil + } + + do { + // Read the contents of the file + let pidString = try String(contentsOf: path.asURL, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) + + // Print the PID string to debug the content + print("PID string: \(pidString)") + + // Check if the PID string can be converted to an Int32 + if let pid = Int32(pidString) { + return pid + } else { + return nil + } + } catch { + // Catch any errors and print them + return nil + } + } + + /// Get the current PID of the process + public func getCurrentPID() -> Int32 { + return getpid() + } + + /// Write .pid file containing PID of process currently using .build directory + public func writePID(pid: pid_t, to: URL, atomically: Bool, encoding: String.Encoding) throws { + try "\(pid)".write(to: pidFilePath.asURL, atomically: true, encoding: .utf8) + } + + /// Delete PID file at URL + public func deletePIDFile(file: URL) throws { + try FileManager.default.removeItem(at: file) + } + +} diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index 386ba5381ca..d4ae11f6a45 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -14,7 +14,7 @@ import _Concurrency import ArgumentParser import Basics import Dispatch -import class Foundation.NSLock +import class Foundation.NSDistributedLock import class Foundation.ProcessInfo import PackageGraph import PackageLoading @@ -22,6 +22,7 @@ import PackageLoading import PackageModel import SPMBuildCore import Workspace +import Foundation.NSFileManager #if USE_IMPL_ONLY_IMPORTS @_implementationOnly @@ -287,6 +288,7 @@ public final class SwiftCommandState { private let hostTriple: Basics.Triple? + private let pidManipulator: pidFileManipulator package var preferredBuildConfiguration = BuildConfiguration.debug /// Create an instance of this tool. @@ -324,7 +326,8 @@ public final class SwiftCommandState { createPackagePath: Bool, hostTriple: Basics.Triple? = nil, fileSystem: any FileSystem = localFileSystem, - environment: Environment = .current + environment: Environment = .current, + pidManipulator: pidFileManipulator? = nil ) throws { self.hostTriple = hostTriple self.fileSystem = fileSystem @@ -407,6 +410,8 @@ public final class SwiftCommandState { self.sharedSwiftSDKsDirectory = try fileSystem.getSharedSwiftSDKsDirectory( explicitDirectory: options.locations.swiftSDKsDirectory ?? options.locations.deprecatedSwiftSDKsDirectory ) + + self.pidManipulator = pidManipulator ?? pidFile(scratchDirectory: self.scratchDirectory) // set global process logging handler AsyncProcess.loggingHandler = { self.observabilityScope.emit(debug: $0) } @@ -1059,16 +1064,16 @@ public final class SwiftCommandState { let workspaceLock = try FileLock.prepareLock(fileToLock: self.scratchDirectory) let lockFile = self.scratchDirectory.appending(".lock").pathString + var lockAcquired = false + // Try a non-blocking lock first so that we can inform the user about an already running SwiftPM. do { try workspaceLock.lock(type: .exclusive, blocking: false) - let pid = ProcessInfo.processInfo.processIdentifier - try? String(pid).write(toFile: lockFile, atomically: true, encoding: .utf8) + lockAcquired = true } catch ProcessLockError.unableToAquireLock(let errno) { if errno == EWOULDBLOCK { - let lockingPID = try? String(contentsOfFile: lockFile, encoding: .utf8) - let pidInfo = lockingPID.map { "(PID: \($0)) " } ?? "" - + let existingPID = self.pidManipulator.readPID(from: self.pidManipulator.pidFilePath) + let pidInfo = existingPID.map { "(PID: \($0)) " } ?? "" if self.options.locations.ignoreLock { self.outputStream .write( @@ -1087,13 +1092,16 @@ public final class SwiftCommandState { // Only if we fail because there's an existing lock we need to acquire again as blocking. try workspaceLock.lock(type: .exclusive, blocking: true) - let pid = ProcessInfo.processInfo.processIdentifier - try? String(pid).write(toFile: lockFile, atomically: true, encoding: .utf8) + lockAcquired = true } } } self.workspaceLock = workspaceLock + + if lockAcquired || self.options.locations.ignoreLock { + try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID(), to: self.pidManipulator.pidFilePath.asURL, atomically: true, encoding: .utf8) + } } fileprivate func releaseLockIfNeeded() { @@ -1105,6 +1113,12 @@ public final class SwiftCommandState { self.workspaceLockState = .unlocked self.workspaceLock?.unlock() + + do { + try self.pidManipulator.deletePIDFile(file: self.pidManipulator.pidFilePath.asURL) + } catch { + self.observabilityScope.emit(warning: "Failed to delete PID file: \(error)") + } } } From 2662019f5b0148815e3e7b846032a0dcd3389ac4 Mon Sep 17 00:00:00 2001 From: John Bute Date: Mon, 5 May 2025 11:48:41 -0400 Subject: [PATCH 2/9] added simplification --- Sources/Basics/Concurrency/PID.swift | 27 ++++++++------------ Sources/CoreCommands/SwiftCommandState.swift | 6 ++--- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index 6e40dc44768..fe9593b91e4 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -9,14 +9,12 @@ import Foundation public protocol pidFileManipulator { var scratchDirectory: AbsolutePath {get set} - var pidFilePath: AbsolutePath { get } init(scratchDirectory: AbsolutePath) - - func readPID(from path: AbsolutePath) -> Int32? - func deletePIDFile(file: URL) throws - func writePID(pid: pid_t, to: URL, atomically: Bool, encoding: String.Encoding) throws + func readPID() -> Int32? + func deletePIDFile() throws + func writePID(pid: pid_t) throws func getCurrentPID() -> Int32 } @@ -31,25 +29,22 @@ public struct pidFile: pidFileManipulator { } /// Return the path of the PackageManager.lock.pid file where the PID is located - public var pidFilePath: AbsolutePath { + private var pidFilePath: AbsolutePath { return self.scratchDirectory.appending(component: "PackageManager.lock.pid") } /// Read the pid file - public func readPID(from path: AbsolutePath) -> Int32? { + public func readPID() -> Int32? { // Check if the file exists - let filePath = path.pathString - if !FileManager.default.fileExists(atPath: filePath) { + let filePath = pidFilePath.pathString + guard FileManager.default.fileExists(atPath: filePath) else { print("File does not exist at path: \(filePath)") return nil } do { // Read the contents of the file - let pidString = try String(contentsOf: path.asURL, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) - - // Print the PID string to debug the content - print("PID string: \(pidString)") + let pidString = try String(contentsOf: pidFilePath.asURL, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) // Check if the PID string can be converted to an Int32 if let pid = Int32(pidString) { @@ -69,13 +64,13 @@ public struct pidFile: pidFileManipulator { } /// Write .pid file containing PID of process currently using .build directory - public func writePID(pid: pid_t, to: URL, atomically: Bool, encoding: String.Encoding) throws { + public func writePID(pid: pid_t) throws { try "\(pid)".write(to: pidFilePath.asURL, atomically: true, encoding: .utf8) } /// Delete PID file at URL - public func deletePIDFile(file: URL) throws { - try FileManager.default.removeItem(at: file) + public func deletePIDFile() throws { + try FileManager.default.removeItem(at: pidFilePath.asURL) } } diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index d4ae11f6a45..5cadba61c84 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -1072,7 +1072,7 @@ public final class SwiftCommandState { lockAcquired = true } catch ProcessLockError.unableToAquireLock(let errno) { if errno == EWOULDBLOCK { - let existingPID = self.pidManipulator.readPID(from: self.pidManipulator.pidFilePath) + let existingPID = self.pidManipulator.readPID() let pidInfo = existingPID.map { "(PID: \($0)) " } ?? "" if self.options.locations.ignoreLock { self.outputStream @@ -1100,7 +1100,7 @@ public final class SwiftCommandState { self.workspaceLock = workspaceLock if lockAcquired || self.options.locations.ignoreLock { - try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID(), to: self.pidManipulator.pidFilePath.asURL, atomically: true, encoding: .utf8) + try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID()) } } @@ -1115,7 +1115,7 @@ public final class SwiftCommandState { self.workspaceLock?.unlock() do { - try self.pidManipulator.deletePIDFile(file: self.pidManipulator.pidFilePath.asURL) + try self.pidManipulator.deletePIDFile() } catch { self.observabilityScope.emit(warning: "Failed to delete PID file: \(error)") } From 6f02455c38ecc066c9f53d9acfba0e57cb79d858 Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 7 May 2025 10:43:40 -0400 Subject: [PATCH 3/9] added tests, naming consistency --- Sources/Basics/Concurrency/PID.swift | 14 +++---- Sources/CoreCommands/SwiftCommandState.swift | 16 ++++--- .../SwiftCommandStateTests.swift | 42 +++++++++++++++++++ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index fe9593b91e4..17ac65e29b0 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -7,7 +7,7 @@ import Foundation -public protocol pidFileManipulator { +public protocol PIDFileHandler { var scratchDirectory: AbsolutePath {get set} init(scratchDirectory: AbsolutePath) @@ -20,7 +20,7 @@ public protocol pidFileManipulator { -public struct pidFile: pidFileManipulator { +public struct PIDFile: PIDFileHandler { public var scratchDirectory: AbsolutePath @@ -29,14 +29,14 @@ public struct pidFile: pidFileManipulator { } /// Return the path of the PackageManager.lock.pid file where the PID is located - private var pidFilePath: AbsolutePath { + private var lockFilePath: AbsolutePath { return self.scratchDirectory.appending(component: "PackageManager.lock.pid") } /// Read the pid file public func readPID() -> Int32? { // Check if the file exists - let filePath = pidFilePath.pathString + let filePath = lockFilePath.pathString guard FileManager.default.fileExists(atPath: filePath) else { print("File does not exist at path: \(filePath)") return nil @@ -44,7 +44,7 @@ public struct pidFile: pidFileManipulator { do { // Read the contents of the file - let pidString = try String(contentsOf: pidFilePath.asURL, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) + let pidString = try String(contentsOf: lockFilePath.asURL, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) // Check if the PID string can be converted to an Int32 if let pid = Int32(pidString) { @@ -65,12 +65,12 @@ public struct pidFile: pidFileManipulator { /// Write .pid file containing PID of process currently using .build directory public func writePID(pid: pid_t) throws { - try "\(pid)".write(to: pidFilePath.asURL, atomically: true, encoding: .utf8) + try "\(pid)".write(to: lockFilePath.asURL, atomically: true, encoding: .utf8) } /// Delete PID file at URL public func deletePIDFile() throws { - try FileManager.default.removeItem(at: pidFilePath.asURL) + try FileManager.default.removeItem(at: lockFilePath.asURL) } } diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index 5cadba61c84..1f2f3634d88 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -288,7 +288,7 @@ public final class SwiftCommandState { private let hostTriple: Basics.Triple? - private let pidManipulator: pidFileManipulator + private let pidManipulator: PIDFileHandler package var preferredBuildConfiguration = BuildConfiguration.debug /// Create an instance of this tool. @@ -327,7 +327,7 @@ public final class SwiftCommandState { hostTriple: Basics.Triple? = nil, fileSystem: any FileSystem = localFileSystem, environment: Environment = .current, - pidManipulator: pidFileManipulator? = nil + pidManipulator: PIDFileHandler? = nil ) throws { self.hostTriple = hostTriple self.fileSystem = fileSystem @@ -411,7 +411,7 @@ public final class SwiftCommandState { explicitDirectory: options.locations.swiftSDKsDirectory ?? options.locations.deprecatedSwiftSDKsDirectory ) - self.pidManipulator = pidManipulator ?? pidFile(scratchDirectory: self.scratchDirectory) + self.pidManipulator = pidManipulator ?? PIDFile(scratchDirectory: self.scratchDirectory) // set global process logging handler AsyncProcess.loggingHandler = { self.observabilityScope.emit(debug: $0) } @@ -1072,8 +1072,8 @@ public final class SwiftCommandState { lockAcquired = true } catch ProcessLockError.unableToAquireLock(let errno) { if errno == EWOULDBLOCK { - let existingPID = self.pidManipulator.readPID() - let pidInfo = existingPID.map { "(PID: \($0)) " } ?? "" + let existingProcessPID = self.pidManipulator.readPID() + let pidInfo = existingProcessPID.map { "(PID: \($0)) " } ?? "" if self.options.locations.ignoreLock { self.outputStream .write( @@ -1100,7 +1100,11 @@ public final class SwiftCommandState { self.workspaceLock = workspaceLock if lockAcquired || self.options.locations.ignoreLock { - try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID()) + do { + try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID()) + } catch { + self.observabilityScope.emit(warning: "Failed to write to PID file: \(error)") + } } } diff --git a/Tests/CommandsTests/SwiftCommandStateTests.swift b/Tests/CommandsTests/SwiftCommandStateTests.swift index c5dc4431182..27f65358838 100644 --- a/Tests/CommandsTests/SwiftCommandStateTests.swift +++ b/Tests/CommandsTests/SwiftCommandStateTests.swift @@ -533,6 +533,48 @@ final class SwiftCommandStateTests: CommandsTestCase { } } } + + func testPIDFileHandlerLifecycle() async throws { + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + var pidHandler: PIDFile = PIDFile(scratchDirectory: scratchPath) + + + + // Ensure no PID file exists initially + XCTAssertNil(pidHandler.readPID(), "No PID should exist initially") + + // Write current PID + let currentPID = pidHandler.getCurrentPID() + try pidHandler.writePID(pid: currentPID) + + + // Read PID back + let readPID = pidHandler.readPID() + XCTAssertEqual(readPID, currentPID, "PID read should match written PID") + + // Delete the file + try pidHandler.deletePIDFile() + + // Ensure file is gone + XCTAssertNil(pidHandler.readPID(), "PID should be nil after deletion") + } + } + + func testMalformedPIDFile() async throws { + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidPath = scratchPath.appending(component: "PackageManager.lock.pid") + try localFileSystem.writeFileContents(pidPath, bytes: "notanumber") + + let pidHandler = PIDFile(scratchDirectory: scratchPath) + XCTAssertNil(pidHandler.readPID(), "Malformed PID file should result in nil") + } + } } extension SwiftCommandState { From c16f2b72ebdef431eeac646150196f0bcf61eebe Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 7 May 2025 11:01:44 -0400 Subject: [PATCH 4/9] formatting --- Sources/Basics/Concurrency/PID.swift | 40 ++++++++++--------- Sources/CoreCommands/SwiftCommandState.swift | 7 ++-- .../SwiftCommandStateTests.swift | 5 +-- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index 17ac65e29b0..6e91fc3661d 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -1,50 +1,53 @@ +//===----------------------------------------------------------------------===// // -// PID.swift -// SwiftPM +// This source file is part of the Swift open source project // -// Created by John Bute on 2025-04-24. +// Copyright (c) 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception // +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// import Foundation public protocol PIDFileHandler { - var scratchDirectory: AbsolutePath {get set} + var scratchDirectory: AbsolutePath { get set } init(scratchDirectory: AbsolutePath) - + func readPID() -> Int32? func deletePIDFile() throws func writePID(pid: pid_t) throws func getCurrentPID() -> Int32 } - - public struct PIDFile: PIDFileHandler { - public var scratchDirectory: AbsolutePath - + public init(scratchDirectory: AbsolutePath) { self.scratchDirectory = scratchDirectory } - + /// Return the path of the PackageManager.lock.pid file where the PID is located private var lockFilePath: AbsolutePath { - return self.scratchDirectory.appending(component: "PackageManager.lock.pid") + self.scratchDirectory.appending(component: "PackageManager.lock.pid") } /// Read the pid file public func readPID() -> Int32? { // Check if the file exists - let filePath = lockFilePath.pathString - guard FileManager.default.fileExists(atPath: filePath) else { + let filePath = self.lockFilePath.pathString + guard FileManager.default.fileExists(atPath: filePath) else { print("File does not exist at path: \(filePath)") return nil } do { // Read the contents of the file - let pidString = try String(contentsOf: lockFilePath.asURL, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines) + let pidString = try String(contentsOf: lockFilePath.asURL, encoding: .utf8) + .trimmingCharacters(in: .whitespacesAndNewlines) // Check if the PID string can be converted to an Int32 if let pid = Int32(pidString) { @@ -60,17 +63,16 @@ public struct PIDFile: PIDFileHandler { /// Get the current PID of the process public func getCurrentPID() -> Int32 { - return getpid() + getpid() } /// Write .pid file containing PID of process currently using .build directory public func writePID(pid: pid_t) throws { - try "\(pid)".write(to: lockFilePath.asURL, atomically: true, encoding: .utf8) + try "\(pid)".write(to: self.lockFilePath.asURL, atomically: true, encoding: .utf8) } - + /// Delete PID file at URL public func deletePIDFile() throws { - try FileManager.default.removeItem(at: lockFilePath.asURL) + try FileManager.default.removeItem(at: self.lockFilePath.asURL) } - } diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index 1f2f3634d88..4389dab39a3 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -289,6 +289,7 @@ public final class SwiftCommandState { private let hostTriple: Basics.Triple? private let pidManipulator: PIDFileHandler + package var preferredBuildConfiguration = BuildConfiguration.debug /// Create an instance of this tool. @@ -410,7 +411,7 @@ public final class SwiftCommandState { self.sharedSwiftSDKsDirectory = try fileSystem.getSharedSwiftSDKsDirectory( explicitDirectory: options.locations.swiftSDKsDirectory ?? options.locations.deprecatedSwiftSDKsDirectory ) - + self.pidManipulator = pidManipulator ?? PIDFile(scratchDirectory: self.scratchDirectory) // set global process logging handler @@ -1098,7 +1099,7 @@ public final class SwiftCommandState { } self.workspaceLock = workspaceLock - + if lockAcquired || self.options.locations.ignoreLock { do { try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID()) @@ -1117,7 +1118,7 @@ public final class SwiftCommandState { self.workspaceLockState = .unlocked self.workspaceLock?.unlock() - + do { try self.pidManipulator.deletePIDFile() } catch { diff --git a/Tests/CommandsTests/SwiftCommandStateTests.swift b/Tests/CommandsTests/SwiftCommandStateTests.swift index 27f65358838..d856c8760ac 100644 --- a/Tests/CommandsTests/SwiftCommandStateTests.swift +++ b/Tests/CommandsTests/SwiftCommandStateTests.swift @@ -539,9 +539,7 @@ final class SwiftCommandStateTests: CommandsTestCase { let scratchPath = tmpDir.appending(component: "scratch") try localFileSystem.createDirectory(scratchPath) - var pidHandler: PIDFile = PIDFile(scratchDirectory: scratchPath) - - + var pidHandler = PIDFile(scratchDirectory: scratchPath) // Ensure no PID file exists initially XCTAssertNil(pidHandler.readPID(), "No PID should exist initially") @@ -550,7 +548,6 @@ final class SwiftCommandStateTests: CommandsTestCase { let currentPID = pidHandler.getCurrentPID() try pidHandler.writePID(pid: currentPID) - // Read PID back let readPID = pidHandler.readPID() XCTAssertEqual(readPID, currentPID, "PID read should match written PID") From 511ece9298770a4aaf6f6d27ec49647d1e545219 Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 7 May 2025 11:19:28 -0400 Subject: [PATCH 5/9] fixed simple mistake --- Sources/CoreCommands/SwiftCommandState.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index 4389dab39a3..9be257a74c7 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -14,7 +14,7 @@ import _Concurrency import ArgumentParser import Basics import Dispatch -import class Foundation.NSDistributedLock +import class Foundation.NSLock import class Foundation.ProcessInfo import PackageGraph import PackageLoading @@ -22,7 +22,6 @@ import PackageLoading import PackageModel import SPMBuildCore import Workspace -import Foundation.NSFileManager #if USE_IMPL_ONLY_IMPORTS @_implementationOnly From cfdc8ae7c01bc9745c4d8113161977184fbcd67d Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 7 May 2025 14:49:27 -0400 Subject: [PATCH 6/9] fixed bug where .build directory was not built yet when testing spm --- Sources/Basics/Concurrency/PID.swift | 7 +++++++ Tests/CommandsTests/SwiftCommandStateTests.swift | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index 6e91fc3661d..18fdd27b5f3 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -68,6 +68,13 @@ public struct PIDFile: PIDFileHandler { /// Write .pid file containing PID of process currently using .build directory public func writePID(pid: pid_t) throws { + let parent = self.lockFilePath.parentDirectory + try FileManager.default.createDirectory( + at: parent.asURL, + withIntermediateDirectories: true, + attributes: nil + ) + try "\(pid)".write(to: self.lockFilePath.asURL, atomically: true, encoding: .utf8) } diff --git a/Tests/CommandsTests/SwiftCommandStateTests.swift b/Tests/CommandsTests/SwiftCommandStateTests.swift index d856c8760ac..4acb84d9caa 100644 --- a/Tests/CommandsTests/SwiftCommandStateTests.swift +++ b/Tests/CommandsTests/SwiftCommandStateTests.swift @@ -539,7 +539,7 @@ final class SwiftCommandStateTests: CommandsTestCase { let scratchPath = tmpDir.appending(component: "scratch") try localFileSystem.createDirectory(scratchPath) - var pidHandler = PIDFile(scratchDirectory: scratchPath) + let pidHandler = PIDFile(scratchDirectory: scratchPath) // Ensure no PID file exists initially XCTAssertNil(pidHandler.readPID(), "No PID should exist initially") From 453c15bd57cd147da0a601f33d848cc842d00951 Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 7 May 2025 16:31:01 -0400 Subject: [PATCH 7/9] added platform compatibility --- Sources/Basics/Concurrency/PID.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index 18fdd27b5f3..fe53a66a50e 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -19,7 +19,7 @@ public protocol PIDFileHandler { func readPID() -> Int32? func deletePIDFile() throws - func writePID(pid: pid_t) throws + func writePID(pid: Int32) throws func getCurrentPID() -> Int32 } @@ -61,13 +61,13 @@ public struct PIDFile: PIDFileHandler { } } - /// Get the current PID of the process + /// Get the current PID of the proces public func getCurrentPID() -> Int32 { - getpid() + return ProcessInfo.processInfo.processIdentifier } /// Write .pid file containing PID of process currently using .build directory - public func writePID(pid: pid_t) throws { + public func writePID(pid: Int32) throws { let parent = self.lockFilePath.parentDirectory try FileManager.default.createDirectory( at: parent.asURL, From 74793e862872bd708570f9f3f37e535a459d8933 Mon Sep 17 00:00:00 2001 From: John Bute Date: Mon, 12 May 2025 12:18:13 -0400 Subject: [PATCH 8/9] changed header of file --- Sources/Basics/Concurrency/PID.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index fe53a66a50e..6fb5aaf8228 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -1,12 +1,12 @@ //===----------------------------------------------------------------------===// // -// This source file is part of the Swift open source project +// This source file is part of the Swift.org open source project // -// Copyright (c) 2023 Apple Inc. and the Swift project authors +// Copyright (c) 2025 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // -// See http://swift.org/LICENSE.txt for license information -// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors // //===----------------------------------------------------------------------===// From 31cb5e3bcfd8256eeafbcb78578e80baa25e30c4 Mon Sep 17 00:00:00 2001 From: John Bute Date: Wed, 14 May 2025 13:05:04 -0400 Subject: [PATCH 9/9] added more tests and fixed diagnostics --- Sources/Basics/Concurrency/PID.swift | 42 ++--- Sources/CoreCommands/SwiftCommandState.swift | 12 +- Tests/BasicsTests/Concurrency/PIDTests.swift | 168 ++++++++++++++++++ .../SwiftCommandStateTests.swift | 39 ---- 4 files changed, 200 insertions(+), 61 deletions(-) create mode 100644 Tests/BasicsTests/Concurrency/PIDTests.swift diff --git a/Sources/Basics/Concurrency/PID.swift b/Sources/Basics/Concurrency/PID.swift index 6fb5aaf8228..38ab613cfd0 100644 --- a/Sources/Basics/Concurrency/PID.swift +++ b/Sources/Basics/Concurrency/PID.swift @@ -17,7 +17,7 @@ public protocol PIDFileHandler { init(scratchDirectory: AbsolutePath) - func readPID() -> Int32? + func readPID() throws -> Int32 func deletePIDFile() throws func writePID(pid: Int32) throws func getCurrentPID() -> Int32 @@ -36,34 +36,27 @@ public struct PIDFile: PIDFileHandler { } /// Read the pid file - public func readPID() -> Int32? { + public func readPID() throws -> Int32 { // Check if the file exists let filePath = self.lockFilePath.pathString guard FileManager.default.fileExists(atPath: filePath) else { - print("File does not exist at path: \(filePath)") - return nil + throw PIDError.noSuchPiDFile } - do { - // Read the contents of the file - let pidString = try String(contentsOf: lockFilePath.asURL, encoding: .utf8) - .trimmingCharacters(in: .whitespacesAndNewlines) - - // Check if the PID string can be converted to an Int32 - if let pid = Int32(pidString) { - return pid - } else { - return nil - } - } catch { - // Catch any errors and print them - return nil + let pidString = try String(contentsOf: lockFilePath.asURL, encoding: .utf8) + .trimmingCharacters(in: .whitespacesAndNewlines) + + // Try to convert to Int32, or throw an error + guard let pid = Int32(pidString) else { + throw PIDError.invalidPIDFormat } + + return pid } /// Get the current PID of the proces public func getCurrentPID() -> Int32 { - return ProcessInfo.processInfo.processIdentifier + ProcessInfo.processInfo.processIdentifier } /// Write .pid file containing PID of process currently using .build directory @@ -80,6 +73,15 @@ public struct PIDFile: PIDFileHandler { /// Delete PID file at URL public func deletePIDFile() throws { - try FileManager.default.removeItem(at: self.lockFilePath.asURL) + do { + try FileManager.default.removeItem(at: self.lockFilePath.asURL) + } catch { + throw PIDError.noSuchPiDFile + } + } + + public enum PIDError: Error { + case invalidPIDFormat + case noSuchPiDFile } } diff --git a/Sources/CoreCommands/SwiftCommandState.swift b/Sources/CoreCommands/SwiftCommandState.swift index 9be257a74c7..d8f46cc5584 100644 --- a/Sources/CoreCommands/SwiftCommandState.swift +++ b/Sources/CoreCommands/SwiftCommandState.swift @@ -1072,7 +1072,15 @@ public final class SwiftCommandState { lockAcquired = true } catch ProcessLockError.unableToAquireLock(let errno) { if errno == EWOULDBLOCK { - let existingProcessPID = self.pidManipulator.readPID() + var existingProcessPID: Int32? = nil + + // Attempt to read the PID + do { + existingProcessPID = try self.pidManipulator.readPID() + } catch { + self.observabilityScope.emit(debug: "Cannot read PID file: \(error)") + } + let pidInfo = existingProcessPID.map { "(PID: \($0)) " } ?? "" if self.options.locations.ignoreLock { self.outputStream @@ -1103,7 +1111,7 @@ public final class SwiftCommandState { do { try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID()) } catch { - self.observabilityScope.emit(warning: "Failed to write to PID file: \(error)") + self.observabilityScope.emit(debug: "Failed to write to PID file: \(error)") } } } diff --git a/Tests/BasicsTests/Concurrency/PIDTests.swift b/Tests/BasicsTests/Concurrency/PIDTests.swift new file mode 100644 index 00000000000..4c4aaded27d --- /dev/null +++ b/Tests/BasicsTests/Concurrency/PIDTests.swift @@ -0,0 +1,168 @@ +// +// PIDTests.swift +// SwiftPM +// +// Created by John Bute on 2025-05-14. +// + +import Basics +import Foundation +import Testing + +struct PIDTests { + @Test + func testWritePIDMultipleCalls() throws { + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidHandler = PIDFile(scratchDirectory: scratchPath) + + let pid1: Int32 = 1234 + let pid2: Int32 = 5678 + let pid3: Int32 = 9012 + + try pidHandler.writePID(pid: pid1) + try pidHandler.writePID(pid: pid2) + try pidHandler.writePID(pid: pid3) + + #expect(throws: Never.self) { + try pidHandler.readPID() + } + + let pid = try pidHandler.readPID() + + #expect(pid == pid3) + } + } + + @Test + func testDeleteExistingPIDFile() async throws { + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidHandler = PIDFile(scratchDirectory: scratchPath) + let currentPID = pidHandler.getCurrentPID() + try pidHandler.writePID(pid: currentPID) + + #expect(throws: Never.self) { + try pidHandler.deletePIDFile() + } + } + } + + @Test + func testDeleteNonExistingPIDFile() async throws { + try withTemporaryDirectory { tmpDir in + let filePath = tmpDir.appending(component: "scratch") + + let handler = PIDFile(scratchDirectory: filePath) + + #expect(throws: PIDFile.PIDError.noSuchPiDFile) { + try handler.deletePIDFile() + } + } + } + + @Test + func testFileDoesNotExist() throws { + // Create a temporary directory + try withTemporaryDirectory { tmpDir in + let filePath = tmpDir.appending(component: "scratch") + let handler = PIDFile(scratchDirectory: filePath) + + #expect(throws: PIDFile.PIDError.noSuchPiDFile) { + try handler.readPID() + } + } + } + + @Test + func testInvalidPIDFormat() async throws { + // Create a temporary directory + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidFilePath = scratchPath.appending(component: "PackageManager.lock.pid") + let handler = PIDFile(scratchDirectory: scratchPath) + + // Write invalid content (non-numeric PID) + let invalidPIDContent = "invalidPID" + try localFileSystem.writeFileContents(pidFilePath, bytes: .init(encodingAsUTF8: invalidPIDContent)) + + #expect(throws: PIDFile.PIDError.invalidPIDFormat) { + try handler.readPID() + } + } + } + + // Test case to check if the function works when a valid PID is in the file + @Test + func testValidPIDFormat() throws { + // Create a temporary directory + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidFilePath = scratchPath.appending(component: "PackageManager.lock.pid") + + let handler = PIDFile(scratchDirectory: scratchPath) + + // Write a valid PID content + let validPIDContent = "12345" + try localFileSystem.writeFileContents(pidFilePath, bytes: .init(encodingAsUTF8: validPIDContent)) + + let pid = try handler.readPID() + #expect(pid == 12345) + } + } + + @Test + func testPIDFileHandlerLifecycle() throws { + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidHandler = PIDFile(scratchDirectory: scratchPath) + + // Ensure no PID file exists initially + #expect(throws: PIDFile.PIDError.noSuchPiDFile) { + try pidHandler.readPID() + } + + // Write current PID + let currentPID = pidHandler.getCurrentPID() + try pidHandler.writePID(pid: currentPID) + + // Read PID back + let readPID = try pidHandler.readPID() + #expect(readPID == currentPID, "PID read should match written PID") + + // Delete the file + try pidHandler.deletePIDFile() + + // Ensure file is gone + #expect(throws: PIDFile.PIDError.noSuchPiDFile) { + try pidHandler.readPID() + } + } + } + + @Test + func testMalformedPIDFile() throws { + try withTemporaryDirectory { tmpDir in + let scratchPath = tmpDir.appending(component: "scratch") + try localFileSystem.createDirectory(scratchPath) + + let pidPath = scratchPath.appending(component: "PackageManager.lock.pid") + try localFileSystem.writeFileContents(pidPath, bytes: "notanumber") + + let pidHandler = PIDFile(scratchDirectory: scratchPath) + #expect(throws: PIDFile.PIDError.invalidPIDFormat) { + try pidHandler.readPID() + } + } + } +} diff --git a/Tests/CommandsTests/SwiftCommandStateTests.swift b/Tests/CommandsTests/SwiftCommandStateTests.swift index 4acb84d9caa..c5dc4431182 100644 --- a/Tests/CommandsTests/SwiftCommandStateTests.swift +++ b/Tests/CommandsTests/SwiftCommandStateTests.swift @@ -533,45 +533,6 @@ final class SwiftCommandStateTests: CommandsTestCase { } } } - - func testPIDFileHandlerLifecycle() async throws { - try withTemporaryDirectory { tmpDir in - let scratchPath = tmpDir.appending(component: "scratch") - try localFileSystem.createDirectory(scratchPath) - - let pidHandler = PIDFile(scratchDirectory: scratchPath) - - // Ensure no PID file exists initially - XCTAssertNil(pidHandler.readPID(), "No PID should exist initially") - - // Write current PID - let currentPID = pidHandler.getCurrentPID() - try pidHandler.writePID(pid: currentPID) - - // Read PID back - let readPID = pidHandler.readPID() - XCTAssertEqual(readPID, currentPID, "PID read should match written PID") - - // Delete the file - try pidHandler.deletePIDFile() - - // Ensure file is gone - XCTAssertNil(pidHandler.readPID(), "PID should be nil after deletion") - } - } - - func testMalformedPIDFile() async throws { - try withTemporaryDirectory { tmpDir in - let scratchPath = tmpDir.appending(component: "scratch") - try localFileSystem.createDirectory(scratchPath) - - let pidPath = scratchPath.appending(component: "PackageManager.lock.pid") - try localFileSystem.writeFileContents(pidPath, bytes: "notanumber") - - let pidHandler = PIDFile(scratchDirectory: scratchPath) - XCTAssertNil(pidHandler.readPID(), "Malformed PID file should result in nil") - } - } } extension SwiftCommandState {