Skip to content

Commit fd46cdc

Browse files
authored
[NFC] Remove unused fileSystem from GraphLoadingNode (#7154)
### Motivation: `GraphLoadingNode`s in the same graph always use same file system instance, there's no need to store references to it in each node. ### Modifications: Removed `fileSystem` property on `GraphLoadingNode`, as it also simplifies `Equatable` and `Hashable` implementations on this type. ### Result: `GraphLoadingNode` is now a true value type, with no need to define `Equatable` and `Hashable` manually on it.
1 parent 24b9352 commit fd46cdc

File tree

3 files changed

+24
-35
lines changed

3 files changed

+24
-35
lines changed

Sources/PackageGraph/GraphLoadingNode.swift

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import PackageModel
2020
///
2121
/// - SeeAlso: ``DependencyResolutionNode``
2222
public struct GraphLoadingNode: Equatable, Hashable {
23-
2423
/// The package identity.
2524
public let identity: PackageIdentity
2625

@@ -30,30 +29,16 @@ public struct GraphLoadingNode: Equatable, Hashable {
3029
/// The product filter applied to the package.
3130
public let productFilter: ProductFilter
3231

33-
/// The file system to use for loading the given package.
34-
public let fileSystem: FileSystem
35-
36-
public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter, fileSystem: FileSystem) {
32+
public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter) {
3733
self.identity = identity
3834
self.manifest = manifest
3935
self.productFilter = productFilter
40-
self.fileSystem = fileSystem
4136
}
4237

4338
/// Returns the dependencies required by this node.
4439
internal var requiredDependencies: [PackageDependency] {
4540
return self.manifest.dependenciesRequired(for: self.productFilter)
4641
}
47-
48-
public func hash(into hasher: inout Hasher) {
49-
hasher.combine(identity)
50-
hasher.combine(manifest)
51-
hasher.combine(productFilter)
52-
}
53-
54-
public static func == (lhs: GraphLoadingNode, rhs: GraphLoadingNode) -> Bool {
55-
return lhs.identity == rhs.identity && lhs.manifest == rhs.manifest && lhs.productFilter == rhs.productFilter
56-
}
5742
}
5843

5944
extension GraphLoadingNode: CustomStringConvertible {

Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,14 @@ extension PackageGraph {
4646
root.manifests.forEach {
4747
manifestMap[$0.key] = ($0.value, fileSystem)
4848
}
49-
let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
50-
node.requiredDependencies.compactMap{ dependency in
51-
return manifestMap[dependency.identity].map { (manifest, fileSystem) in
52-
GraphLoadingNode(identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter, fileSystem: fileSystem)
49+
func nodeSuccessorsProvider(node: GraphLoadingNode) -> [GraphLoadingNode] {
50+
node.requiredDependencies.compactMap { dependency in
51+
manifestMap[dependency.identity].map { (manifest, fileSystem) in
52+
GraphLoadingNode(
53+
identity: dependency.identity,
54+
manifest: manifest,
55+
productFilter: dependency.productFilter
56+
)
5357
}
5458
}
5559
}
@@ -59,11 +63,15 @@ extension PackageGraph {
5963
manifestMap[$0.identity]?.manifest
6064
})
6165
let rootManifestNodes = root.packages.map { identity, package in
62-
GraphLoadingNode(identity: identity, manifest: package.manifest, productFilter: .everything, fileSystem: fileSystem)
66+
GraphLoadingNode(identity: identity, manifest: package.manifest, productFilter: .everything)
6367
}
64-
let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependency) -> GraphLoadingNode? in
68+
let rootDependencyNodes = root.dependencies.lazy.compactMap { dependency in
6569
manifestMap[dependency.identity].map {
66-
GraphLoadingNode(identity: dependency.identity, manifest: $0.manifest, productFilter: dependency.productFilter, fileSystem: $0.fs)
70+
GraphLoadingNode(
71+
identity: dependency.identity,
72+
manifest: $0.manifest,
73+
productFilter: dependency.productFilter
74+
)
6775
}
6876
}
6977
let inputManifests = rootManifestNodes + rootDependencyNodes
@@ -72,13 +80,13 @@ extension PackageGraph {
7280
var allNodes: [GraphLoadingNode]
7381

7482
// Detect cycles in manifest dependencies.
75-
if let cycle = findCycle(inputManifests, successors: successors) {
83+
if let cycle = findCycle(inputManifests, successors: nodeSuccessorsProvider) {
7684
observabilityScope.emit(PackageGraphError.cycleDetected(cycle))
7785
// Break the cycle so we can build a partial package graph.
7886
allNodes = inputManifests.filter({ $0.manifest != cycle.cycle[0] })
7987
} else {
80-
// Sort all manifests toplogically.
81-
allNodes = try topologicalSort(inputManifests, successors: successors)
88+
// Sort all manifests topologically.
89+
allNodes = try topologicalSort(inputManifests, successors: nodeSuccessorsProvider)
8290
}
8391

8492
var flattenedManifests: [PackageIdentity: GraphLoadingNode] = [:]
@@ -87,8 +95,7 @@ extension PackageGraph {
8795
let merged = GraphLoadingNode(
8896
identity: node.identity,
8997
manifest: node.manifest,
90-
productFilter: existing.productFilter.union(node.productFilter),
91-
fileSystem: node.fileSystem
98+
productFilter: existing.productFilter.union(node.productFilter)
9299
)
93100
flattenedManifests[node.identity] = merged
94101
} else {
@@ -123,7 +130,7 @@ extension PackageGraph {
123130
shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
124131
testEntryPointPath: testEntryPointPath,
125132
createREPLProduct: manifest.packageKind.isRoot ? createREPLProduct : false,
126-
fileSystem: node.fileSystem,
133+
fileSystem: fileSystem,
127134
observabilityScope: nodeObservabilityScope
128135
)
129136
let package = try builder.construct()

Sources/Workspace/Workspace+Manifests.swift

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,7 @@ extension Workspace {
181181
let node = GraphLoadingNode(
182182
identity: identity,
183183
manifest: package.manifest,
184-
productFilter: .everything,
185-
fileSystem: workspace.fileSystem
184+
productFilter: .everything
186185
)
187186
return node
188187
} + root.dependencies.compactMap { dependency in
@@ -192,8 +191,7 @@ extension Workspace {
192191
GraphLoadingNode(
193192
identity: dependency.identity,
194193
manifest: manifest,
195-
productFilter: dependency.productFilter,
196-
fileSystem: workspace.fileSystem
194+
productFilter: dependency.productFilter
197195
)
198196
}
199197
}
@@ -247,8 +245,7 @@ extension Workspace {
247245
GraphLoadingNode(
248246
identity: dependency.identity,
249247
manifest: manifest,
250-
productFilter: dependency.productFilter,
251-
fileSystem: workspace.fileSystem
248+
productFilter: dependency.productFilter
252249
)
253250
}
254251
}

0 commit comments

Comments
 (0)