From 7a8ce148660700e52c4f7d67aa80b17f1bda6afc Mon Sep 17 00:00:00 2001 From: Alberto De Bortoli Date: Fri, 23 Jan 2026 22:55:47 +0100 Subject: [PATCH] Unlink unlisted tools --- .../LucaCore/Core/Installer/Installer.swift | 40 +++- .../Core/Uninstaller/Uninstaller.swift | 2 +- Sources/LucaCore/Core/Unlinker/Unlinker.swift | 3 +- Sources/LucaCore/Models/Tool.swift | 10 + Tests/Core/InstallerTests.swift | 171 ++++++++++++++++++ .../Lucafiles/Lucafile_valid_subset.yml | 12 ++ 6 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 Tests/Fixtures/Lucafiles/Lucafile_valid_subset.yml diff --git a/Sources/LucaCore/Core/Installer/Installer.swift b/Sources/LucaCore/Core/Installer/Installer.swift index 29cb82e..77aec26 100644 --- a/Sources/LucaCore/Core/Installer/Installer.swift +++ b/Sources/LucaCore/Core/Installer/Installer.swift @@ -24,6 +24,8 @@ public struct Installer { private let downloader: Downloading private let permissionManager: PermissionManaging private let symLinker: SymLinking + private let linkedToolsLister: LinkedToolsLister + private let unlinker: Unlinker private let ignoreArchitectureCheck: Bool public init(fileManager: FileManaging, ignoreArchitectureCheck: Bool, printer: Printing) { @@ -36,6 +38,8 @@ public struct Installer { self.downloader = Downloader(fileDownloader: fileDownloader) self.permissionManager = PermissionManager(fileManager: fileManager) self.symLinker = SymLinker(fileManager: fileManager) + self.linkedToolsLister = LinkedToolsLister(fileManager: fileManager) + self.unlinker = Unlinker(fileManager: fileManager, printer: printer) self.ignoreArchitectureCheck = ignoreArchitectureCheck } @@ -44,10 +48,20 @@ public struct Installer { let releaseInfoProvider = ReleaseInfoProvider(dataDownloader: dataDownloader) let specLoader = SpecLoader(fileManager: .default) let toolFactory = ToolFactory(releaseInfoProvider: releaseInfoProvider, specLoader: specLoader) - printer.printFormatted("\(.raw("๐Ÿง  Detecting tools to install..."))") + + printer.printFormatted("\(.info("๐Ÿง  Detecting tools to install..."))") + printer.printFormatted("") + let tools = try await toolFactory.toolsForInstallationType(installationType) - printer.printFormatted("\(.raw("๐Ÿƒโ€โ™‚๏ธ Installing tools for the current project."))") + + // Unlink orphaned tools only when installing from a spec + if case .spec = installationType { + try unlinkOrphanedTools(specTools: tools) + } + + printer.printFormatted("\(.info("๐Ÿƒโ€โ™‚๏ธ Installing tools for the current project."))") printer.printFormatted("") + try await installTools(tools) } @@ -86,7 +100,7 @@ public struct Installer { let symLink = try symLinker.setSymLink(for: enrichedTool) printer.printFormatted("\(.raw("๐Ÿ”— Recreated symlink at \(symLink.path)"))") - printer.printFormatted("\(.success("๐Ÿ™Œ Tool \(tool.name) version \(tool.version) installed for the current project."))") + printer.printFormatted("\(.primary("๐Ÿ™Œ Tool \(tool.name) version \(tool.version) installed for the current project."))") } private func install(_ tool: Tool) async throws { @@ -114,7 +128,7 @@ public struct Installer { case .executable: try installExecutable(tool: tool, downloadedFile: downloadedFile, installationDestination: installationDestination) } - printer.printFormatted("\(.success("๐Ÿ™Œ Tool \(tool.name) version \(tool.version) installed for the current project."))") + printer.printFormatted("\(.primary("๐Ÿ™Œ Tool \(tool.name) version \(tool.version) installed for the current project."))") } private func installArchive(tool: Tool, downloadedFile: URL, installationDestination: URL) throws { @@ -218,4 +232,22 @@ public struct Installer { } } } + + private func unlinkOrphanedTools(specTools: [Tool]) throws { + let linkedTools = try linkedToolsLister.linkedTools() + + // Build a set of tool names from spec for efficient lookup + let specToolNames: Set = Set(specTools.map(\.name)) + + // Find linked tools that are not in the spec + let orphanedTools = linkedTools.filter { linkedTool in + !specToolNames.contains(linkedTool.name) + } + + // Unlink each orphaned tool + for orphanedTool in orphanedTools { + printer.printFormatted("\(.raw("๐Ÿงน Unlinking \(orphanedTool.binaryName) (removed from spec)..."))") + try unlinker.unlink(symlink: orphanedTool.binaryName) + } + } } diff --git a/Sources/LucaCore/Core/Uninstaller/Uninstaller.swift b/Sources/LucaCore/Core/Uninstaller/Uninstaller.swift index 85c8c41..3ba8166 100644 --- a/Sources/LucaCore/Core/Uninstaller/Uninstaller.swift +++ b/Sources/LucaCore/Core/Uninstaller/Uninstaller.swift @@ -31,7 +31,7 @@ public struct Uninstaller { if fileManager.fileExists(atPath: versionFolder.path) { printer.printFormatted("\(.raw("๐Ÿ‘€ Uninstalling \(tool) \(version)..."))") try fileManager.removeItem(at: versionFolder) - printer.printFormatted("\(.success("๐Ÿ™Œ \(tool) \(version) has been uninstalled."))") + printer.printFormatted("\(.primary("๐Ÿ™Œ \(tool) \(version) has been uninstalled."))") // Clean up tool folder if empty let toolFolder = fileManager.toolsFolder.appending(component: tool) diff --git a/Sources/LucaCore/Core/Unlinker/Unlinker.swift b/Sources/LucaCore/Core/Unlinker/Unlinker.swift index d3edfe2..29f16d4 100644 --- a/Sources/LucaCore/Core/Unlinker/Unlinker.swift +++ b/Sources/LucaCore/Core/Unlinker/Unlinker.swift @@ -28,7 +28,8 @@ public struct Unlinker { if fileManager.fileExists(atPath: symlinkFile.path) { printer.printFormatted("\(.raw("๐Ÿ‘€ Removing symlink \(symlink)..."))") try fileManager.removeItem(at: symlinkFile) - printer.printFormatted("\(.success("๐Ÿ™Œ Symlink \(symlink) has been removed."))") + printer.printFormatted("\(.primary("๐Ÿ™Œ Symlink \(symlink) has been removed."))") + printer.printFormatted("") } else { throw UninstallerError.symlinkNotFound(symlink: symlink) } diff --git a/Sources/LucaCore/Models/Tool.swift b/Sources/LucaCore/Models/Tool.swift index de68eab..6df8cb8 100644 --- a/Sources/LucaCore/Models/Tool.swift +++ b/Sources/LucaCore/Models/Tool.swift @@ -20,6 +20,16 @@ struct Tool: Codable { let algorithm: ChecksumAlgorithm? } +extension Tool { + /// Resolves the expected binary name for comparison with linked tools. + /// Priority: desiredBinaryName > binaryPath basename > tool name. + var expectedBinaryName: String { + if let desiredBinaryName { return desiredBinaryName } + if let binaryPath { return URL(fileURLWithPath: binaryPath).lastPathComponent } + return name + } +} + struct EnrichedTool: Codable { /// Logical name of the tool (used for directory hierarchy). let name: String diff --git a/Tests/Core/InstallerTests.swift b/Tests/Core/InstallerTests.swift index 739cd7b..1e890ae 100644 --- a/Tests/Core/InstallerTests.swift +++ b/Tests/Core/InstallerTests.swift @@ -217,6 +217,177 @@ struct InstallerTests { } } + @Test + func test_installSpec_unlinksOrphanedTools() async throws { + let installer = Installer(fileManager: fileManager, ignoreArchitectureCheck: true, printer: PrinterMock()) + + // First, install the full spec with all tools + let fullFixture = Fixture(filename: "Lucafile_valid", type: "yml") + let bundle = Bundle.module + let fullPath = try #require(bundle.path(forResource: fullFixture.filename, ofType: fullFixture.type)) + let fullSpec = try spec(for: fullFixture) + + try await installer.install(installationType: .spec(specPath: URL(string: fullPath)!)) + + // Verify all tools are installed and linked + for tool in fullSpec.tools { + let toolSymLink = fileManager.activeFolder + .appending(component: tool.expectedBinaryName) + #expect(fileManager.fileExists(atPath: toolSymLink.path)) + } + + // Now install a subset spec (missing PackageGenerator and ToggleGen) + let subsetFixture = Fixture(filename: "Lucafile_valid_subset", type: "yml") + let subsetPath = try #require(bundle.path(forResource: subsetFixture.filename, ofType: subsetFixture.type)) + let subsetSpec = try spec(for: subsetFixture) + + try await installer.install(installationType: .spec(specPath: URL(string: subsetPath)!)) + + // Verify tools in subset spec are still linked + for tool in subsetSpec.tools { + let toolSymLink = fileManager.activeFolder + .appending(component: tool.expectedBinaryName) + #expect(fileManager.fileExists(atPath: toolSymLink.path)) + } + + // Verify tools NOT in subset spec have been unlinked (by tool name, not binary name) + let subsetToolNames = Set(subsetSpec.tools.map(\.name)) + let removedTools = fullSpec.tools.filter { !subsetToolNames.contains($0.name) } + + for tool in removedTools { + let toolSymLink = fileManager.activeFolder + .appending(component: tool.expectedBinaryName) + #expect(!fileManager.fileExists(atPath: toolSymLink.path)) + } + } + + @Test + func test_installIndividual_doesNotUnlinkExistingTools() async throws { + let installer = Installer(fileManager: fileManager, ignoreArchitectureCheck: true, printer: PrinterMock()) + + // First, install a spec with multiple tools + let fullFixture = Fixture(filename: "Lucafile_valid", type: "yml") + let bundle = Bundle.module + let fullPath = try #require(bundle.path(forResource: fullFixture.filename, ofType: fullFixture.type)) + let fullSpec = try spec(for: fullFixture) + + try await installer.install(installationType: .spec(specPath: URL(string: fullPath)!)) + + // Install an individual tool (not from the spec) + let swiftLintFixture = Fixture(filename: "Lucafile_LowVersion", type: "yml") + let swiftLintSpec = try spec(for: swiftLintFixture) + let swiftLintTool = swiftLintSpec.tools.first! + + try await installer.install( + installationType: .individualInline( + name: swiftLintTool.name, + version: swiftLintTool.version, + url: swiftLintTool.url, + binaryPath: swiftLintTool.binaryPath, + desiredBinaryName: swiftLintTool.desiredBinaryName, + checksum: swiftLintTool.checksum, + algorithm: swiftLintTool.algorithm + ) + ) + + // Verify ALL original tools from the full spec are still linked + // (individual installs should NOT unlink existing tools) + for tool in fullSpec.tools { + let toolSymLink = fileManager.activeFolder + .appending(component: tool.expectedBinaryName) + #expect(fileManager.fileExists(atPath: toolSymLink.path)) + } + + // Verify the new individual tool is also linked + let swiftLintSymLink = fileManager.activeFolder + .appending(component: swiftLintTool.expectedBinaryName) + #expect(fileManager.fileExists(atPath: swiftLintSymLink.path)) + } + + @Test + func test_installSpec_noOrphanedTools() async throws { + let installer = Installer(fileManager: fileManager, ignoreArchitectureCheck: true, printer: PrinterMock()) + + // Install a spec + let fixture = Fixture(filename: "Lucafile_valid", type: "yml") + let bundle = Bundle.module + let path = try #require(bundle.path(forResource: fixture.filename, ofType: fixture.type)) + let spec = try spec(for: fixture) + + try await installer.install(installationType: .spec(specPath: URL(string: path)!)) + + // Install the same spec again + try await installer.install(installationType: .spec(specPath: URL(string: path)!)) + + // All tools should still be linked (no orphans to unlink) + for tool in spec.tools { + let toolSymLink = fileManager.activeFolder + .appending(component: tool.expectedBinaryName) + #expect(fileManager.fileExists(atPath: toolSymLink.path)) + } + } + + @Test + func test_installSpec_doesNotUnlinkToolsWithDifferentBinaryNameCasing() async throws { + // This test verifies that tools are not incorrectly unlinked when the symlink name + // differs from the tool name (e.g., "swiftlint" symlink vs "SwiftLint" tool name). + // The orphan detection should compare by tool name, not binary name. + + let installer = Installer(fileManager: fileManager, ignoreArchitectureCheck: true, printer: PrinterMock()) + + // Install SwiftLint from the spec + let fixture = Fixture(filename: "Lucafile_LowVersion", type: "yml") + let bundle = Bundle.module + let path = try #require(bundle.path(forResource: fixture.filename, ofType: fixture.type)) + + try await installer.install(installationType: .spec(specPath: URL(string: path)!)) + + // The symlink is created with the binary name (lowercase "swiftlint") + // but the tool name in the spec is "SwiftLint" + let swiftlintSymLink = fileManager.activeFolder.appending(component: "swiftlint") + #expect(fileManager.fileExists(atPath: swiftlintSymLink.path)) + + // Reinstall the same spec - the tool should NOT be unlinked + // because the comparison should be by tool name ("SwiftLint"), not binary name ("swiftlint") + try await installer.install(installationType: .spec(specPath: URL(string: path)!)) + + // Verify the symlink still exists after reinstall + #expect(fileManager.fileExists(atPath: swiftlintSymLink.path)) + } + + @Test + func test_installSpec_unlinksToolsByName() async throws { + // This test verifies that orphan detection correctly identifies tools to unlink + // by comparing tool names (from folder structure) against spec tool names. + + let installer = Installer(fileManager: fileManager, ignoreArchitectureCheck: true, printer: PrinterMock()) + + // First, install a spec with SwiftLint + let swiftLintFixture = Fixture(filename: "Lucafile_LowVersion", type: "yml") + let bundle = Bundle.module + let swiftLintPath = try #require(bundle.path(forResource: swiftLintFixture.filename, ofType: swiftLintFixture.type)) + + try await installer.install(installationType: .spec(specPath: URL(string: swiftLintPath)!)) + + let swiftlintSymLink = fileManager.activeFolder.appending(component: "swiftlint") + #expect(fileManager.fileExists(atPath: swiftlintSymLink.path)) + + // Now install a different spec that does NOT include SwiftLint + let differentFixture = Fixture(filename: "Lucafile_valid_subset", type: "yml") + let differentPath = try #require(bundle.path(forResource: differentFixture.filename, ofType: differentFixture.type)) + + try await installer.install(installationType: .spec(specPath: URL(string: differentPath)!)) + + // SwiftLint should be unlinked because it's not in the new spec (by tool name) + #expect(!fileManager.fileExists(atPath: swiftlintSymLink.path)) + + // But tools in the new spec should still be linked + let sourcerySymLink = fileManager.activeFolder.appending(component: "sourcery") + let firebaseSymLink = fileManager.activeFolder.appending(component: "firebase") + #expect(fileManager.fileExists(atPath: sourcerySymLink.path)) + #expect(fileManager.fileExists(atPath: firebaseSymLink.path)) + } + private func spec(for fixture: Fixture) throws -> Spec { let bundle = Bundle.module let path = try #require(bundle.path(forResource: fixture.filename, ofType: fixture.type)) diff --git a/Tests/Fixtures/Lucafiles/Lucafile_valid_subset.yml b/Tests/Fixtures/Lucafiles/Lucafile_valid_subset.yml new file mode 100644 index 0000000..661355c --- /dev/null +++ b/Tests/Fixtures/Lucafiles/Lucafile_valid_subset.yml @@ -0,0 +1,12 @@ +--- +tools: + - name: FirebaseCLI + desiredBinaryName: firebase + version: 14.12.1 + url: https://github.com/firebase/firebase-tools/releases/download/v14.12.1/firebase-tools-macos + - name: Sourcery + binaryPath: bin/sourcery + version: 2.2.7 + url: https://github.com/krzysztofzablocki/Sourcery/releases/download/2.2.7/sourcery-2.2.7.zip + +version: 0.0.1