Skip to content

Commit 350745a

Browse files
committed
[Build] Emit BuildPlan diagnostics to DiagnosticsEngine
<rdar://problem/41342798> SwiftPM is not warning about non whitelisted flags <rdar://problem/40260898> [SR-7690]: SwiftPM does not print any warnings which is must be emitted at all during `build plan` including message about pkg-config provider hinting feature
1 parent f20b42d commit 350745a

File tree

5 files changed

+64
-28
lines changed

5 files changed

+64
-28
lines changed

Sources/Build/BuildPlan.swift

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -524,13 +524,6 @@ public final class ProductBuildDescription {
524524
}
525525
}
526526

527-
/// The delegate interface used by the build plan to report status information.
528-
public protocol BuildPlanDelegate: class {
529-
530-
/// The build plan emitted this warning.
531-
func warning(message: String)
532-
}
533-
534527
/// A build plan for a package graph.
535528
public class BuildPlan {
536529

@@ -573,9 +566,6 @@ public class BuildPlan {
573566
return AnySequence(productMap.values)
574567
}
575568

576-
/// Build plan delegate.
577-
public let delegate: BuildPlanDelegate?
578-
579569
/// The filesystem to operate on.
580570
let fileSystem: FileSystem
581571

@@ -587,13 +577,11 @@ public class BuildPlan {
587577
buildParameters: BuildParameters,
588578
graph: PackageGraph,
589579
diagnostics: DiagnosticsEngine,
590-
delegate: BuildPlanDelegate? = nil,
591580
fileSystem: FileSystem = localFileSystem
592581
) throws {
593582
self.buildParameters = buildParameters
594583
self.graph = graph
595584
self.diagnostics = diagnostics
596-
self.delegate = delegate
597585
self.fileSystem = fileSystem
598586

599587
// Create build target description for each target which we need to plan.
@@ -828,10 +816,9 @@ public class BuildPlan {
828816
}
829817
// If there is no pc file on system and we have an available provider, emit a warning.
830818
if let provider = result.provider, result.couldNotFindConfigFile {
831-
delegate?.warning(message: "you may be able to install \(result.pkgConfigName) using your system-packager:")
832-
delegate?.warning(message: provider.installText)
819+
diagnostics.emit(data: PkgConfigHintDiagnostic(pkgConfigName: result.pkgConfigName, installText: provider.installText))
833820
} else if let error = result.error {
834-
delegate?.warning(message: "error while trying to use pkgConfig flags for \(target.name): \(error)")
821+
diagnostics.emit(error)
835822
}
836823
pkgConfigCache[target] = (result.cFlags, result.libs)
837824
return pkgConfigCache[target]!
@@ -840,3 +827,18 @@ public class BuildPlan {
840827
/// Cache for pkgConfig flags.
841828
private var pkgConfigCache = [SystemLibraryTarget: (cFlags: [String], libs: [String])]()
842829
}
830+
831+
public struct PkgConfigHintDiagnostic: DiagnosticData {
832+
public static let id = DiagnosticID(
833+
type: PkgConfigHintDiagnostic.self,
834+
name: "org.swift.diags.pkg-config-hint",
835+
defaultBehavior: .warning,
836+
description: {
837+
$0 <<< "you may be able to install" <<< { $0.pkgConfigName } <<< "using your system-packager:\n"
838+
$0 <<< { $0.installText }
839+
}
840+
)
841+
842+
let pkgConfigName: String
843+
let installText: String
844+
}

Sources/Commands/SwiftTestTool.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ struct SpecifierDeprecatedDiagnostic: DiagnosticData {
3030
)
3131
}
3232

33+
struct LinuxTestDiscoveryDiagnostic: DiagnosticData {
34+
static let id = DiagnosticID(
35+
type: LinuxTestDiscoveryDiagnostic.self,
36+
name: "org.swift.diags.linux-test-discovery",
37+
defaultBehavior: .warning,
38+
description: {
39+
$0 <<< "can't discover tests on Linux; please use this option on macOS instead"
40+
}
41+
)
42+
}
43+
3344
/// Diagnostic data for zero --filter matches.
3445
struct NoMatchingTestsWarning: DiagnosticData {
3546
static let id = DiagnosticID(
@@ -185,7 +196,7 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
185196

186197
case .generateLinuxMain:
187198
#if os(Linux)
188-
warning(message: "can't discover new tests on Linux; please use this option on macOS instead")
199+
diagnostics.emit(data: LinuxTestDiscoveryDiagnostic())
189200
#endif
190201
let graph = try loadPackageGraph()
191202
let testPath = try buildTestsIfNeeded(options, graph: graph)

Sources/Commands/SwiftTool.swift

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -572,11 +572,6 @@ public class SwiftTool<Options: ToolOptions> {
572572

573573
/// Build a subset of products and targets using swift-build-tool.
574574
func build(plan: BuildPlan, subset: BuildSubset) throws {
575-
guard !plan.graph.rootPackages[0].targets.isEmpty else {
576-
warning(message: "no targets to build in package")
577-
return
578-
}
579-
580575
guard let llbuildTargetName = subset.llbuildTargetName(for: plan.graph, diagnostics: diagnostics) else {
581576
return
582577
}
@@ -738,13 +733,6 @@ enum BuildSubset {
738733
case target(String)
739734
}
740735

741-
extension SwiftTool: BuildPlanDelegate {
742-
public func warning(message: String) {
743-
// FIXME: Coloring would be nice.
744-
print("warning: " + message)
745-
}
746-
}
747-
748736
extension BuildSubset {
749737
/// Returns the name of the llbuild target that corresponds to the build subset.
750738
func llbuildTargetName(for graph: PackageGraph, diagnostics: DiagnosticsEngine) -> String? {

Tests/BuildTests/BuildPlanTests.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,40 @@ final class BuildPlanTests: XCTestCase {
827827
graph: graph, diagnostics: diagnostics, fileSystem: fs)
828828
}
829829
}
830+
831+
func testPkgConfigError() throws {
832+
let fileSystem = InMemoryFileSystem(emptyFiles:
833+
"/A/Sources/ATarget/foo.swift",
834+
"/A/Sources/BTarget/module.modulemap"
835+
)
836+
837+
let diagnostics = DiagnosticsEngine()
838+
let graph = loadPackageGraph(root: "/A", fs: fileSystem, diagnostics: diagnostics,
839+
manifests: [
840+
Manifest.createV4Manifest(
841+
name: "A",
842+
path: "/A",
843+
url: "/A",
844+
targets: [
845+
TargetDescription(name: "ATarget", dependencies: ["BTarget"]),
846+
TargetDescription(
847+
name: "BTarget",
848+
type: .system,
849+
pkgConfig: "BTarget",
850+
providers: [
851+
.brew(["BTarget"]),
852+
.apt(["BTarget"]),
853+
]
854+
)
855+
]),
856+
]
857+
)
858+
859+
_ = try BuildPlan(buildParameters: mockBuildParameters(),
860+
graph: graph, diagnostics: diagnostics, fileSystem: fileSystem)
861+
862+
XCTAssertTrue(diagnostics.diagnostics.contains(where: { ($0.data is PkgConfigHintDiagnostic) }))
863+
}
830864
}
831865

832866
// MARK:- Test Helpers

Tests/BuildTests/XCTestManifests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ extension BuildPlanTests {
1414
("testDynamicProducts", testDynamicProducts),
1515
("testExecAsDependency", testExecAsDependency),
1616
("testNonReachableProductsAndTargets", testNonReachableProductsAndTargets),
17+
("testPkgConfigError", testPkgConfigError),
1718
("testSwiftCMixed", testSwiftCMixed),
1819
("testSystemPackageBuildPlan", testSystemPackageBuildPlan),
1920
("testTestModule", testTestModule),

0 commit comments

Comments
 (0)