Skip to content

Commit ea1730b

Browse files
Improve error message about the PackageGraphError.productDependencyNotFound (#7419)
Fixed a bug in the productDependencyNotFound error message ### Motivation: fix #7398 The above issue suggests the following two defects. 1. package dependency resolution changes depending on the order of packages (alphabetical order) 2. the phrase "Did you meen..." in the error message in the error message is not on target. ### Modifications: The first problem is as described in the issue, if users rename the directory containing Package.swift from repro to zzz, the "Did you meen..." will appear. will appear. Essentially, the error minutes should be displayed in full, regardless of the name of the directory. This is due to the fact that the loop used to resolve dependencies depends on the alphabetical order of the directories and the side effect on allTargetName inside the loop. Therefore, the side effect for allTargetName inside the loop has been moved to the outside of the loop. The second problem is that when dependency A of a package is not found, a strange suggestion "Did you meen `A`? ", which is a strange suggestion. This is because when a dependency is not found, the message "Did you meen `<target name>`" is printed if there is a dependency with a similar name (even the exact same name). Thus, if the names are the same, "Did you meen `.product(name: ... , package: "swift-argugument")"`" instead of the found target name. ### Result: Command to execute: `swift build` Result of command:. `error: 'repro': product 'ArgumentParser' required by package 'repro' target 'repro' not found. Did you mean '.product(name: "ArgumentParser", package: "swift-argument-parser")'?` Condition: The following steps were taken to create the environment. 1. mkdir repro 1. cd repro 1. swift package init --type executable 1. Open Package.swift and make sure it has this content: ```swift // swift-tools-version: 5.10 import PackageDescription let package = Package( name: "repro", dependencies: [ .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.3.0") ], targets: [ .executableTarget( name: "repro", dependencies: ["ArgumentParser"] ) ] ) ``` --------- Co-authored-by: Yuta Saito <[email protected]>
1 parent 9386170 commit ea1730b

File tree

3 files changed

+74
-8
lines changed

3 files changed

+74
-8
lines changed

Sources/PackageGraph/ModulesGraph+Loading.swift

+18-5
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,13 @@ private func createResolvedPackages(
434434
// Track if multiple targets are found with the same name.
435435
var foundDuplicateTarget = false
436436

437+
for packageBuilder in packageBuilders {
438+
for target in packageBuilder.targets {
439+
// Record if we see a duplicate target.
440+
foundDuplicateTarget = foundDuplicateTarget || !allTargetNames.insert(target.target.name).inserted
441+
}
442+
}
443+
437444
// Do another pass and establish product dependencies of each target.
438445
for packageBuilder in packageBuilders {
439446
let package = packageBuilder.package
@@ -493,9 +500,6 @@ private func createResolvedPackages(
493500

494501
// Establish dependencies in each target.
495502
for targetBuilder in packageBuilder.targets {
496-
// Record if we see a duplicate target.
497-
foundDuplicateTarget = foundDuplicateTarget || !allTargetNames.insert(targetBuilder.target.name).inserted
498-
499503
// Directly add all the system module dependencies.
500504
targetBuilder.dependencies += implicitSystemTargetDeps.map { .target($0, conditions: []) }
501505

@@ -524,13 +528,22 @@ private func createResolvedPackages(
524528
} else {
525529
// Find a product name from the available product dependencies that is most similar to the required product name.
526530
let bestMatchedProductName = bestMatch(for: productRef.name, from: Array(allTargetNames))
531+
var packageContainingBestMatchedProduct: String?
532+
if let bestMatchedProductName, productRef.name == bestMatchedProductName {
533+
let dependentPackages = packageBuilder.dependencies.map(\.package)
534+
for p in dependentPackages where p.targets.contains(where: { $0.name == bestMatchedProductName }) {
535+
packageContainingBestMatchedProduct = p.identity.description
536+
break
537+
}
538+
}
527539
let error = PackageGraphError.productDependencyNotFound(
528540
package: package.identity.description,
529541
targetName: targetBuilder.target.name,
530542
dependencyProductName: productRef.name,
531543
dependencyPackageName: productRef.package,
532544
dependencyProductInDecl: !declProductsAsDependency.isEmpty,
533-
similarProductName: bestMatchedProductName
545+
similarProductName: bestMatchedProductName,
546+
packageContainingSimilarProduct: packageContainingBestMatchedProduct
534547
)
535548
packageObservabilityScope.emit(error)
536549
}
@@ -568,7 +581,7 @@ private func createResolvedPackages(
568581
// If a target with similar name was encountered before, we emit a diagnostic.
569582
if foundDuplicateTarget {
570583
var duplicateTargets = [String: [Package]]()
571-
for targetName in allTargetNames.sorted() {
584+
for targetName in Set(allTargetNames).sorted() {
572585
let packages = packageBuilders
573586
.filter({ $0.targets.contains(where: { $0.target.name == targetName }) })
574587
.map{ $0.package }

Sources/PackageGraph/ModulesGraph.swift

+5-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ enum PackageGraphError: Swift.Error {
2323
case cycleDetected((path: [Manifest], cycle: [Manifest]))
2424

2525
/// The product dependency not found.
26-
case productDependencyNotFound(package: String, targetName: String, dependencyProductName: String, dependencyPackageName: String?, dependencyProductInDecl: Bool, similarProductName: String?)
26+
case productDependencyNotFound(package: String, targetName: String, dependencyProductName: String, dependencyPackageName: String?, dependencyProductInDecl: Bool, similarProductName: String?, packageContainingSimilarProduct: String?)
2727

2828
/// The package dependency already satisfied by a different dependency package
2929
case dependencyAlreadySatisfiedByIdentifier(package: String, dependencyLocation: String, otherDependencyURL: String, identity: PackageIdentity)
@@ -230,12 +230,14 @@ extension PackageGraphError: CustomStringConvertible {
230230
(cycle.path + cycle.cycle).map({ $0.displayName }).joined(separator: " -> ") +
231231
" -> " + cycle.cycle[0].displayName
232232

233-
case .productDependencyNotFound(let package, let targetName, let dependencyProductName, let dependencyPackageName, let dependencyProductInDecl, let similarProductName):
233+
case .productDependencyNotFound(let package, let targetName, let dependencyProductName, let dependencyPackageName, let dependencyProductInDecl, let similarProductName, let packageContainingSimilarProduct):
234234
if dependencyProductInDecl {
235235
return "product '\(dependencyProductName)' is declared in the same package '\(package)' and can't be used as a dependency for target '\(targetName)'."
236236
} else {
237237
var description = "product '\(dependencyProductName)' required by package '\(package)' target '\(targetName)' \(dependencyPackageName.map{ "not found in package '\($0)'" } ?? "not found")."
238-
if let similarProductName {
238+
if let similarProductName, let packageContainingSimilarProduct {
239+
description += " Did you mean '.product(name: \"\(similarProductName)\", package: \"\(packageContainingSimilarProduct)\")'?"
240+
} else if let similarProductName {
239241
description += " Did you mean '\(similarProductName)'?"
240242
}
241243
return description

Tests/PackageGraphTests/ModulesGraphTests.swift

+51
Original file line numberDiff line numberDiff line change
@@ -2676,6 +2676,57 @@ final class ModulesGraphTests: XCTestCase {
26762676

26772677
XCTAssertEqual(observability.diagnostics.count, 0, "unexpected diagnostics: \(observability.diagnostics.map { $0.description })")
26782678
}
2679+
2680+
func testDependencyResolutionWithErrorMessages() throws {
2681+
let fs = InMemoryFileSystem(emptyFiles:
2682+
"/aaa/Sources/aaa/main.swift",
2683+
"/zzz/Sources/zzz/source.swift"
2684+
)
2685+
2686+
let observability = ObservabilitySystem.makeForTesting()
2687+
let _ = try loadModulesGraph(
2688+
fileSystem: fs,
2689+
manifests: [
2690+
Manifest.createRootManifest(
2691+
displayName: "aaa",
2692+
path: "/aaa",
2693+
dependencies: [
2694+
.localSourceControl(path: "/zzz", requirement: .upToNextMajor(from: "1.0.0"))
2695+
],
2696+
products: [],
2697+
targets: [
2698+
TargetDescription(
2699+
name: "aaa",
2700+
dependencies: ["zzy"],
2701+
type: .executable
2702+
)
2703+
]),
2704+
Manifest.createRootManifest(
2705+
displayName: "zzz",
2706+
path: "/zzz",
2707+
products: [
2708+
ProductDescription(
2709+
name: "zzz",
2710+
type: .library(.automatic),
2711+
targets: ["zzz"]
2712+
)
2713+
],
2714+
targets: [
2715+
TargetDescription(
2716+
name: "zzz"
2717+
)
2718+
])
2719+
],
2720+
observabilityScope: observability.topScope
2721+
)
2722+
2723+
testDiagnostics(observability.diagnostics) { result in
2724+
result.check(
2725+
diagnostic: "product 'zzy' required by package 'aaa' target 'aaa' not found. Did you mean 'zzz'?",
2726+
severity: .error
2727+
)
2728+
}
2729+
}
26792730
}
26802731

26812732

0 commit comments

Comments
 (0)