Skip to content

Commit b9faf24

Browse files
committed
Verify that only targets that are manifest dependencies can be imported.
This change introduces a build plan verification step that attempts to detect scenarios where a target contains an `import` of another target in the package without declaring the imported target as a dependency in the manifest. This is done via SwiftDriver's import-prescan capability which relies on libSwiftScan to quickly parse a target's sources and identify all `import`ed modules.
1 parent 73c47c0 commit b9faf24

File tree

2 files changed

+129
-2
lines changed

2 files changed

+129
-2
lines changed

Sources/Build/ManifestBuilder.swift

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ extension LLBuildManifestBuilder {
208208
} else if buildParameters.emitSwiftModuleSeparately {
209209
try self.addSwiftCmdsEmitSwiftModuleSeparately(target, inputs: inputs, objectNodes: objectNodes, moduleNode: moduleNode)
210210
} else {
211-
self.addCmdWithBuiltinSwiftTool(target, inputs: inputs, cmdOutputs: cmdOutputs)
211+
try self.addCmdWithBuiltinSwiftTool(target, inputs: inputs, cmdOutputs: cmdOutputs)
212212
}
213213

214214
self.addTargetCmd(target, cmdOutputs: cmdOutputs)
@@ -474,14 +474,63 @@ extension LLBuildManifestBuilder {
474474
)
475475
}
476476

477+
// Emit an error if this target imports another target in this build
478+
// without specifying it as a dependency
479+
private func verifyTargetImports(_ target: SwiftTargetBuildDescription) throws {
480+
// ArtemC: prototype for target import verification
481+
var verificationFailure = false
482+
do {
483+
484+
var commandLine = try target.emitCommandLine();
485+
commandLine.append("-driver-use-frontend-path")
486+
commandLine.append(buildParameters.toolchain.swiftCompiler.pathString)
487+
let resolver = try ArgsResolver(fileSystem: target.fs)
488+
let executor = SPMSwiftDriverExecutor(resolver: resolver,
489+
fileSystem: target.fs,
490+
env: ProcessEnv.vars)
491+
var driver = try Driver(args: commandLine,
492+
diagnosticsEngine: plan.diagnostics,
493+
fileSystem: target.fs,
494+
executor: executor)
495+
let imports = try driver.performImportPrescan().imports
496+
let dependencyNames = Set(try target.target.recursiveTargetDependencies().map { $0.c99name })
497+
let nonDependencyTargetNames =
498+
Set(plan.targetMap.map { $0.value.target.c99name }
499+
.filter { !dependencyNames.contains($0) })
500+
501+
print("--------------------------------------------------------")
502+
print("Perform target import verification for: \(target.target.c99name)")
503+
print("Imports: \(imports)")
504+
print("Dependencies: \(dependencyNames)")
505+
print("Other targets: \(nonDependencyTargetNames)")
506+
print("--------------------------------------------------------")
507+
508+
let importedTargetsMissingDependency = Set(imports).intersection(nonDependencyTargetNames)
509+
if !importedTargetsMissingDependency.isEmpty {
510+
verificationFailure = true
511+
plan.diagnostics.emit(error: "Target \(target.target.name) imports another target (\(importedTargetsMissingDependency.first!)) in the package without declaring it a dependency.", location: nil)
512+
}
513+
} catch {
514+
print("Unable to verify target imports due to: \(error)")
515+
return
516+
}
517+
if verificationFailure {
518+
throw StringError("Build manifest creation failed.")
519+
}
520+
}
521+
477522
private func addCmdWithBuiltinSwiftTool(
478523
_ target: SwiftTargetBuildDescription,
479524
inputs: [Node],
480525
cmdOutputs: [Node]
481-
) {
526+
) throws {
482527
let isLibrary = target.target.type == .library || target.target.type == .test
483528
let cmdName = target.target.getCommandName(config: buildConfig)
484529

530+
// Emit an error if this target imports another target in this build
531+
// without specifying it as a dependency
532+
try verifyTargetImports(target)
533+
485534
manifest.addSwiftCmd(
486535
name: cmdName,
487536
inputs: inputs,

Tests/BuildTests/BuildPlanTests.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,84 @@ final class BuildPlanTests: XCTestCase {
167167
#endif
168168
}
169169

170+
func testTargetImportDependenciesOnlyVerification() throws {
171+
try withTemporaryDirectory { path in
172+
// Create a test package with three targets:
173+
// A and B
174+
let fs = localFileSystem
175+
try fs.changeCurrentWorkingDirectory(to: path)
176+
let testDirPath = path.appending(component: "VerificationTest")
177+
let buildDirPath = path.appending(component: ".build")
178+
let sourcesPath = testDirPath.appending(component: "Sources")
179+
let aPath = sourcesPath.appending(component: "A")
180+
let bPath = sourcesPath.appending(component: "B")
181+
try fs.createDirectory(testDirPath)
182+
try fs.createDirectory(buildDirPath)
183+
try fs.createDirectory(sourcesPath)
184+
try fs.createDirectory(aPath)
185+
try fs.createDirectory(bPath)
186+
let main = aPath.appending(component: "main.swift")
187+
let aSwift = aPath.appending(component: "A.swift")
188+
let bSwift = bPath.appending(component: "B.swift")
189+
try localFileSystem.writeFileContents(main) {
190+
$0 <<< "baz();"
191+
}
192+
try localFileSystem.writeFileContents(aSwift) {
193+
$0 <<< "public func foo() { }"
194+
}
195+
try localFileSystem.writeFileContents(bSwift) {
196+
// This target depends on, and imports 'A', but forgets to
197+
// specify it as dependency in the manifest.
198+
$0 <<< "import A;"
199+
$0 <<< "public func bar() { foo() }"
200+
}
201+
202+
// Plan package build
203+
let diagnostics = DiagnosticsEngine()
204+
let graph = try loadPackageGraph(fs: fs, diagnostics: diagnostics,
205+
manifests: [
206+
Manifest.createV4Manifest(
207+
name: "VerificationTest",
208+
path: testDirPath.description,
209+
packageKind: .root,
210+
packageLocation: "/VerificationTest",
211+
targets: [
212+
TargetDescription(name: "A", dependencies: []),
213+
TargetDescription(name: "B", dependencies: [])
214+
]),
215+
]
216+
)
217+
XCTAssertNoDiagnostics(diagnostics)
218+
do {
219+
let plan = try BuildPlan(
220+
buildParameters: mockBuildParameters(
221+
buildPath: buildDirPath,
222+
config: .release,
223+
toolchain: Resources.default.toolchain,
224+
destinationTriple: Resources.default.toolchain.triple,
225+
useExplicitModuleBuild: false
226+
),
227+
graph: graph,
228+
diagnostics: diagnostics,
229+
fileSystem: fs
230+
)
231+
232+
let yaml = buildDirPath.appending(component: "release.yaml")
233+
let llbuild = LLBuildManifestBuilder(plan)
234+
XCTAssertThrows(StringError("Build manifest creation failed."), { try llbuild.generateManifest(at: yaml) })
235+
XCTAssertEqual(diagnostics.diagnostics.count, 1)
236+
let firstDiagnostic = diagnostics.diagnostics.first.map({ $0.message.text })
237+
XCTAssertTrue(firstDiagnostic == "Target B imports another target (A) in the package without declaring it a dependency.")
238+
} catch Driver.Error.unableToDecodeFrontendTargetInfo {
239+
// If the toolchain being used is sufficiently old, the integrated driver
240+
// will not be able to parse the `-print-target-info` output. In which case,
241+
// we cannot yet rely on the integrated swift driver.
242+
// This effectively guards the test from running on unupported, older toolchains.
243+
throw XCTSkip()
244+
}
245+
}
246+
}
247+
170248
func testExplicitSwiftPackageBuild() throws {
171249
try withTemporaryDirectory { path in
172250
// Create a test package with three targets:

0 commit comments

Comments
 (0)