Consolidate plugin accessible tools

Until now, we had two almost identical code paths for processing accessible tools and e.g. the command plugin code didn't get the fixes from #5945. This PR consolidates the two and also fixes a bug regarding executable path handling of vended tools.

Author: neonichu

Sources/Commands/PackageTools/PluginCommand.swift

@@ -160,22 +160,14 @@ struct PluginCommand: SwiftCommand {
             + getEnvSearchPaths(pathString: ProcessEnv.path, currentWorkingDirectory: .none)
 
         // Build or bring up-to-date any executable host-side tools on which this plugin depends. Add them and any binary dependencies to the tool-names-to-path map.
-        var toolNamesToPaths: [String: AbsolutePath] = [:]
-        // Add supported triples info per tool so they can be looked up when running the tool
-        var toolNamesToTriples: [String: [String]] = [:]
-        for dep in try plugin.accessibleTools(packageGraph: packageGraph, fileSystem: swiftTool.fileSystem, environment: try swiftTool.buildParameters().buildEnvironment, for: try pluginScriptRunner.hostTriple) {
+        let (toolNamesToPaths, toolNamesToTriples) = try plugin.processAccessibleTools(packageGraph: packageGraph, fileSystem: swiftTool.fileSystem, environment: try swiftTool.buildParameters().buildEnvironment, for: try pluginScriptRunner.hostTriple) { name, path in
             let buildSystem = try swiftTool.createBuildSystem(explicitBuildSystem: .native, cacheBuildManifest: false)
-            switch dep {
-            case .builtTool(let name, _):
-                // Build the product referenced by the tool, and add the executable to the tool map. Product dependencies are not supported within a package, so if the tool happens to be from the same package, we instead find the executable that corresponds to the product. There is always one, because of autogeneration of implicit executables with the same name as the target if there isn't an explicit one.
-                try buildSystem.build(subset: .product(name))
-                if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: { $0.product.name == name}) {
-                    toolNamesToPaths[name] = builtTool.binaryPath
-                }
-            case .vendedTool(let name, let path, let triples):
-                toolNamesToPaths[name] = path
-                // Need triples info for .vendedTool
-                toolNamesToTriples[name] = triples
+            // Build the product referenced by the tool, and add the executable to the tool map. Product dependencies are not supported within a package, so if the tool happens to be from the same package, we instead find the executable that corresponds to the product. There is always one, because of autogeneration of implicit executables with the same name as the target if there isn't an explicit one.
+            try buildSystem.build(subset: .product(name))
+            if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: { $0.product.name == name}) {
+                return builtTool.binaryPath
+            } else {
+                return nil
             }
         }
 

Sources/SPMBuildCore/PluginInvocation.swift

@@ -364,33 +364,15 @@ extension PackageGraph {
             for pluginTarget in pluginTargets {
                 // Determine the tools to which this plugin has access, and create a name-to-path mapping from tool
                 // names to the corresponding paths. Built tools are assumed to be in the build tools directory.
-                let accessibleTools = try pluginTarget.accessibleTools(packageGraph: self, fileSystem: fileSystem, environment: buildEnvironment, for: try pluginScriptRunner.hostTriple)
-                let toolNamesToPaths = accessibleTools.reduce(into: [String: AbsolutePath](), { dict, tool in
-                    switch tool {
-                    case .builtTool(let name, let path):
-                        dict[name] = builtToolsDir.appending(path)
-                    case .vendedTool(let name, let path, _):
-                        dict[name] = path
-                    }
-                })
-                let builtToolNames = accessibleTools.compactMap { (accTool) -> String? in
-                    if case .builtTool(let name, _) = accTool {
-                        return name
-                    }
-                    return nil
+                var builtToolNames: [String] = []
+                let (toolNamesToPaths, toolNamesToTriples) = try pluginTarget.processAccessibleTools(packageGraph: self, fileSystem: fileSystem, environment: buildEnvironment, for: try pluginScriptRunner.hostTriple) { name, path in
+                    builtToolNames.append(name)
+                    return builtToolsDir.appending(path)
                 }
 
                 // Determine additional input dependencies for any plugin commands, based on any executables the plugin target depends on.
                 let toolPaths = toolNamesToPaths.values.sorted()
 
-                let toolNamesToTriples = accessibleTools.reduce(into: [String: [String]](), { dict, tool in
-                    switch tool {
-                    case .vendedTool(let name, _, let triples):
-                        dict[name, default: []].append(contentsOf: triples)
-                    default: break
-                    }
-                })
-
                 // Assign a plugin working directory based on the package, target, and plugin.
                 let pluginOutputDir = outputDir.appending(components: package.identity.description, target.name, pluginTarget.name)
 
@@ -529,7 +511,7 @@ public extension PluginTarget {
     }
 
     /// The set of tools that are accessible to this plugin.
-    func accessibleTools(packageGraph: PackageGraph, fileSystem: FileSystem, environment: BuildEnvironment, for hostTriple: Triple) throws -> Set<PluginAccessibleTool> {
+    private func accessibleTools(packageGraph: PackageGraph, fileSystem: FileSystem, environment: BuildEnvironment, for hostTriple: Triple) throws -> Set<PluginAccessibleTool> {
         return try Set(self.dependencies(satisfying: environment).flatMap { dependency -> [PluginAccessibleTool] in
             let builtToolName: String
             let executableOrBinaryTarget: Target
@@ -560,6 +542,31 @@ public extension PluginTarget {
             }
         })
     }
+
+    func processAccessibleTools(packageGraph: PackageGraph, fileSystem: FileSystem, environment: BuildEnvironment, for hostTriple: Triple, builtToolHandler: (_ name: String, _ path: RelativePath) throws -> AbsolutePath?) throws -> (toolNamesToPaths: [String: AbsolutePath], toolNamesToTriples: [String: [String]]) {
+        var toolNamesToPaths: [String: AbsolutePath] = [:]
+        // Add supported triples info per tool so they can be looked up when running the tool
+        var toolNamesToTriples: [String: [String]] = [:]
+
+        for dep in try accessibleTools(packageGraph: packageGraph, fileSystem: fileSystem, environment: environment, for: hostTriple) {
+            switch dep {
+            case .builtTool(let name, let path):
+                if let path = try builtToolHandler(name, path) {
+                    toolNamesToPaths[name] = path
+                }
+            case .vendedTool(let name, let path, let triples):
+                // Avoid having the path of an unsupported tool overwrite a supported one.
+                guard !triples.isEmpty || toolNamesToPaths[name] == nil else {
+                    continue
+                }
+                toolNamesToPaths[name] = path
+                // Need triples info for .vendedTool
+                toolNamesToTriples[name, default: []].append(contentsOf: triples)
+            }
+        }
+
+        return (toolNamesToPaths, toolNamesToTriples)
+    }
 }
 
 fileprivate extension Target.Dependency {

Tests/SPMBuildCoreTests/PluginInvocationTests.swift

@@ -1113,14 +1113,14 @@ class PluginInvocationTests: XCTestCase {
                         print("Looking for LocalBinaryTool...")
                         let localBinaryTool = try context.tool(named: "LocalBinaryTool")
                         print("... found it at \\(localBinaryTool.path)")
-                        return []
+                        return [.buildCommand(displayName: "", executable: localBinaryTool.path, arguments: [], inputFiles: [], outputFiles: [])]
                     }
                  }
             """
             try localFileSystem.writeFileContents(myPluginTargetDir.appending(component: "plugin.swift"), string: content)
             let artifactVariants = artifactSupportedTriples.map {
                 """
-                { "path": "LocalBinaryTool.sh", "supportedTriples": ["\($0.tripleString)"] }
+                { "path": "LocalBinaryTool\($0.tripleString).sh", "supportedTriples": ["\($0.tripleString)"] }
                 """
             }
 
@@ -1238,6 +1238,7 @@ class PluginInvocationTests: XCTestCase {
                 $0.value.forEach {
                     XCTAssertTrue($0.succeeded, "plugin unexpectedly failed")
                     XCTAssertEqual($0.diagnostics.map { $0.message }, [], "plugin produced unexpected diagnostics")
+                    XCTAssertEqual($0.buildCommands.first?.configuration.executable.basename, "LocalBinaryTool\(hostTriple.tripleString).sh")
                 }
             }
         }