Skip to content

[5.6] fix dependency resolution edge cases issues (#3985) #3997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 9, 2022
Merged

[5.6] fix dependency resolution edge cases issues (#3985) #3997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 32 additions & 22 deletions Sources/PackageGraph/Pubgrub/PubgrubDependencyResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1216,41 +1216,51 @@ internal final class PubGrubPackageContainer {

// Since the pinned version is most likely to succeed, we don't compute bounds for its
// incompatibilities.
return try Array(constraints.map { (constraint: PackageContainerConstraint) -> [Incompatibility] in
guard case .versionSet(let vs) = constraint.requirement else {
return try constraints.flatMap { constraint -> [Incompatibility] in
// We only have version-based requirements at this point.
guard case .versionSet(let constraintRequirement) = constraint.requirement else {
throw InternalError("Unexpected unversioned requirement: \(constraint)")
}
return try constraint.nodes().map { dependencyNode in
var terms: OrderedSet<Term> = []
// the package version requirement
terms.append(Term(node, .exact(version)))
terms.append(Term(not: dependencyNode, vs))
// the dependency's version requirement
terms.append(Term(not: dependencyNode, constraintRequirement))
return try Incompatibility(terms, root: root, cause: .dependency(node: node))
}
}.joined())
}
}

let (lowerBounds, upperBounds) = try self.computeBounds(for: node,
constraints: constraints,
startingWith: version)

return try constraints.flatMap { constraint in
return try constraint.nodes().map { constraintNode in
var terms: OrderedSet<Term> = []
let lowerBound = lowerBounds[constraintNode] ?? "0.0.0"
let upperBound = upperBounds[constraintNode] ?? Version(version.major + 1, 0, 0)
assert(lowerBound < upperBound)
let (lowerBounds, upperBounds) = try self.computeBounds(
for: node,
constraints: constraints,
startingWith: version
)

// We only have version-based requirements at this point.
guard case .versionSet(let vs) = constraint.requirement else {
throw InternalError("Unexpected unversioned requirement: \(constraint)")
return try constraints.flatMap { constraint -> [Incompatibility] in
// We only have version-based requirements at this point.
guard case .versionSet(let constraintRequirement) = constraint.requirement else {
throw InternalError("Unexpected unversioned requirement: \(constraint)")
}
return try constraint.nodes().compactMap { constraintNode in
// cycle
guard node != constraintNode else {
return nil
}

let requirement: VersionSetSpecifier = .range(lowerBound ..< upperBound)
terms.append(Term(node, requirement))
terms.append(Term(not: constraintNode, vs))

// Make a record for this dependency so we don't have to recompute the bounds when the selected version falls within the bounds.
self.emittedIncompatibilities[constraintNode] = requirement.union(emittedIncompatibilities[constraintNode] ?? .empty)
if let lowerBound = lowerBounds[constraintNode], let upperBound = upperBounds[constraintNode] {
assert(lowerBound < upperBound)
let requirement: VersionSetSpecifier = .range(lowerBound ..< upperBound)
self.emittedIncompatibilities[constraintNode] = requirement.union(emittedIncompatibilities[constraintNode] ?? .empty)
}

var terms: OrderedSet<Term> = []
// the package version requirement
terms.append(Term(node, .exact(version)))
// the dependency's version requirement
terms.append(Term(not: constraintNode, constraintRequirement))

return try Incompatibility(terms, root: root, cause: .dependency(node: node))
}
Expand Down
25 changes: 19 additions & 6 deletions Sources/Workspace/Workspace.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ extension Workspace {
observabilityScope: ObservabilityScope
) throws -> [(PackageReference, Workspace.PackageStateChange)]? {
// Create cache directories.
createCacheDirectories(observabilityScope: observabilityScope)
self.createCacheDirectories(observabilityScope: observabilityScope)

// FIXME: this should not block
// Load the root manifests and currently checked out manifests.
Expand All @@ -799,7 +799,9 @@ extension Workspace {
guard let pinsStore = observabilityScope.trap({ try self.pinsStore.load() }) else { return nil }

// Ensure we don't have any error at this point.
guard !observabilityScope.errorsReported else { return nil }
guard !observabilityScope.errorsReported else {
return nil
}

// Add unversioned constraints for edited packages.
var updateConstraints = currentManifests.editedPackagesConstraints()
Expand Down Expand Up @@ -830,17 +832,28 @@ extension Workspace {
// Reset the active resolver.
self.activeResolver = nil

guard !observabilityScope.errorsReported else { return nil }
guard !observabilityScope.errorsReported else {
return nil
}

if dryRun {
return observabilityScope.trap { return try computePackageStateChanges(root: graphRoot, resolvedDependencies: updateResults, updateBranches: true, observabilityScope: observabilityScope) }
return observabilityScope.trap {
return try self.computePackageStateChanges(root: graphRoot, resolvedDependencies: updateResults, updateBranches: true, observabilityScope: observabilityScope)
}
}

// Update the checkouts based on new dependency resolution.
let packageStateChanges = self.updateDependenciesCheckouts(root: graphRoot, updateResults: updateResults, updateBranches: true, observabilityScope: observabilityScope)

// Load the updated manifests.
let updatedDependencyManifests = try self.loadDependencyManifests(root: graphRoot, observabilityScope: observabilityScope)
// If we have missing packages, something is fundamentally wrong with the resolution of the graph
let stillMissingPackages = updatedDependencyManifests.computePackages().missing
guard stillMissingPackages.isEmpty else {
let missing = stillMissingPackages.map{ $0.description }
observabilityScope.emit(error: "exhausted attempts to resolve the dependencies graph, with '\(missing.joined(separator: "', '"))' unresolved.")
return nil
}

// Update the resolved file.
self.saveResolvedFile(
Expand Down Expand Up @@ -1721,7 +1734,7 @@ extension Workspace {
let dependenciesToLoad = dependenciesRequired.map{ $0.createPackageRef() }.filter { !loadedManifests.keys.contains($0.identity) }
let dependenciesManifests = try temp_await { self.loadManagedManifests(for: dependenciesToLoad, observabilityScope: observabilityScope, completion: $0) }
dependenciesManifests.forEach { loadedManifests[$0.key] = $0.value }
return pair.item.dependenciesRequired(for: pair.key.productFilter).compactMap { dependency in
return dependenciesRequired.compactMap { dependency in
loadedManifests[dependency.identity].flatMap {
// we also compare the location as this function may attempt to load
// dependencies that have the same identity but from a different location
Expand Down Expand Up @@ -2704,7 +2717,7 @@ extension Workspace {
) -> [(PackageReference, PackageStateChange)] {
// Get the update package states from resolved results.
guard let packageStateChanges = observabilityScope.trap({
try computePackageStateChanges(root: root, resolvedDependencies: updateResults, updateBranches: updateBranches, observabilityScope: observabilityScope)
try self.computePackageStateChanges(root: root, resolvedDependencies: updateResults, updateBranches: updateBranches, observabilityScope: observabilityScope)
}) else {
return []
}
Expand Down
Loading