Skip to content

Commit 6e99ca3

Browse files
pusukuripusukuri
authored
Build: avoid unnecessary warnings for build tool inputs (#7300)
Consider plugin build command input files into consideration and don't emit unnecessary warnings ### Motivation: Starting with 5.9 SwiftPM produces warnings for unhandled files but doesn’t take plugin build command input files into consideration. Consider plugin build command input files into consideration and don't emit unnecessary warnings. Resolves rdar://113256834
1 parent e10ff90 commit 6e99ca3

File tree

14 files changed

+290
-31
lines changed

14 files changed

+290
-31
lines changed

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Package.swift

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// swift-tools-version: 5.9
2+
import PackageDescription
3+
4+
let package = Package(
5+
name: "MySourceGenPluginNoPreBuildCommands",
6+
products: [
7+
// The product that vends MySourceGenBuildToolPlugin to client packages.
8+
.plugin(
9+
name: "MySourceGenBuildToolPlugin",
10+
targets: ["MySourceGenBuildToolPlugin"]
11+
),
12+
// The product that vends the MySourceGenBuildTool executable to client packages.
13+
.executable(
14+
name: "MySourceGenBuildTool",
15+
targets: ["MySourceGenBuildTool"]
16+
),
17+
],
18+
targets: [
19+
// A local tool that uses a build tool plugin.
20+
.executableTarget(
21+
name: "MyLocalTool",
22+
plugins: [
23+
"MySourceGenBuildToolPlugin",
24+
]
25+
),
26+
// A local tool that uses a prebuild plugin.
27+
.executableTarget(
28+
name: "MyOtherLocalTool",
29+
plugins: [
30+
"MySourceGenBuildToolPlugin",
31+
]
32+
),
33+
// The plugin that generates build tool commands to invoke MySourceGenBuildTool.
34+
.plugin(
35+
name: "MySourceGenBuildToolPlugin",
36+
capability: .buildTool(),
37+
dependencies: [
38+
"MySourceGenBuildTool",
39+
]
40+
),
41+
// The command line tool that generates source files.
42+
.executableTarget(
43+
name: "MySourceGenBuildTool",
44+
dependencies: [
45+
"MySourceGenBuildToolLib",
46+
]
47+
),
48+
// A library used by MySourceGenBuildTool (not the client).
49+
.target(
50+
name: "MySourceGenBuildToolLib"
51+
),
52+
// A runtime library that the client needs to link against.
53+
.target(
54+
name: "MySourceGenRuntimeLib"
55+
),
56+
// Unit tests for the plugin.
57+
.testTarget(
58+
name: "MySourceGenPluginTests",
59+
dependencies: [
60+
"MySourceGenRuntimeLib",
61+
],
62+
plugins: [
63+
"MySourceGenBuildToolPlugin",
64+
]
65+
)
66+
]
67+
)

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Plugins/MySourceGenBuildToolPlugin/plugin.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import PackagePlugin
2+
3+
@main
4+
struct MyPlugin: BuildToolPlugin {
5+
#if USE_CREATE
6+
let verb = "Creating"
7+
#else
8+
let verb = "Generating"
9+
#endif
10+
11+
func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
12+
print("Hello from the Build Tool Plugin!")
13+
guard let target = target as? SourceModuleTarget else { return [] }
14+
return try target.sourceFiles.map{ $0.path }.compactMap {
15+
guard $0.extension == "dat" else { return .none }
16+
let outputName = $0.stem + ".swift"
17+
let outputPath = context.pluginWorkDirectory.appending(outputName)
18+
return .buildCommand(
19+
displayName:
20+
"\(verb) \(outputName) from \($0.lastComponent)",
21+
executable:
22+
try context.tool(named: "MySourceGenBuildTool").path,
23+
arguments: [
24+
"\($0)",
25+
"\(outputPath)"
26+
],
27+
inputFiles: [
28+
$0,
29+
],
30+
outputFiles: [
31+
outputPath
32+
]
33+
)
34+
}
35+
}
36+
}

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/foo.dat

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
I am Foo!

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyLocalTool/main.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
print("Generated string Foo: '\(foo)'")

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/bar.dat

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
I am Bar!

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/baz.dat

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
I am Baz!

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MyOtherLocalTool/main.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// print("Generated string Bar: '\(bar)'")
2+
// print("Generated string Baz: '\(baz)'")

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildTool/main.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import Foundation
2+
import MySourceGenBuildToolLib
3+
4+
// 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.
5+
if ProcessInfo.processInfo.arguments.count != 3 {
6+
print("usage: MySourceGenBuildTool <input> <output>")
7+
exit(1)
8+
}
9+
let inputFile = ProcessInfo.processInfo.arguments[1]
10+
let outputFile = ProcessInfo.processInfo.arguments[2]
11+
12+
let variableName = URL(fileURLWithPath: inputFile).deletingPathExtension().lastPathComponent
13+
14+
let inputData = FileManager.default.contents(atPath: inputFile) ?? Data()
15+
let dataAsHex = inputData.map { String(format: "%02hhx", $0) }.joined()
16+
let outputString = "public var \(variableName) = \(dataAsHex.quotedForSourceCode)\n"
17+
let outputData = outputString.data(using: .utf8)
18+
FileManager.default.createFile(atPath: outputFile, contents: outputData)

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenBuildToolLib/library.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import Foundation
2+
3+
extension String {
4+
5+
public var quotedForSourceCode: String {
6+
return "\"" + self
7+
.replacingOccurrences(of: "\\", with: "\\\\")
8+
.replacingOccurrences(of: "\"", with: "\\\"")
9+
+ "\""
10+
}
11+
}

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Sources/MySourceGenRuntimeLib/library.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
public func GetLibraryName() -> String {
2+
return "MySourceGenRuntimeLib"
3+
}

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/MySourceGenPluginNoPreBuildCommandsTests.swift

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import XCTest
2+
import class Foundation.Bundle
3+
4+
// for rdar://113256834
5+
final class SwiftyProtobufTests: XCTestCase {
6+
func testMyLocalTool() throws {
7+
// This is an example of a functional test case.
8+
// Use XCTAssert and related functions to verify your tests produce the correct
9+
// results.
10+
11+
// Some of the APIs that we use below are available in macOS 10.13 and above.
12+
guard #available(macOS 10.13, *) else {
13+
return
14+
}
15+
16+
// Mac Catalyst won't have `Process`, but it is supported for executables.
17+
#if !targetEnvironment(macCatalyst)
18+
19+
let fooBinary = productsDirectory.appendingPathComponent("MyLocalTool")
20+
21+
let process = Process()
22+
process.executableURL = fooBinary
23+
24+
let pipe = Pipe()
25+
process.standardOutput = pipe
26+
27+
try process.run()
28+
process.waitUntilExit()
29+
30+
let data = pipe.fileHandleForReading.readDataToEndOfFile()
31+
let output = String(data: data, encoding: .utf8)
32+
XCTAssertEqual(output, "Generated string Foo: \'4920616d20466f6f210a\'\n")
33+
#endif
34+
}
35+
36+
func testMyOtherLocalTool() throws {
37+
// This is an example of a functional test case.
38+
// Use XCTAssert and related functions to verify your tests produce the correct
39+
// results.
40+
41+
// Some of the APIs that we use below are available in macOS 10.13 and above.
42+
guard #available(macOS 10.13, *) else {
43+
return
44+
}
45+
46+
// Mac Catalyst won't have `Process`, but it is supported for executables.
47+
#if !targetEnvironment(macCatalyst)
48+
49+
let fooBinary = productsDirectory.appendingPathComponent("MyOtherLocalTool")
50+
51+
let process = Process()
52+
process.executableURL = fooBinary
53+
54+
let pipe = Pipe()
55+
process.standardOutput = pipe
56+
57+
try process.run()
58+
process.waitUntilExit()
59+
60+
let data = pipe.fileHandleForReading.readDataToEndOfFile()
61+
let output = String(data: data, encoding: .utf8)
62+
XCTAssertEqual(output, "")
63+
#endif
64+
}
65+
66+
/// Returns path to the built products directory.
67+
var productsDirectory: URL {
68+
#if os(macOS)
69+
for bundle in Bundle.allBundles where bundle.bundlePath.hasSuffix(".xctest") {
70+
return bundle.bundleURL.deletingLastPathComponent()
71+
}
72+
fatalError("couldn't find the products directory")
73+
#else
74+
return Bundle.main.bundleURL
75+
#endif
76+
}
77+
}

‎Fixtures/Miscellaneous/Plugins/MySourceGenPluginNoPreBuildCommands/Tests/MySourceGenPluginNoPreBuildCommandsTests/lunch.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
coffee

‎Sources/Build/BuildOperation.swift

Lines changed: 62 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
147147
///
148148
/// This will try skip build planning if build manifest caching is enabled
149149
/// and the package structure hasn't changed.
150-
public func getBuildDescription() throws -> BuildDescription {
150+
public func getBuildDescription(subset: BuildSubset? = nil) throws -> BuildDescription {
151151
return try self.buildDescription.memoize {
152152
if self.cacheBuildManifest {
153153
do {
@@ -171,7 +171,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
171171
}
172172
}
173173
// We need to perform actual planning if we reach here.
174-
return try self.plan().description
174+
return try self.plan(subset: subset).description
175175
}
176176
}
177177

@@ -261,7 +261,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
261261
// Get the build description (either a cached one or newly created).
262262

263263
// Get the build description
264-
let buildDescription = try getBuildDescription()
264+
let buildDescription = try getBuildDescription(subset: subset)
265265

266266
// Verify dependency imports on the described targets
267267
try verifyTargetImports(in: buildDescription)
@@ -447,10 +447,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
447447
}
448448

449449
/// Create the build plan and return the build description.
450-
private func plan() throws -> (description: BuildDescription, manifest: LLBuildManifest) {
450+
private func plan(subset: BuildSubset? = nil) throws -> (description: BuildDescription, manifest: LLBuildManifest) {
451451
// Load the package graph.
452452
let graph = try getPackageGraph()
453-
454453
let buildToolPluginInvocationResults: [ResolvedTarget.ID: (target: ResolvedTarget, results: [BuildToolPluginInvocationResult])]
455454
let prebuildCommandResults: [ResolvedTarget.ID: [PrebuildCommandResult]]
456455
// Invoke any build tool plugins in the graph to generate prebuild commands and build commands.
@@ -530,33 +529,44 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
530529
}
531530

532531
// Emit warnings about any unhandled files in authored packages. We do this after applying build tool plugins, once we know what files they handled.
533-
for package in graph.rootPackages where package.manifest.toolsVersion >= .v5_3 {
534-
for target in package.targets {
535-
// Get the set of unhandled files in targets.
536-
var unhandledFiles = Set(target.underlying.others)
537-
if unhandledFiles.isEmpty { continue }
538-
539-
// Subtract out any that were inputs to any commands generated by plugins.
540-
if let result = buildToolPluginInvocationResults[target.id]?.results {
541-
let handledFiles = result.flatMap { $0.buildCommands.flatMap { $0.inputFiles } }
542-
unhandledFiles.subtract(handledFiles)
543-
}
544-
if unhandledFiles.isEmpty { continue }
545-
546-
// Emit a diagnostic if any remain. This is kept the same as the previous message for now, but this could be improved.
547-
let diagnosticsEmitter = self.observabilityScope.makeDiagnosticsEmitter {
548-
var metadata = ObservabilityMetadata()
549-
metadata.packageIdentity = package.identity
550-
metadata.packageKind = package.manifest.packageKind
551-
metadata.targetName = target.name
552-
return metadata
553-
}
554-
var warning = "found \(unhandledFiles.count) file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n"
555-
for file in unhandledFiles {
556-
warning += " " + file.pathString + "\n"
557-
}
558-
diagnosticsEmitter.emit(warning: warning)
532+
// rdar://113256834 This fix works for the plugins that do not have PreBuildCommands.
533+
let targetsToConsider: [ResolvedTarget]
534+
if let subset = subset, let recursiveDependencies = try
535+
subset.recursiveDependencies(for: graph, observabilityScope: observabilityScope) {
536+
targetsToConsider = recursiveDependencies
537+
} else {
538+
targetsToConsider = Array(graph.reachableTargets)
539+
}
540+
541+
for target in targetsToConsider {
542+
guard let package = graph.package(for: target), package.manifest.toolsVersion >= .v5_3 else {
543+
continue
559544
}
545+
546+
// Get the set of unhandled files in targets.
547+
var unhandledFiles = Set(target.underlying.others)
548+
if unhandledFiles.isEmpty { continue }
549+
550+
// Subtract out any that were inputs to any commands generated by plugins.
551+
if let result = buildToolPluginInvocationResults[target.id]?.results {
552+
let handledFiles = result.flatMap{ $0.buildCommands.flatMap{ $0.inputFiles } }
553+
unhandledFiles.subtract(handledFiles)
554+
}
555+
if unhandledFiles.isEmpty { continue }
556+
557+
// Emit a diagnostic if any remain. This is kept the same as the previous message for now, but this could be improved.
558+
let diagnosticsEmitter = self.observabilityScope.makeDiagnosticsEmitter {
559+
var metadata = ObservabilityMetadata()
560+
metadata.packageIdentity = package.identity
561+
metadata.packageKind = package.manifest.packageKind
562+
metadata.targetName = target.name
563+
return metadata
564+
}
565+
var warning = "found \(unhandledFiles.count) file(s) which are unhandled; explicitly declare them as resources or exclude from the target\n"
566+
for file in unhandledFiles {
567+
warning += " " + file.pathString + "\n"
568+
}
569+
diagnosticsEmitter.emit(warning: warning)
560570
}
561571

562572
// Create the build plan based, on the graph and any information from plugins.
@@ -782,6 +792,27 @@ extension BuildDescription {
782792
}
783793

784794
extension BuildSubset {
795+
func recursiveDependencies(for graph: PackageGraph, observabilityScope: ObservabilityScope) throws -> [ResolvedTarget]? {
796+
switch self {
797+
case .allIncludingTests:
798+
return Array(graph.reachableTargets)
799+
case .allExcludingTests:
800+
return graph.reachableTargets.filter { $0.type != .test }
801+
case .product(let productName):
802+
guard let product = graph.allProducts.first(where: { $0.name == productName }) else {
803+
observabilityScope.emit(error: "no product named '\(productName)'")
804+
return nil
805+
}
806+
return try product.recursiveTargetDependencies()
807+
case .target(let targetName):
808+
guard let target = graph.allTargets.first(where: { $0.name == targetName }) else {
809+
observabilityScope.emit(error: "no target named '\(targetName)'")
810+
return nil
811+
}
812+
return try target.recursiveTargetDependencies()
813+
}
814+
}
815+
785816
/// Returns the name of the llbuild target that corresponds to the build subset.
786817
func llbuildTargetName(for graph: PackageGraph, config: String, observabilityScope: ObservabilityScope)
787818
-> String?

‎Tests/FunctionalTests/PluginTests.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ class PluginTests: XCTestCase {
3434
}
3535
}
3636

37+
func testUseOfBuildToolPluginTargetNoPreBuildCommands() throws {
38+
// Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).
39+
try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")
40+
try fixture(name: "Miscellaneous/Plugins") { fixturePath in
41+
let (_, stderr) = try executeSwiftTest(fixturePath.appending("MySourceGenPluginNoPreBuildCommands"))
42+
XCTAssertTrue(stderr.contains("file(s) which are unhandled; explicitly declare them as resources or exclude from the target"), "expected warning not emitted")
43+
}
44+
}
45+
3746
func testUseOfBuildToolPluginProductByExecutableAcrossPackages() throws {
3847
// Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).
3948
try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")

0 commit comments

Comments
 (0)