Skip to content

Commit 50e5396

Browse files
authored
Do not include all of the build dir for API digester to avoid conflicting definitions. (#8776)
Do not include all of the build dir for API digester to avoid conflicting definitions. ### Motivation: Issue #8081 : when a package or its dependencies define a tool plugin, then diagnose-api-breaking-changes report false API breakage. Investigation shown that diagnose-api-breaking-changes passes for analysis (with `-I`) all of the build dir, with all of the dependencies. When a tool plugin is in the dependency tree, then dependencies for the plugin itself are built with `.host` destination. If it happens that these modules are also built for `.target` destination, then `swift-api-digester` fails to analyze them, and public API declared there is printed as removed. ``` /Users/yy/work/pub/tst/.build/arm64-apple-macosx/debug/MyLib-tool.build/module.modulemap:1:8: error: redefinition of module 'MyLib' module MyLib { ^ /Users/yy/work/pub/tst/.build/arm64-apple-macosx/debug/MyLib.build/module.modulemap:1:8: note: previously defined here module MyLib { ^ Failed to load module: MyLib ``` ### Modifications: The goal for this PR is to reduce analyzed surface from "all of the build directory" down to modules built for target. ### Result: Before the change, example in Issue #8081 ``` 1 breaking change detected in mypackage: 💔 API breakage: var myPublicString has been removed ``` After the change: ``` No breaking changes detected in mypackage ```
1 parent 2ec37b7 commit 50e5396

File tree

6 files changed

+56
-3
lines changed

6 files changed

+56
-3
lines changed
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// swift-tools-version: 6.1
2+
import PackageDescription
3+
4+
let package = Package(
5+
name: "package-with-plugin",
6+
products: [.library(name: "PackageLib", targets: ["TargetLib"])],
7+
targets: [
8+
.target(name: "TargetLib"),
9+
.executableTarget(name: "BuildTool", dependencies: ["TargetLib"]),
10+
.plugin(
11+
name: "BuildPlugin",
12+
capability: .command(intent: .custom(verb: "do-it-now", description: "")),
13+
dependencies: ["BuildTool"]
14+
),
15+
]
16+
)
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import Foundation
2+
import PackagePlugin
3+
4+
@main
5+
final class BuildToolPlugin: CommandPlugin {
6+
func performCommand(context: PluginContext, arguments: [String]) async throws {
7+
_ = try context.tool(named: "BuildTool")
8+
}
9+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import TargetLib
2+
3+
@main
4+
public struct BuildTool {
5+
static func main() {
6+
TargetLibStruct.do_it()
7+
}
8+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
public enum TargetLibStruct {
2+
public static func do_it() {
3+
print("Hello!")
4+
}
5+
}

Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,8 +585,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
585585

586586
public func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String] {
587587
// API tool runs on products, hence using `self.productsBuildParameters`, not `self.toolsBuildParameters`
588-
let buildPath = self.destinationBuildParameters.buildPath.pathString
589-
var arguments = ["-I", buildPath]
588+
var arguments: [String] = []
590589

591590
// swift-symbolgraph-extract does not support parsing `-use-ld=lld` and
592591
// will silently error failing the operation. Filter out this flag
@@ -610,7 +609,12 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
610609
for target in self.targets {
611610
switch target {
612611
case .swift(let targetDescription):
613-
arguments += ["-I", targetDescription.moduleOutputPath.parentDirectory.pathString]
612+
if target.destination == .target {
613+
// Include in the analysis surface target destination. That way auxiliary
614+
// modules from building a build tool (destination == .host) won't conflict
615+
// with the modules intended to analyze.
616+
arguments += ["-I", targetDescription.moduleOutputPath.parentDirectory.pathString]
617+
}
614618
case .clang(let targetDescription):
615619
if let includeDir = targetDescription.moduleMap?.parentDirectory {
616620
arguments += ["-I", includeDir.pathString]

Tests/CommandsTests/APIDiffTests.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,17 @@ class APIDiffTestCase: CommandsBuildProviderTestCase {
328328
}
329329
}
330330

331+
332+
func testAPIDiffPackageWithPlugin() async throws {
333+
try skipIfApiDigesterUnsupportedOrUnset()
334+
try await fixture(name: "Miscellaneous/APIDiff/") { fixturePath in
335+
let packageRoot = fixturePath.appending("WithPlugin")
336+
let (output, _) = try await execute(["diagnose-api-breaking-changes", "1.2.3"], packagePath: packageRoot)
337+
XCTAssertMatch(output, .contains("No breaking changes detected in TargetLib"))
338+
}
339+
}
340+
341+
331342
func testBadTreeish() async throws {
332343
try skipIfApiDigesterUnsupportedOrUnset()
333344
try await fixture(name: "Miscellaneous/APIDiff/") { fixturePath in

0 commit comments

Comments
 (0)