Skip to content

Commit 72cfc88

Browse files
Improve Resolver Determinism (#3862)
* Added test. * Implemented fix.
1 parent 442f20e commit 72cfc88

File tree

2 files changed

+94
-2
lines changed

2 files changed

+94
-2
lines changed

Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ private func createResolvedPackages(
230230
metadata: package.diagnosticsMetadata
231231
)
232232

233-
var dependencies = [PackageIdentity: ResolvedPackageBuilder]()
233+
var dependencies = OrderedDictionary<PackageIdentity, ResolvedPackageBuilder>()
234234
var dependenciesByNameForTargetDependencyResolution = [String: ResolvedPackageBuilder]()
235235

236236
// Establish the manifest-declared package dependencies.
@@ -257,7 +257,7 @@ private func createResolvedPackages(
257257
// check if this resolved package already listed in the dependencies
258258
// this means that the dependencies share the same identity
259259
// FIXME: this works but the way we find out about this is based on a side effect, need to improve it
260-
guard !dependencies.keys.contains(resolvedPackage.package.identity) else {
260+
guard dependencies[resolvedPackage.package.identity] == nil else {
261261
let error = PackageGraphError.dependencyAlreadySatisfiedByIdentifier(
262262
package: package.identity.description,
263263
dependencyLocation: dependencyLocation,

Tests/PackageGraphTests/PackageGraphTests.swift

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,98 @@ class PackageGraphTests: XCTestCase {
12691269
})
12701270
}
12711271

1272+
func testResolutionDeterminism() throws {
1273+
let fileSystem = InMemoryFileSystem(
1274+
emptyFiles: [
1275+
"/A/Sources/A/A.swift",
1276+
"/B/Sources/B/B.swift",
1277+
"/C/Sources/C/C.swift",
1278+
"/D/Sources/D/D.swift",
1279+
"/E/Sources/E/E.swift",
1280+
"/F/Sources/F/F.swift",
1281+
]
1282+
)
1283+
1284+
let observability = ObservabilitySystem.makeForTesting()
1285+
_ = try loadPackageGraph(
1286+
fs: fileSystem,
1287+
manifests: [
1288+
Manifest.createRootManifest(
1289+
name: "A",
1290+
path: .init("/A"),
1291+
dependencies: [
1292+
.localSourceControl(path: .init("/B"), requirement: .upToNextMajor(from: "1.0.0")),
1293+
.localSourceControl(path: .init("/C"), requirement: .upToNextMajor(from: "1.0.0")),
1294+
.localSourceControl(path: .init("/D"), requirement: .upToNextMajor(from: "1.0.0")),
1295+
.localSourceControl(path: .init("/E"), requirement: .upToNextMajor(from: "1.0.0")),
1296+
.localSourceControl(path: .init("/F"), requirement: .upToNextMajor(from: "1.0.0")),
1297+
],
1298+
targets: [
1299+
TargetDescription(name: "A", dependencies: []),
1300+
]),
1301+
Manifest.createFileSystemManifest(
1302+
name: "B",
1303+
path: .init("/B"),
1304+
products: [
1305+
ProductDescription(name: "B", type: .library(.automatic), targets: ["B"])
1306+
],
1307+
targets: [
1308+
TargetDescription(name: "B"),
1309+
]
1310+
),
1311+
Manifest.createFileSystemManifest(
1312+
name: "C",
1313+
path: .init("/C"),
1314+
products: [
1315+
ProductDescription(name: "C", type: .library(.automatic), targets: ["C"])
1316+
],
1317+
targets: [
1318+
TargetDescription(name: "C"),
1319+
]
1320+
),
1321+
Manifest.createFileSystemManifest(
1322+
name: "D",
1323+
path: .init("/D"),
1324+
products: [
1325+
ProductDescription(name: "D", type: .library(.automatic), targets: ["D"])
1326+
],
1327+
targets: [
1328+
TargetDescription(name: "D"),
1329+
]
1330+
),
1331+
Manifest.createFileSystemManifest(
1332+
name: "E",
1333+
path: .init("/E"),
1334+
products: [
1335+
ProductDescription(name: "E", type: .library(.automatic), targets: ["E"])
1336+
],
1337+
targets: [
1338+
TargetDescription(name: "E"),
1339+
]
1340+
),
1341+
Manifest.createFileSystemManifest(
1342+
name: "F",
1343+
path: .init("/F"),
1344+
products: [
1345+
ProductDescription(name: "F", type: .library(.automatic), targets: ["F"])
1346+
],
1347+
targets: [
1348+
TargetDescription(name: "F"),
1349+
]
1350+
),
1351+
],
1352+
observabilityScope: observability.topScope
1353+
)
1354+
1355+
testDiagnostics(observability.diagnostics) { result in
1356+
result.check(diagnostic: "dependency 'b' is not used by any target", severity: .warning)
1357+
result.check(diagnostic: "dependency 'c' is not used by any target", severity: .warning)
1358+
result.check(diagnostic: "dependency 'd' is not used by any target", severity: .warning)
1359+
result.check(diagnostic: "dependency 'e' is not used by any target", severity: .warning)
1360+
result.check(diagnostic: "dependency 'f' is not used by any target", severity: .warning)
1361+
}
1362+
}
1363+
12721364
func testTargetDependencies_Pre52() throws {
12731365
let fs = InMemoryFileSystem(emptyFiles:
12741366
"/Foo/Sources/Foo/foo.swift",

0 commit comments

Comments
 (0)