From 6c548c32dce0225186d6cb542424a01b4cf4f365 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Wed, 23 Apr 2025 08:50:45 -0400 Subject: [PATCH 1/3] Add a modeled command-line for better checking Swiftly has many different commands that it invokes in different parts of its code base, supporting tooling, and tests. This list includes tar, git, pkgutil, and dscl among oseveral others. For the most part these tools are called using arrays that are manually assembled at the call site. In order to exercise that callsite in tests the test must necessarily become "medium" size because it involves integration testing with that tool, instead of "small", which is where most tests should be. A small test of that array would verify that the length has the correct arguments, number, spelling, and order. Also, there are various types of string interpolation in these arrays, which limit the type-safety. Introduce a new way of modeling command-line invocations using Swift types. Commands and subcommands are modeled using structs. Use functions to assemble the structs, and methods to assemble structs for subcommands. Arguments become function arguments, with default values if they are optional. Flags, and options will be described using enum cases with carrying values for options. Some commands are runnable, while others are not. Introduce a Runnable protocol that marks runnable commands, and provides a default implementation of a run() method to run the assembled command. Other commands can produce output. Introduce an Output protocol and default implementation that runs the command and receives the output as a string that the callsite can use. Some callsites make decisions based on whether a tool is present or not. Provide a convenient way to verify whether a command exists on the system. Use this mechanism to skip tests when they are run in an environment that doesn't have the tool. Pick one command, dscl, to use with the new modeled command-line. Write small tests that don't invoke the tool. Instead they verify the configuration that is produced with different typical inputs from swiftly. Add a single medium size test that runs dscl from a modeled command, enabled only if dscl exists on the system. Add tags for medium, and large tests. Mark existing large tests, which is the HTTPClientTests. Leave all of the remaining tests as untagged, (i.e. small). Mark the CommandLineTest that invokes the dscl tool as medium. --- Package.swift | 3 + Sources/MacOSPlatform/MacOS.swift | 11 +- Sources/SwiftlyCore/Commands.swift | 87 ++++++++ Sources/SwiftlyCore/ModeledCommandLine.swift | 198 ++++++++++++++++++ Tests/SwiftlyTests/CommandLineTests.swift | 31 +++ Tests/SwiftlyTests/HTTPClientTests.swift | 9 +- Tests/SwiftlyTests/SwiftlyTests.swift | 5 + .../BuildSwiftlyRelease.swift | 50 ++--- 8 files changed, 348 insertions(+), 46 deletions(-) create mode 100644 Sources/SwiftlyCore/Commands.swift create mode 100644 Sources/SwiftlyCore/ModeledCommandLine.swift create mode 100644 Tests/SwiftlyTests/CommandLineTests.swift diff --git a/Package.swift b/Package.swift index 6688b6a3..d8896350 100644 --- a/Package.swift +++ b/Package.swift @@ -117,6 +117,9 @@ let package = Package( .executableTarget( name: "build-swiftly-release", dependencies: [ + .target(name: "SwiftlyCore"), + .target(name: "LinuxPlatform", condition: .when(platforms: [.linux])), + .target(name: "MacOSPlatform", condition: .when(platforms: [.macOS])), .product(name: "ArgumentParser", package: "swift-argument-parser"), ], path: "Tools/build-swiftly-release" diff --git a/Sources/MacOSPlatform/MacOS.swift b/Sources/MacOSPlatform/MacOS.swift index c7a02158..13fa2fef 100644 --- a/Sources/MacOSPlatform/MacOS.swift +++ b/Sources/MacOSPlatform/MacOS.swift @@ -2,6 +2,7 @@ import Foundation import SwiftlyCore import SystemPackage +typealias sys = SwiftlyCore.SystemCommand typealias fs = SwiftlyCore.FileSystem public struct SwiftPkgInfo: Codable { @@ -191,13 +192,9 @@ public struct MacOS: Platform { } public func getShell() async throws -> String { - if let directoryInfo = try await runProgramOutput("dscl", ".", "-read", "\(fs.home)") { - for line in directoryInfo.components(separatedBy: "\n") { - if line.hasPrefix("UserShell: ") { - if case let comps = line.components(separatedBy: ": "), comps.count == 2 { - return comps[1] - } - } + for (key, value) in try await sys.dscl(datasource: ".").read(path: fs.home).properties(self) { + if key == "UserShell" { + return value } } diff --git a/Sources/SwiftlyCore/Commands.swift b/Sources/SwiftlyCore/Commands.swift new file mode 100644 index 00000000..2aa54a79 --- /dev/null +++ b/Sources/SwiftlyCore/Commands.swift @@ -0,0 +1,87 @@ +import Foundation +import SystemPackage + +// Directory Service command line utility for macOS +public enum SystemCommand { + public static func dscl(executable: Executable = DsclCommand.defaultExecutable, datasource: String? = nil) -> DsclCommand { + DsclCommand(executable: executable, datasource: datasource) + } + + public struct DsclCommand { + public static var defaultExecutable: Executable { .name("dscl") } + + var executable: Executable + var datasource: String? + + internal init( + executable: Executable, + datasource: String? + ) { + self.executable = executable + self.datasource = datasource + } + + func config() -> Configuration { + var args: [String] = [] + + if let datasource = self.datasource { + args += [datasource] + } + + return Configuration( + executable: self.executable, + arguments: .init(args), + environment: .inherit + ) + } + + public func read(path: FilePath? = nil, keys: [String]) -> ReadCommand { + ReadCommand(dscl: self, path: path, keys: keys) + } + + public func read(path: FilePath? = nil, keys: String...) -> ReadCommand { + self.read(path: path, keys: keys) + } + + public struct ReadCommand: Output { + var dscl: DsclCommand + var path: FilePath? + var keys: [String] + + internal init(dscl: DsclCommand, path: FilePath?, keys: [String]) { + self.dscl = dscl + self.path = path + self.keys = keys + } + + public func config() -> Configuration { + var c = self.dscl.config() + + var args = c.arguments.storage.map(\.description) + ["-read"] + + if let path = self.path { + args += [path.string] + self.keys + } + + c.arguments = .init(args) + + return c + } + } + } +} + +extension SystemCommand.DsclCommand.ReadCommand { + public func properties(_ p: Platform) async throws -> [(key: String, value: String)] { + let output = try await self.output(p) + guard let output else { return [] } + + var props: [(key: String, value: String)] = [] + for line in output.components(separatedBy: "\n") { + if case let comps = line.components(separatedBy: ": "), comps.count == 2 { + props.append((key: comps[0], value: comps[1])) + } + } + return props + } +} diff --git a/Sources/SwiftlyCore/ModeledCommandLine.swift b/Sources/SwiftlyCore/ModeledCommandLine.swift new file mode 100644 index 00000000..1dae228f --- /dev/null +++ b/Sources/SwiftlyCore/ModeledCommandLine.swift @@ -0,0 +1,198 @@ +import Foundation +import SystemPackage + +public enum CommandLineError: Error { + case invalidArgs + case errorExit(exitCode: Int32, program: String) + case unknownVersion +} + +// This section is a clone of the Configuration type from the new Subprocess package, until we can depend on that package. +public struct Configuration: Sendable { + /// The executable to run. + public var executable: Executable + /// The arguments to pass to the executable. + public var arguments: Arguments + /// The environment to use when running the executable. + public var environment: Environment +} + +public struct Executable: Sendable, Hashable { + internal enum Storage: Sendable, Hashable { + case executable(String) + case path(FilePath) + } + + internal let storage: Storage + + private init(_config: Storage) { + self.storage = _config + } + + /// Locate the executable by its name. + /// `Subprocess` will use `PATH` value to + /// determine the full path to the executable. + public static func name(_ executableName: String) -> Self { + .init(_config: .executable(executableName)) + } + + /// Locate the executable by its full path. + /// `Subprocess` will use this path directly. + public static func path(_ filePath: FilePath) -> Self { + .init(_config: .path(filePath)) + } +} + +public struct Environment: Sendable, Hashable { + internal enum Configuration: Sendable, Hashable { + case inherit([String: String]) + case custom([String: String]) + } + + internal let config: Configuration + + init(config: Configuration) { + self.config = config + } + + /// Child process should inherit the same environment + /// values from its parent process. + public static var inherit: Self { + .init(config: .inherit([:])) + } + + /// Override the provided `newValue` in the existing `Environment` + public func updating(_ newValue: [String: String]) -> Self { + .init(config: .inherit(newValue)) + } + + /// Use custom environment variables + public static func custom(_ newValue: [String: String]) -> Self { + .init(config: .custom(newValue)) + } +} + +internal enum StringOrRawBytes: Sendable, Hashable { + case string(String) + + var stringValue: String? { + switch self { + case let .string(string): + return string + } + } + + var description: String { + switch self { + case let .string(string): + return string + } + } + + var count: Int { + switch self { + case let .string(string): + return string.count + } + } + + func hash(into hasher: inout Hasher) { + // If Raw bytes is valid UTF8, hash it as so + switch self { + case let .string(string): + hasher.combine(string) + } + } +} + +public struct Arguments: Sendable, ExpressibleByArrayLiteral, Hashable { + public typealias ArrayLiteralElement = String + + internal let storage: [StringOrRawBytes] + + /// Create an Arguments object using the given literal values + public init(arrayLiteral elements: String...) { + self.storage = elements.map { .string($0) } + } + + /// Create an Arguments object using the given array + public init(_ array: [String]) { + self.storage = array.map { .string($0) } + } +} + +extension Executable { + public func exists() async throws -> Bool { + switch self.storage { + case let .path(p): + return (try await FileSystem.exists(atPath: p)) + case let .executable(e): + let path = ProcessInfo.processInfo.environment["PATH"] + + guard let path else { return false } + + for p in path.split(separator: ":") { + if try await FileSystem.exists(atPath: FilePath(String(p)) / e) { + return true + } + } + + return false + } + } +} + +public protocol Runnable { + func config() -> Configuration +} + +extension Runnable { + public func run(_ p: Platform, quiet: Bool) async throws { + let c = self.config() + let executable = switch c.executable.storage { + case let .executable(name): + name + case let .path(p): + p.string + } + let args = c.arguments.storage.map(\.description) + var env: [String: String] = ProcessInfo.processInfo.environment + switch c.environment.config { + case let .inherit(newValue): + for (key, value) in newValue { + env[key] = value + } + case let .custom(newValue): + env = newValue + } + try await p.runProgram([executable] + args, quiet: quiet, env: env) + } +} + +public protocol Output { + func config() -> Configuration +} + +// TODO: look into making this something that can be Decodable (i.e. streamable) +extension Output { + public func output(_ p: Platform) async throws -> String? { + let c = self.config() + let executable = switch c.executable.storage { + case let .executable(name): + name + case let .path(p): + p.string + } + let args = c.arguments.storage.map(\.description) + var env: [String: String] = ProcessInfo.processInfo.environment + switch c.environment.config { + case let .inherit(newValue): + for (key, value) in newValue { + env[key] = value + } + case let .custom(newValue): + env = newValue + } + return try await p.runProgramOutput(executable, args, env: env) + } +} diff --git a/Tests/SwiftlyTests/CommandLineTests.swift b/Tests/SwiftlyTests/CommandLineTests.swift new file mode 100644 index 00000000..62e2aecf --- /dev/null +++ b/Tests/SwiftlyTests/CommandLineTests.swift @@ -0,0 +1,31 @@ +@testable import Swiftly +@testable import SwiftlyCore +import SystemPackage +import Testing + +public typealias sys = SystemCommand + +@Suite +public struct CommandLineTests { + @Test func testDsclModel() { + var config = sys.dscl(datasource: ".").read(path: .init("/Users/swiftly"), keys: "UserShell").config() + #expect(config.executable == .name("dscl")) + #expect(config.arguments.storage.map(\.description) == [".", "-read", "/Users/swiftly", "UserShell"]) + + config = sys.dscl(datasource: ".").read(path: .init("/Users/swiftly"), keys: "UserShell", "Picture").config() + #expect(config.executable == .name("dscl")) + #expect(config.arguments.storage.map(\.description) == [".", "-read", "/Users/swiftly", "UserShell", "Picture"]) + } + + @Test( + .tags(.medium), + .enabled { + try await sys.DsclCommand.defaultExecutable.exists() + } + ) + func testDscl() async throws { + let properties = try await sys.dscl(datasource: ".").read(path: fs.home, keys: "UserShell").properties(Swiftly.currentPlatform) + #expect(properties.count == 1) // Only one shell for the current user + #expect(properties[0].key == "UserShell") // The one property key should be the one that is requested + } +} diff --git a/Tests/SwiftlyTests/HTTPClientTests.swift b/Tests/SwiftlyTests/HTTPClientTests.swift index c40235f4..96a0472d 100644 --- a/Tests/SwiftlyTests/HTTPClientTests.swift +++ b/Tests/SwiftlyTests/HTTPClientTests.swift @@ -7,7 +7,7 @@ import SystemPackage import Testing @Suite(.serialized) struct HTTPClientTests { - @Test func getSwiftOrgGPGKeys() async throws { + @Test(.tags(.large)) func getSwiftOrgGPGKeys() async throws { let tmpFile = fs.mktemp() try await fs.create(file: tmpFile, contents: nil) @@ -24,7 +24,7 @@ import Testing } } - @Test func getSwiftToolchain() async throws { + @Test(.tags(.large)) func getSwiftToolchain() async throws { let tmpFile = fs.mktemp() try await fs.create(file: tmpFile, contents: nil) let tmpFileSignature = fs.mktemp(ext: ".sig") @@ -53,7 +53,7 @@ import Testing } } - @Test func getSwiftlyRelease() async throws { + @Test(.tags(.large)) func getSwiftlyRelease() async throws { let tmpFile = fs.mktemp() try await fs.create(file: tmpFile, contents: nil) let tmpFileSignature = fs.mktemp(ext: ".sig") @@ -82,7 +82,7 @@ import Testing } } - @Test func getSwiftlyReleaseMetadataFromSwiftOrg() async throws { + @Test(.tags(.large)) func getSwiftlyReleaseMetadataFromSwiftOrg() async throws { let httpClient = SwiftlyHTTPClient(httpRequestExecutor: HTTPRequestExecutorImpl()) do { let currentRelease = try await httpClient.getCurrentSwiftlyRelease() @@ -94,6 +94,7 @@ import Testing } @Test( + .tags(.large), arguments: [PlatformDefinition.macOS, .ubuntu2404, .ubuntu2204, .rhel9, .fedora39, .amazonlinux2, .debian12], [SwiftlyWebsiteAPI.Components.Schemas.Architecture.x8664, .aarch64] diff --git a/Tests/SwiftlyTests/SwiftlyTests.swift b/Tests/SwiftlyTests/SwiftlyTests.swift index 1c49e7f5..3eebe26a 100644 --- a/Tests/SwiftlyTests/SwiftlyTests.swift +++ b/Tests/SwiftlyTests/SwiftlyTests.swift @@ -20,6 +20,11 @@ struct SwiftlyTestError: LocalizedError { let message: String } +extension Tag { + @Tag static var medium: Self + @Tag static var large: Self +} + let unmockedMsg = "All swiftly test case logic must be mocked in order to prevent mutation of the system running the test. This test must either run swiftly components inside a SwiftlyTests.with... closure, or it must have one of the @Test traits, such as @Test(.testHome), or @Test(.mock...)" actor OutputHandlerFail: OutputHandler { diff --git a/Tools/build-swiftly-release/BuildSwiftlyRelease.swift b/Tools/build-swiftly-release/BuildSwiftlyRelease.swift index c4d2bc2c..cf6219b0 100644 --- a/Tools/build-swiftly-release/BuildSwiftlyRelease.swift +++ b/Tools/build-swiftly-release/BuildSwiftlyRelease.swift @@ -1,5 +1,18 @@ import ArgumentParser import Foundation +import SwiftlyCore + +#if os(macOS) +import MacOSPlatform +#elseif os(Linux) +import LinuxPlatform +#endif + +#if os(macOS) +let currentPlatform = MacOS() +#elseif os(Linux) +let currentPlatform = Linux() +#endif public struct SwiftPlatform: Codable { public var name: String? @@ -110,39 +123,6 @@ public func runProgramOutput(_ program: String, _ args: String...) async throws } } -#if os(macOS) -public func getShell() async throws -> String { - if let directoryInfo = try await runProgramOutput("dscl", ".", "-read", FileManager.default.homeDirectoryForCurrentUser.path) { - for line in directoryInfo.components(separatedBy: "\n") { - if line.hasPrefix("UserShell: ") { - if case let comps = line.components(separatedBy: ": "), comps.count == 2 { - return comps[1] - } - } - } - } - - // Fall back to zsh on macOS - return "/bin/zsh" -} - -#elseif os(Linux) -public func getShell() async throws -> String { - if let passwds = try await runProgramOutput("getent", "passwd") { - for line in passwds.components(separatedBy: "\n") { - if line.hasPrefix("root:") { - if case let comps = line.components(separatedBy: ":"), comps.count > 1 { - return comps[comps.count - 1] - } - } - } - } - - // Fall back on bash on Linux and other Unixes - return "/bin/bash" -} -#endif - @main struct BuildSwiftlyRelease: AsyncParsableCommand { static let configuration = CommandConfiguration( @@ -183,11 +163,11 @@ struct BuildSwiftlyRelease: AsyncParsableCommand { } func assertTool(_ name: String, message: String) async throws -> String { - guard let _ = try? await runProgramOutput(getShell(), "-c", "which which") else { + guard let _ = try? await runProgramOutput(currentPlatform.getShell(), "-c", "which which") else { throw Error(message: "The which command could not be found. Please install it with your package manager.") } - guard let location = try? await runProgramOutput(getShell(), "-c", "which \(name)") else { + guard let location = try? await runProgramOutput(currentPlatform.getShell(), "-c", "which \(name)") else { throw Error(message: message) } From 7bfcbb59a747ea0181c2471372cb0482d0270f56 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Wed, 23 Apr 2025 15:11:36 -0400 Subject: [PATCH 2/3] Code review feedback --- Sources/SwiftlyCore/Commands.swift | 9 +++++++-- Sources/SwiftlyCore/ModeledCommandLine.swift | 21 -------------------- Tests/SwiftlyTests/SwiftlyTests.swift | 21 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/Sources/SwiftlyCore/Commands.swift b/Sources/SwiftlyCore/Commands.swift index 2aa54a79..641fcfbd 100644 --- a/Sources/SwiftlyCore/Commands.swift +++ b/Sources/SwiftlyCore/Commands.swift @@ -1,8 +1,13 @@ import Foundation import SystemPackage +public enum SystemCommand {} + +// This file contains a set of system commands that's used by Swiftly and its related tests and tooling + // Directory Service command line utility for macOS -public enum SystemCommand { +// See dscl(1) for details +extension SystemCommand { public static func dscl(executable: Executable = DsclCommand.defaultExecutable, datasource: String? = nil) -> DsclCommand { DsclCommand(executable: executable, datasource: datasource) } @@ -30,7 +35,7 @@ public enum SystemCommand { return Configuration( executable: self.executable, - arguments: .init(args), + arguments: Arguments(args), environment: .inherit ) } diff --git a/Sources/SwiftlyCore/ModeledCommandLine.swift b/Sources/SwiftlyCore/ModeledCommandLine.swift index 1dae228f..a7a21a9b 100644 --- a/Sources/SwiftlyCore/ModeledCommandLine.swift +++ b/Sources/SwiftlyCore/ModeledCommandLine.swift @@ -121,27 +121,6 @@ public struct Arguments: Sendable, ExpressibleByArrayLiteral, Hashable { } } -extension Executable { - public func exists() async throws -> Bool { - switch self.storage { - case let .path(p): - return (try await FileSystem.exists(atPath: p)) - case let .executable(e): - let path = ProcessInfo.processInfo.environment["PATH"] - - guard let path else { return false } - - for p in path.split(separator: ":") { - if try await FileSystem.exists(atPath: FilePath(String(p)) / e) { - return true - } - } - - return false - } - } -} - public protocol Runnable { func config() -> Configuration } diff --git a/Tests/SwiftlyTests/SwiftlyTests.swift b/Tests/SwiftlyTests/SwiftlyTests.swift index 3eebe26a..e74ee1a9 100644 --- a/Tests/SwiftlyTests/SwiftlyTests.swift +++ b/Tests/SwiftlyTests/SwiftlyTests.swift @@ -25,6 +25,27 @@ extension Tag { @Tag static var large: Self } +extension Executable { + public func exists() async throws -> Bool { + switch self.storage { + case let .path(p): + return (try await FileSystem.exists(atPath: p)) + case let .executable(e): + let path = ProcessInfo.processInfo.environment["PATH"] + + guard let path else { return false } + + for p in path.split(separator: ":") { + if try await FileSystem.exists(atPath: FilePath(String(p)) / e) { + return true + } + } + + return false + } + } +} + let unmockedMsg = "All swiftly test case logic must be mocked in order to prevent mutation of the system running the test. This test must either run swiftly components inside a SwiftlyTests.with... closure, or it must have one of the @Test traits, such as @Test(.testHome), or @Test(.mock...)" actor OutputHandlerFail: OutputHandler { From aef47b73950374fd0f862472f1b69fd072f3dd36 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Wed, 23 Apr 2025 15:39:07 -0400 Subject: [PATCH 3/3] Use the UserShell key in order to limit the output of the dscl command --- Sources/MacOSPlatform/MacOS.swift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Sources/MacOSPlatform/MacOS.swift b/Sources/MacOSPlatform/MacOS.swift index 13fa2fef..6d289b81 100644 --- a/Sources/MacOSPlatform/MacOS.swift +++ b/Sources/MacOSPlatform/MacOS.swift @@ -192,10 +192,8 @@ public struct MacOS: Platform { } public func getShell() async throws -> String { - for (key, value) in try await sys.dscl(datasource: ".").read(path: fs.home).properties(self) { - if key == "UserShell" { - return value - } + for (key, value) in try await sys.dscl(datasource: ".").read(path: fs.home, keys: "UserShell").properties(self) { + return value } // Fall back to zsh on macOS