Fix a bug where an executable target used by a plugin might not get linked before the plugin needs it (#3858)

* Implemented fix by synthesizing a product for any executable that a plug‐in target depends on (funnelling it into the same code that does the synthesis for executable targets in the root package).  Without this, the plugin target would depend only on the executable target (compiling the code) but not on the product (linking it into an executable).
* Added tests.

Author: SDGGiesbrecht

Fixtures/Miscellaneous/Plugins/ClientOfPluginWithInternalExecutable/Package.swift

@@ -0,0 +1,17 @@
+// swift-tools-version: 5.5
+import PackageDescription
+
+let package = Package(
+    name: "ClientOfPluginWithInternalExecutable",
+    dependencies: [
+        .package(path: "../PluginWithInternalExecutable")
+    ],
+    targets: [
+        .executableTarget(
+            name: "RootTarget",
+            plugins: [
+                .plugin(name: "PluginScriptProduct", package: "PluginWithInternalExecutable")
+            ]
+        ),
+    ]
+)

Fixtures/Miscellaneous/Plugins/ClientOfPluginWithInternalExecutable/Sources/RootTarget/foo.dat

@@ -0,0 +1 @@
+I am Foo!

Fixtures/Miscellaneous/Plugins/ClientOfPluginWithInternalExecutable/Sources/RootTarget/main.swift

@@ -0,0 +1 @@
+print("Generated string Foo: '\(foo)'")

Fixtures/Miscellaneous/Plugins/InvalidUseOfInternalPluginExecutable/Package.swift

@@ -0,0 +1,20 @@
+// swift-tools-version: 5.5
+import PackageDescription
+
+let package = Package(
+    name: "InvalidUseOfInternalPluginExecutable",
+    dependencies: [
+        .package(path: "../PluginWithInternalExecutable")
+    ],
+    targets: [
+        .executableTarget(
+            name: "RootTarget",
+            dependencies: [
+                .product(name: "PluginExecutable", package: "PluginWithInternalExecutable")
+            ],
+            plugins: [
+                .plugin(name: "PluginScriptProduct", package: "PluginWithInternalExecutable")
+            ]
+        ),
+    ]
+)

Fixtures/Miscellaneous/Plugins/InvalidUseOfInternalPluginExecutable/Sources/RootTarget/foo.dat

@@ -0,0 +1 @@
+I am Foo!

Fixtures/Miscellaneous/Plugins/InvalidUseOfInternalPluginExecutable/Sources/RootTarget/main.swift

@@ -0,0 +1 @@
+print("Generated string Foo: '\(foo)'")

Fixtures/Miscellaneous/Plugins/PluginWithInternalExecutable/Package.swift

@@ -0,0 +1,24 @@
+// swift-tools-version: 5.5
+import PackageDescription
+
+let package = Package(
+    name: "PluginWithInternalExecutable",
+    products: [
+        .plugin(
+            name: "PluginScriptProduct",
+            targets: ["PluginScriptTarget"]
+        ),
+    ],
+    targets: [
+        .plugin(
+            name: "PluginScriptTarget",
+            capability: .buildTool(),
+            dependencies: [
+                "PluginExecutable",
+            ]
+        ),
+        .executableTarget(
+            name: "PluginExecutable"
+        ),
+    ]
+)

Fixtures/Miscellaneous/Plugins/PluginWithInternalExecutable/Plugins/PluginScriptTarget/Script.swift

@@ -0,0 +1,31 @@
+import PackagePlugin
+
+@main
+struct PluginScript: BuildToolPlugin {
+
+    func createBuildCommands(context: TargetBuildContext) throws -> [Command] {
+        print("Hello from the plugin script!")
+
+        return try context.inputFiles.map{ $0.path }.compactMap {
+            guard $0.extension == "dat" else { return .none }
+            let outputName = $0.stem + ".swift"
+            let outputPath = context.pluginWorkDirectory.appending(outputName)
+            return .buildCommand(
+                displayName:
+                    "Generating \(outputName) from \($0.lastComponent)",
+                executable:
+                    try context.tool(named: "PluginExecutable").path,
+                arguments: [
+                    "\($0)",
+                    "\(outputPath)"
+                ],
+                inputFiles: [
+                    $0,
+                ],
+                outputFiles: [
+                    outputPath
+                ]
+            )
+        }
+    }
+}

Fixtures/Miscellaneous/Plugins/PluginWithInternalExecutable/Sources/PluginExecutable/Utilities.swift

@@ -0,0 +1,11 @@
+import Foundation
+
+extension String {
+
+    var quotedForSourceCode: String {
+        return "\"" + self
+            .replacingOccurrences(of: "\\", with: "\\\\")
+            .replacingOccurrences(of: "\"", with: "\\\"")
+            + "\""
+    }
+}

Fixtures/Miscellaneous/Plugins/PluginWithInternalExecutable/Sources/PluginExecutable/main.swift

@@ -0,0 +1,17 @@
+import Foundation
+
+// Sample source generator tool that emits a Swift variable declaration of a string containing the hex representation of the contents of a file as a quoted string.  The variable name is the base name of the input file.  The input file is the first argument and the output file is the second.
+if ProcessInfo.processInfo.arguments.count != 3 {
+    print("usage: MySourceGenBuildTool <input> <output>")
+    exit(1)
+}
+let inputFile = ProcessInfo.processInfo.arguments[1]
+let outputFile = ProcessInfo.processInfo.arguments[2]
+
+let variableName = URL(fileURLWithPath: inputFile).deletingPathExtension().lastPathComponent
+
+let inputData = FileManager.default.contents(atPath: inputFile) ?? Data()
+let dataAsHex = inputData.map { String(format: "%02hhx", $0) }.joined()
+let outputString = "public var \(variableName) = \(dataAsHex.quotedForSourceCode)\n"
+let outputData = outputString.data(using: .utf8)
+FileManager.default.createFile(atPath: outputFile, contents: outputData)

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -397,8 +397,11 @@ private func createResolvedPackages(
 
         // Get all the products from dependencies of this package.
         let productDependencies = packageBuilder.dependencies
-            .flatMap({ $0.products })
-            .filter({ $0.product.type != .test })
+            .flatMap({ (dependency: ResolvedPackageBuilder) -> [ResolvedProductBuilder] in
+                // Filter out synthesized products such as tests and implicit executables.
+                let explicit = Set(dependency.package.manifest.products.lazy.map({ $0.name }))
+                return dependency.products.filter({ explicit.contains($0.product.name) })
+            })
         let productDependencyMap = productDependencies.spm_createDictionary({ ($0.product.name, $0) })
 
         // Establish dependencies in each target.

Sources/PackageLoading/PackageBuilder.swift

@@ -1255,37 +1255,46 @@ public final class PackageBuilder {
             append(Product(name: product.name, type: product.type, targets: targets))
         }
 
-        // Add implicit executables - for root packages only.
+        // Add implicit executables - for root packages and for dependency plugins.
 
-        if self.manifest.packageKind.isRoot {
-            // Compute the list of targets which are being used in an
-            // executable product so we don't create implicit executables
-            // for them.
-            let explicitProductsTargets = Set(self.manifest.products.flatMap{ product -> [String] in
-                switch product.type {
-                case .library, .plugin, .test:
-                    return []
-                case .executable, .snippet:
-                    return product.targets
-                }
-            })
-
-            let productMap = products.reduce(into: [String: Product]()) { partial, iterator in
-                partial[iterator.key] = iterator.item
+        // Compute the list of targets which are being used in an
+        // executable product so we don't create implicit executables
+        // for them.
+        let explicitProductsTargets = Set(self.manifest.products.flatMap{ product -> [String] in
+            switch product.type {
+            case .library, .plugin, .test:
+                return []
+            case .executable, .snippet:
+                return product.targets
             }
+        })
 
-            for target in targets where target.type == .executable {
-                if explicitProductsTargets.contains(target.name) {
-                    // If there is already an executable target with this name, skip generating a product for it
-                    continue
-                } else if let product = productMap[target.name] {
-                    // If there is already a product with this name skip generating a product for it,
-                    // but warn if that product is not executable
-                    if product.type != .executable {
-                        self.observabilityScope.emit(warning: "The target named '\(target.name)' was identified as an executable target but a non-executable product with this name already exists.")
-                    }
-                    continue
-                } else {
+        let productMap = products.reduce(into: [String: Product]()) { partial, iterator in
+            partial[iterator.key] = iterator.item
+        }
+
+        let implicitPlugInExecutables = Set(
+            targets.lazy
+                .filter({ $0.type == .plugin })
+                .flatMap({ $0.dependencies })
+                .map({ $0.name })
+        )
+
+        for target in targets where target.type == .executable {
+            if self.manifest.packageKind.isRoot && explicitProductsTargets.contains(target.name) {
+                // If there is already an executable target with this name, skip generating a product for it
+                // (This shortcut only works for the root manifest, because for dependencies,
+                // products that correspond to plug‐ins may have been culled during resolution.)
+                continue
+            } else if let product = productMap[target.name] {
+                // If there is already a product with this name skip generating a product for it,
+                // but warn if that product is not executable
+                if product.type != .executable {
+                    self.observabilityScope.emit(warning: "The target named '\(target.name)' was identified as an executable target but a non-executable product with this name already exists.")
+                }
+                continue
+            } else {
+                if self.manifest.packageKind.isRoot || implicitPlugInExecutables.contains(target.name) {
                     // Generate an implicit product for the executable target
                     let product = Product(name: target.name, type: .executable, targets: [target])
                     append(product)

Sources/PackageModel/Manifest.swift

@@ -256,6 +256,9 @@ public final class Manifest {
             for targetDependency in target.dependencies {
                 register(targetDependency: targetDependency, registry: &registry, availablePackages: availablePackages)
             }
+            for requiredPlugIn in target.pluginUsages ?? [] {
+                register(requiredPlugIn: requiredPlugIn, registry: &registry, availablePackages: availablePackages)
+            }
         }
 
         // Products whose package could not be determined are marked as needed on every dependency.
@@ -362,6 +365,35 @@ public final class Manifest {
         }
     }
 
+    /// Registers a required plug‐in with a particular dependency if possible, or registers it as unknown.
+    ///
+    /// - Parameters:
+    ///   - requiredPlugIn: The plug‐in to register.
+    ///   - registry: The registry in which to record the assocation.
+    ///   - availablePackages: The set of available packages.
+    private func register(
+        requiredPlugIn: TargetDescription.PluginUsage,
+        registry: inout (known: [PackageIdentity: ProductFilter], unknown: Set<String>),
+        availablePackages: Set<PackageIdentity>
+    ) {
+        switch requiredPlugIn {
+        case .plugin(let name, let package):
+            if let package = package {
+                if !register(
+                    product: name,
+                    inPackage: .plain(package),
+                    registry: &registry.known,
+                    availablePackages: availablePackages) {
+                    // Invalid, diagnosed later; see the dependency version of this method.
+                    registry.unknown.insert(name)
+                }
+            } else {
+                // The plug‐in is in the same package.
+                break
+            }
+        }
+    }
+
     /// Registers a required product with a particular dependency if possible.
     ///
     /// - Parameters:

Tests/FunctionalTests/PluginTests.swift

@@ -77,6 +77,45 @@ class PluginTests: XCTestCase {
         }
     }
 
+    func testUseOfPluginWithInternalExecutable() {
+        // Check if the host compiler supports the '-entry-point-function-name' flag.  It's not needed for this test but is needed to build any executable from a package that uses tools version 5.5.
+        #if swift(<5.5)
+        try XCTSkipIf(true, "skipping because host compiler doesn't support '-entry-point-function-name'")
+        #endif
+
+        fixture(name: "Miscellaneous/Plugins") { path in
+            let (stdout, _) = try executeSwiftBuild(path.appending(component: "ClientOfPluginWithInternalExecutable"))
+            XCTAssert(stdout.contains("Compiling PluginExecutable main.swift"), "stdout:\n\(stdout)")
+            XCTAssert(stdout.contains("Linking PluginExecutable"), "stdout:\n\(stdout)")
+            XCTAssert(stdout.contains("Generating foo.swift from foo.dat"), "stdout:\n\(stdout)")
+            XCTAssert(stdout.contains("Compiling RootTarget foo.swift"), "stdout:\n\(stdout)")
+            XCTAssert(stdout.contains("Linking RootTarget"), "stdout:\n\(stdout)")
+            XCTAssert(stdout.contains("Build complete!"), "stdout:\n\(stdout)")
+        }
+    }
+
+    func testInternalExecutableAvailableOnlyToPlugin() {
+        // Check if the host compiler supports the '-entry-point-function-name' flag.  It's not needed for this test but is needed to build any executable from a package that uses tools version 5.5.
+        #if swift(<5.5)
+        try XCTSkipIf(true, "skipping because host compiler doesn't support '-entry-point-function-name'")
+        #endif
+
+        fixture(name: "Miscellaneous/Plugins") { path in
+            do {
+                let (stdout, _) = try executeSwiftBuild(path.appending(component: "InvalidUseOfInternalPluginExecutable"))
+                XCTFail("Illegally used internal executable.\nstdout:\n\(stdout)")
+            }
+            catch SwiftPMProductError.executionFailure(_, _, let stderr) {
+                XCTAssert(
+                    stderr.contains(
+                        "product 'PluginExecutable' required by package 'invaliduseofinternalpluginexecutable' target 'RootTarget' not found in package 'PluginWithInternalExecutable'."
+                    ),
+                    "stderr:\n\(stderr)"
+                )
+            }
+        }
+    }
+
     func testContrivedTestCases() throws {
         // Check if the host compiler supports the '-entry-point-function-name' flag.  It's not needed for this test but is needed to build any executable from a package that uses tools version 5.5.
         #if swift(<5.5)

Tests/PackageModelTests/ManifestTests.swift

@@ -59,6 +59,7 @@ class ManifestTests: XCTestCase {
                 "Bar",
                 "Baz",
                 "Foo",
+                "MyPlugin",
             ])
             #endif
         }