Skip to content

Commit 4258f94

Browse files
miggs597miggs597
authored
avoid crashing when constraint is not found (#3340)
motivation: less crashes changes: removed the assert that was causing the crash and instead use a guard to throws a StringError
1 parent 1f2eda3 commit 4258f94

File tree

7 files changed

+39
-33
lines changed

7 files changed

+39
-33
lines changed

‎Sources/PackageGraph/PackageGraphRoot.swift

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,43 +69,49 @@ public struct PackageGraphRoot {
6969
}
7070

7171
/// Returns the constraints imposed by root manifests + dependencies.
72-
public func constraints() -> [PackageContainerConstraint] {
72+
public func constraints() throws -> [PackageContainerConstraint] {
7373
let constraints = packageRefs.map{
7474
PackageContainerConstraint(package: $0, requirement: .unversioned, products: .everything)
7575
}
76-
return constraints + dependencies.map{
76+
77+
let depend = try dependencies.map{
7778
PackageContainerConstraint(
7879
package: $0.createPackageRef(),
79-
requirement: $0.toConstraintRequirement(),
80+
requirement: try $0.toConstraintRequirement(),
8081
products: $0.productFilter
8182
)
8283
}
84+
return constraints + depend
8385
}
8486
}
8587

8688
extension PackageDependencyDescription {
8789
/// Returns the constraint requirement representation.
88-
public func toConstraintRequirement() -> PackageRequirement {
90+
public func toConstraintRequirement() throws -> PackageRequirement {
8991
switch self {
9092
case .local:
9193
return .unversioned
9294
case .scm(let data):
93-
return data.requirement.toConstraintRequirement()
95+
return try data.requirement.toConstraintRequirement()
9496
}
9597
}
9698
}
9799

98100
extension PackageDependencyDescription.Requirement {
99101
/// Returns the constraint requirement representation.
100-
public func toConstraintRequirement() -> PackageRequirement {
102+
public func toConstraintRequirement() throws -> PackageRequirement {
101103
switch self {
102104
case .range(let range):
103105
return .versionSet(.range(range))
104106
case .revision(let identifier):
105-
assert(Git.checkRefFormat(ref: identifier))
107+
guard Git.checkRefFormat(ref: identifier) else {
108+
throw StringError("Could not find revision: '\(identifier)'")
109+
}
106110
return .revision(identifier)
107111
case .branch(let identifier):
108-
assert(Git.checkRefFormat(ref: identifier))
112+
guard Git.checkRefFormat(ref: identifier) else {
113+
throw StringError("Could not find branch: '\(identifier)'")
114+
}
109115
return .revision(identifier)
110116
case .exact(let version):
111117
return .versionSet(.exact(version))

‎Sources/PackageGraph/PackageModel+Extensions.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ extension PackageDependencyDescription {
3535

3636
extension Manifest {
3737
/// Constructs constraints of the dependencies in the raw package.
38-
public func dependencyConstraints(productFilter: ProductFilter) -> [PackageContainerConstraint] {
39-
return dependenciesRequired(for: productFilter).map({
38+
public func dependencyConstraints(productFilter: ProductFilter) throws -> [PackageContainerConstraint] {
39+
return try dependenciesRequired(for: productFilter).map({
4040
return PackageContainerConstraint(
4141
package: $0.createPackageRef(),
42-
requirement: $0.toConstraintRequirement(),
42+
requirement: try $0.toConstraintRequirement(),
4343
products: $0.productFilter)
4444
})
4545
}

‎Sources/PackageGraph/RepositoryPackageContainer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ public class RepositoryPackageContainer: PackageContainer, CustomStringConvertib
222222
productFilter: ProductFilter
223223
) throws -> (Manifest, [Constraint]) {
224224
let manifest = try self.loadManifest(at: revision, version: version)
225-
return (manifest, manifest.dependencyConstraints(productFilter: productFilter))
225+
return (manifest, try manifest.dependencyConstraints(productFilter: productFilter))
226226
}
227227

228228
public func getUnversionedDependencies(productFilter: ProductFilter) throws -> [Constraint] {

‎Sources/SPMTestSupport/MockWorkspace.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ public final class MockWorkspace {
359359

360360
let dependencyManifests = try workspace.loadDependencyManifests(root: root, diagnostics: diagnostics)
361361

362-
let result = workspace.precomputeResolution(
362+
let result = try workspace.precomputeResolution(
363363
root: root,
364364
dependencyManifests: dependencyManifests,
365365
pinsStore: pinsStore,

‎Sources/Workspace/ResolverPrecomputationProvider.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,15 @@ private struct LocalPackageContainer: PackageContainer {
131131
func getDependencies(at version: Version, productFilter: ProductFilter) throws -> [PackageContainerConstraint] {
132132
// Because of the implementation of `reversedVersions`, we should only get the exact same version.
133133
precondition(dependency?.checkoutState?.version == version)
134-
return manifest.dependencyConstraints(productFilter: productFilter)
134+
return try manifest.dependencyConstraints(productFilter: productFilter)
135135
}
136136

137137
func getDependencies(at revision: String, productFilter: ProductFilter) throws -> [PackageContainerConstraint] {
138138
// Return the dependencies if the checkout state matches the revision.
139139
if let checkoutState = dependency?.checkoutState,
140140
checkoutState.version == nil,
141141
checkoutState.revision.identifier == revision {
142-
return manifest.dependencyConstraints(productFilter: productFilter)
142+
return try manifest.dependencyConstraints(productFilter: productFilter)
143143
}
144144

145145
throw ResolverPrecomputationError.differentRequirement(
@@ -159,7 +159,7 @@ private struct LocalPackageContainer: PackageContainer {
159159
)
160160
}
161161

162-
return manifest.dependencyConstraints(productFilter: productFilter)
162+
return try manifest.dependencyConstraints(productFilter: productFilter)
163163
}
164164

165165
// Gets the package reference from the managed dependency or computes it for root packages.

‎Sources/Workspace/Workspace.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ extension Workspace {
567567
var updateConstraints = currentManifests.editedPackagesConstraints()
568568

569569
// Create constraints based on root manifest and pins for the update resolution.
570-
updateConstraints += graphRoot.constraints()
570+
updateConstraints += try graphRoot.constraints()
571571

572572
let pinsMap: PinsStore.PinsMap
573573
if packages.isEmpty {
@@ -1122,7 +1122,7 @@ extension Workspace {
11221122
}
11231123

11241124
/// Returns constraints of the dependencies, including edited package constraints.
1125-
func dependencyConstraints() -> [PackageContainerConstraint] {
1125+
func dependencyConstraints() throws -> [PackageContainerConstraint] {
11261126
var allConstraints = [PackageContainerConstraint]()
11271127

11281128
for (externalManifest, managedDependency, productFilter) in dependencies {
@@ -1144,7 +1144,7 @@ extension Workspace {
11441144
case .checkout, .local:
11451145
break
11461146
}
1147-
allConstraints += externalManifest.dependencyConstraints(productFilter: productFilter)
1147+
allConstraints += try externalManifest.dependencyConstraints(productFilter: productFilter)
11481148
}
11491149
return allConstraints
11501150
}
@@ -1811,7 +1811,7 @@ extension Workspace {
18111811

18121812
let currentManifests = try self.loadDependencyManifests(root: graphRoot, diagnostics: diagnostics)
18131813

1814-
let precomputationResult = precomputeResolution(
1814+
let precomputationResult = try precomputeResolution(
18151815
root: graphRoot,
18161816
dependencyManifests: currentManifests,
18171817
pinsStore: pinsStore
@@ -1874,7 +1874,7 @@ extension Workspace {
18741874
} else if !extraConstraints.isEmpty || forceResolution {
18751875
delegate?.willResolveDependencies(reason: .forced)
18761876
} else {
1877-
let result = precomputeResolution(
1877+
let result = try precomputeResolution(
18781878
root: graphRoot,
18791879
dependencyManifests: currentManifests,
18801880
pinsStore: pinsStore,
@@ -1897,7 +1897,7 @@ extension Workspace {
18971897
// Create the constraints.
18981898
var constraints = [PackageContainerConstraint]()
18991899
constraints += currentManifests.editedPackagesConstraints()
1900-
constraints += graphRoot.constraints() + extraConstraints
1900+
constraints += try graphRoot.constraints() + extraConstraints
19011901

19021902
// Perform dependency resolution.
19031903
let resolver = createResolver(pinsMap: pinsStore.pinsMap)
@@ -2002,11 +2002,11 @@ extension Workspace {
20022002
dependencyManifests: DependencyManifests,
20032003
pinsStore: PinsStore,
20042004
extraConstraints: [PackageContainerConstraint] = []
2005-
) -> ResolutionPrecomputationResult {
2005+
) throws -> ResolutionPrecomputationResult {
20062006
let constraints =
2007-
root.constraints() +
2007+
try root.constraints() +
20082008
// Include constraints from the manifests in the graph root.
2009-
root.manifests.flatMap({ $0.dependencyConstraints(productFilter: .everything) }) +
2009+
root.manifests.flatMap({ try $0.dependencyConstraints(productFilter: .everything) }) +
20102010
dependencyManifests.dependencyConstraints() +
20112011
extraConstraints
20122012

‎Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,10 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
387387
"Bar2": .specific(["B2", "Bar1", "Bar3"]),
388388
"Bar3": .specific(["Bar1", "Bar3"]),
389389
]
390-
let v5Constraints = dependencies.map {
390+
let v5Constraints = try dependencies.map {
391391
PackageContainerConstraint(
392392
package: $0.createPackageRef(),
393-
requirement: $0.toConstraintRequirement(),
393+
requirement: try $0.toConstraintRequirement(),
394394
products: v5ProductMapping[$0.nameForTargetDependencyResolutionOnly]!
395395
)
396396
}
@@ -399,10 +399,10 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
399399
"Bar2": .specific(["B2"]),
400400
"Bar3": .specific(["Bar3"]),
401401
]
402-
let v5_2Constraints = dependencies.map {
402+
let v5_2Constraints = try dependencies.map {
403403
PackageContainerConstraint(
404404
package: $0.createPackageRef(),
405-
requirement: $0.toConstraintRequirement(),
405+
requirement: try $0.toConstraintRequirement(),
406406
products: v5_2ProductMapping[$0.nameForTargetDependencyResolutionOnly]!
407407
)
408408
}
@@ -420,7 +420,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
420420
)
421421

422422
XCTAssertEqual(
423-
manifest
423+
try manifest
424424
.dependencyConstraints(productFilter: .everything)
425425
.sorted(by: { $0.package.identity < $1.package.identity }),
426426
[
@@ -444,7 +444,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
444444
)
445445

446446
XCTAssertEqual(
447-
manifest
447+
try manifest
448448
.dependencyConstraints(productFilter: .everything)
449449
.sorted(by: { $0.package.identity < $1.package.identity }),
450450
[
@@ -468,7 +468,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
468468
)
469469

470470
XCTAssertEqual(
471-
manifest
471+
try manifest
472472
.dependencyConstraints(productFilter: .everything)
473473
.sorted(by: { $0.package.identity < $1.package.identity }),
474474
[
@@ -492,7 +492,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
492492
)
493493

494494
XCTAssertEqual(
495-
manifest
495+
try manifest
496496
.dependencyConstraints(productFilter: .specific(Set(products.map { $0.name })))
497497
.sorted(by: { $0.package.identity < $1.package.identity }),
498498
[

0 commit comments

Comments
 (0)