Merge pull request #1761 from aciidb0mb3r/stable-dependency-manifests…

…-4.2

[Workspace] Use topological sort when creating DependencyManifests

Sources/Workspace/Workspace.swift

@@ -900,7 +900,7 @@ extension Workspace {
  var loadedManifests = [String: Manifest]()
 
  // Compute the transitive closure of available dependencies.
- let dependencies = transitiveClosure(inputManifests.map({ KeyedPair($0, key: $0.name) })) { node in
+ let allManifests = try! topologicalSort(inputManifests.map({ KeyedPair($0, key: $0.name) })) { node in
  return node.item.package.dependencies.compactMap({ dependency in
  let ref = dependency.createPackageRef()
  let manifest = loadedManifests[ref.identity] ?? loadManifest(for: ref, diagnostics: diagnostics)
@@ -909,10 +909,9 @@ extension Workspace {
  })
  }
 
- // It is possible that some root dependency is also present as a regular dependency, so we
- // form a unique set of all dependency manifests.
- let allManifests = Set(rootDependencyManifests.map({ KeyedPair($0, key: $0.name) }) + dependencies).map({ $0.item })
- let deps: [(Manifest, ManagedDependency)] = allManifests.map({
+ let allDependencyManifests = allManifests.map({ $0.item }).filter({ !root.manifests.contains($0) })
+
+ let deps: [(Manifest, ManagedDependency)] = allDependencyManifests.map({
  // FIXME: We should use package name directly once this radar is fixed:
  // <rdar://problem/33693433> Ensure that identity and package name
  // are the same once we have an API to specify identity in the

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -994,6 +994,72 @@ final class WorkspaceTests: XCTestCase {
  }
  }
 
+ func testDependencyManifestsOrder() throws {
+ let sandbox = AbsolutePath("/tmp/ws/")
+ let fs = InMemoryFileSystem()
+
+ let workspace = try TestWorkspace(
+ sandbox: sandbox,
+ fs: fs,
+ roots: [
+ TestPackage(
+ name: "Root1",
+ targets: [
+ TestTarget(name: "Root1", dependencies: ["Foo", "Bar", "Baz", "Bam"]),
+ ],
+ products: [],
+ dependencies: [
+ TestDependency(name: "Foo", requirement: .upToNextMajor(from: "1.0.0")),
+ TestDependency(name: "Bar", requirement: .upToNextMajor(from: "1.0.0")),
+ TestDependency(name: "Baz", requirement: .upToNextMajor(from: "1.0.0")),
+ TestDependency(name: "Bam", requirement: .upToNextMajor(from: "1.0.0")),
+ ]
+ ),
+ ],
+ packages: [
+ TestPackage(
+ name: "Foo",
+ targets: [
+ TestTarget(name: "Foo", dependencies: ["Bar", "Baz"]),
+ ],
+ products: [
+ TestProduct(name: "Foo", targets: ["Foo"]),
+ ],
+ dependencies: [
+ TestDependency(name: "Bar", requirement: .upToNextMajor(from: "1.0.0")),
+ TestDependency(name: "Baz", requirement: .upToNextMajor(from: "1.0.0")),
+ ],
+ versions: ["1.0.0"]
+ ),
+ .genericPackage1(named: "Bar"),
+ TestPackage(
+ name: "Baz",
+ targets: [
+ TestTarget(name: "Baz", dependencies: ["Bam"]),
+ ],
+ products: [
+ TestProduct(name: "Baz", targets: ["Baz"]),
+ ],
+ dependencies: [
+ TestDependency(name: "Bam", requirement: .upToNextMajor(from: "1.0.0")),
+ ],
+ versions: ["1.0.0"]
+ ),
+ .genericPackage1(named: "Bam"),
+ ]
+ )
+
+ workspace.checkPackageGraph(roots: ["Root1"]) { (graph, diagnostics) in
+ XCTAssertNoDiagnostics(diagnostics)
+ }
+
+ workspace.loadDependencyManifests(roots: ["Root1"]) { (manifests, diagnostics) in
+ // Ensure that the order of the manifests is stable.
+ XCTAssertEqual(manifests.allManifests().map({$0.name}), ["Foo", "Baz", "Bam", "Bar"])
+ XCTAssertNoDiagnostics(diagnostics)
+ }
+ }
+
  func testBranchAndRevision() throws {
  let sandbox = AbsolutePath("/tmp/ws/")
  let fs = InMemoryFileSystem()
@@ -2169,6 +2235,7 @@ final class WorkspaceTests: XCTestCase {
  ("testUpdate", testUpdate),
  ("testCleanAndReset", testCleanAndReset),
  ("testDependencyManifestLoading", testDependencyManifestLoading),
+ ("testDependencyManifestsOrder", testDependencyManifestsOrder),
  ("testBranchAndRevision", testBranchAndRevision),
  ("testResolve", testResolve),
  ("testDeletedCheckoutDirectory", testDeletedCheckoutDirectory),