Skip to content

Commit

Permalink
Consider dependency possibilities in groups
Browse files Browse the repository at this point in the history 
Dependencies usually don't change their sub-dependency requirements when they
update. As such, we can treat several versions of the same dependency as
functionally equivalent when resolving the dependency tree.

To exploit the above, we introduce a PossibilitySet class, which holds an array
of dependency possibilities, all of which have the same sub-dependencies.
Vertices are pinned to a PossibilitySet, rather than a specific version, during
resolution, with the set filtered as required throughout the process. A concrete
version of each dependency is then chosen once resolution as concluded.
  • Loading branch information
@greysteil @segiddins
greysteil authored and segiddins committed Jul 17, 2017
1 parent 6efefc7 commit efd40f6
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 119 deletions.
19 changes: 11 additions & 8 deletions ARCHITECTURE.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,22 @@ This stack-based approach is used because backtracking (also known as *unwinding
2. The client calls `resolve` with an array of user-requested dependencies and an optional 'locking' `DependencyGraph`
3. The `Resolver` creates a new `Resolution` with those four user-specified parameters and calls `resolve` on it
4. The `Resolution` creates an `initial_state`, which takes the user-requested dependencies and puts them into a `DependencyState`
- In the process of creating the state, the `SpecificationProvider` is asked to sort the dependencies and return all the `possibilities` for the `initial_requirement`
- In the process of creating the state, the `SpecificationProvider` is asked to sort the dependencies and return all the `possibilities` for the `initial_requirement` (taking into account whether the dependency is `locked`). These possibilities are then grouped into `PossibilitySet`s, with each set representing a group of versions for the dependency which share the same sub-dependency requirements
- A `DependencyGraph` is created that has all of these requirements point to `root_vertices`
5. The resolution process now enters its main loop, which continues as long as there is a current `state` to process, and the current state has requirements left to process
6. `UI#indicate_progress` is called to allow the client to report progress
7. If the current state is a `DependencyState`, we have it pop off a `PossibilityState` that encapsulates a single possibility for that dependency
7. If the current state is a `DependencyState`, we have it pop off a `PossibilityState` that encapsulates a `PossibilitySet` for that dependency
8. Process the topmost state on the stack
9. If there's a possibility for the state, `attempt_to_activate` it (jump to #11)
10. If there's no possibility, `create_conflict` if the state is a `PossibilityState`, and then `unwind_for_conflict` until there's a `DependencyState` with a `possibility` atop the stack
11. Check if there is an existing vertex in the `activated` dependency graph with the name of the current `requirement`
12. If so, `attempt_to_activate_existing_spec` (jump to #14). If not, `attempt_to_activate_new_spec`
13. Check if there is a `locked` dependency with the current dependency's name. If there is, verify that both the locked dependency and the current `requirement` are satisfied with the current `possibility`. If either is not satisfied, `create_conflict` and `unwind_for_conflict`
14. In either case, if the requirement is satisfied by the existing spec (if existing), or the new spec (if not), a new state is pushed with the now-activated `possibility`'s own dependencies. Go to #5
9. If there is a non-empty `PossibilitySet` for the state, `attempt_to_activate` it (jump to #11)
10. If there is no non-empty `PossibilitySet` for the state, `create_conflict` if the state is a `PossibilityState`, and then `unwind_for_conflict` until there's a `DependencyState` with a non-empty `PossibilitySet` atop the stack
11. Check if there is an existing vertex in the `activated` dependency graph for the dependency this state's `requirement` relates to
12. If there is no existing vertex in the `activated` dependency graph for the dependency this state's `requirement` relates to, `activate_new_spec`. This creates a new vertex in the `activated` dependency graph, with it's payload set to the possibility's `PossibilitySet`. It also pushes a new `DependencyState`, with the now-activated `PossibilitySet`'s own dependencies. Go to #6
13. If there is an existing, `activated` vertex for the dependency, `attempt_to_filter_existing_spec`
- This filters the contents of the existing vertex's `PossibilitySet` by the current state's `requirement`
- If any possibilities remain within the `PossibilitySet`, it updates the activated vertex's payload with the new, filtered state and pushes a new `DependencyState`
- If no possibilities remain within the `PossibilitySet` after filtering, or if the current state's `PossibilitySet` had a different set of sub-dependecy requirements to the existing vertex's `PossibilitySet`, `create_conflict` and `unwind_for_conflict`, back to the last `DependencyState` that has a chance to not generate a conflict. Go to #6
15. Terminate with the topmost state's dependency graph when there are no more requirements left
16. For each vertex with a payload of allowable versions for this resolution (i.e., a `PossibilitySet`), pick a single specific version.

## Specification Provider

Expand Down
2 changes: 1 addition & 1 deletion lib/molinillo/errors.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class CircularDependencyError < ResolverError
# that caused the error
def initialize(vertices)
super "There is a circular dependency between #{vertices.map(&:name).join(' and ')}"
@dependencies = vertices.map(&:payload).to_set
@dependencies = vertices.map { |vertex| vertex.payload.possibilities.last }.to_set
end
end

Expand Down
206 changes: 109 additions & 97 deletions lib/molinillo/resolution.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@ class Resolution
:activated_by_name
)

# A collection of possibility states that share the same dependencies
# @attr [Array] dependencies the dependencies for this set of possibilities
# @attr [Array] possibilities the possibilities
PossibilitySet = Struct.new(:dependencies, :possibilities)

class PossibilitySet
# String representation of the possibility set, for debugging
def to_s
"[#{possibilities.join(', ')}]"
end

# @return [Object] most up-to-date dependency in the possibility set
def latest_version
# TODO: sorting by version here would be a lot better than relying on
# possibilities having been inserted in the right order...
possibilities.last
end
end

# @return [SpecificationProvider] the provider that knows about
# dependencies, requirements, specifications, versions, etc.
attr_reader :specification_provider
Expand Down Expand Up @@ -78,7 +97,7 @@ def resolve
process_topmost_state
end

activated.freeze
resolve_activated_specs
ensure
end_resolution
end
Expand Down Expand Up @@ -109,6 +128,19 @@ def start_resolution
resolver_ui.before_resolution
end

def resolve_activated_specs
activated.vertices.each do |_, vertex|
next unless vertex.payload

latest_version = vertex.payload.possibilities.reverse_each.find do |possibility|
vertex.requirements.uniq.all? { |req| requirement_satisfied_by?(req, activated, possibility) }
end

activated.set_payload(vertex.name, latest_version)
end
activated.freeze
end

# Ends the resolution process
# @return [void]
def end_resolution
Expand Down Expand Up @@ -317,112 +349,60 @@ def attempt_to_activate
existing_vertex = activated.vertex_named(name)
if existing_vertex.payload
debug(depth) { "Found existing spec (#{existing_vertex.payload})" }
attempt_to_activate_existing_spec(existing_vertex.payload)
attempt_to_filter_existing_spec(existing_vertex)
else
attempt_to_activate_new_spec
activate_new_spec
end
end

# Attempts to activate the current {#possibility} (given that it has
# already been activated)
# Attempts to update the existing vertex's `PossibilitySet` with a filtered version
# @return [void]
def attempt_to_activate_existing_spec(spec)
if requirement_satisfied_by?(requirement, activated, spec)
def attempt_to_filter_existing_spec(vertex)
filtered_set = filtered_possibility_set(vertex)
if !filtered_set.possibilities.empty? &&
(vertex.payload.dependencies == dependencies_for(possibility) ||
possibility_set_contains_prereleases(filtered_set))
activated.set_payload(name, filtered_set)
new_requirements = requirements.dup
push_state_for_requirements(new_requirements, false)
else
return if attempt_to_swap_possibility
create_conflict
debug(depth) { "Unsatisfied by existing spec (#{spec})" }
debug(depth) { "Unsatisfied by existing spec (#{vertex.payload})" }
unwind_for_conflict
end
end

# Attempts to swp the current {#possibility} with the already-activated
# spec with the given name
# @return [Boolean] Whether the possibility was swapped into {#activated}
def attempt_to_swap_possibility
activated.tag(:swap)
vertex = activated.vertex_named(name)
activated.set_payload(name, possibility)
if !vertex.requirements.
all? { |r| requirement_satisfied_by?(r, activated, possibility) } ||
!new_spec_satisfied?
activated.rewind_to(:swap)
return
# Generates a filtered version of the existing vertex's `PossibilitySet` using the
# current state's `requirement`
# @param [Object] existing vertex
# @return [PossibilitySet] filtered possibility set
def filtered_possibility_set(vertex)
# Note: we can't just look at the intersection of `vertex.payload.possibilities`
# and `possibility.possibilities`, because if one of our requirements contains
# a prerelease version the associated prerelease versions will only appear in
# one set (but may match all requirements)
filtered_old_values = vertex.payload.possibilities.select do |poss|
requirement_satisfied_by?(requirement, activated, poss)
end
fixup_swapped_children(vertex)
activate_spec
end

# Ensures there are no orphaned successors to the given {vertex}.
# @param [DependencyGraph::Vertex] vertex the vertex to fix up.
# @return [void]
def fixup_swapped_children(vertex) # rubocop:disable Metrics/CyclomaticComplexity
payload = vertex.payload
deps = dependencies_for(payload).group_by(&method(:name_for))
vertex.outgoing_edges.each do |outgoing_edge|
requirement = outgoing_edge.requirement
parent_index = @parents_of[requirement].last
succ = outgoing_edge.destination
matching_deps = Array(deps[succ.name])
dep_matched = matching_deps.include?(requirement)

# only push the current index when it was originally required by the
# same named spec
if parent_index && states[parent_index].name == name
@parents_of[requirement].push(states.size - 1)
end

if matching_deps.empty? && !succ.root? && succ.predecessors.to_a == [vertex]
debug(depth) { "Removing orphaned spec #{succ.name} after swapping #{name}" }
succ.requirements.each { |r| @parents_of.delete(r) }

removed_names = activated.detach_vertex_named(succ.name).map(&:name)
requirements.delete_if do |r|
# the only removed vertices are those with no other requirements,
# so it's safe to delete only based upon name here
removed_names.include?(name_for(r))
end
elsif !dep_matched
debug(depth) { "Removing orphaned dependency #{requirement} after swapping #{name}" }
# also reset if we're removing the edge, but only if its parent has
# already been fixed up
@parents_of[requirement].push(states.size - 1) if @parents_of[requirement].empty?

activated.delete_edge(outgoing_edge)
requirements.delete(requirement)
end
filtered_new_values = possibility.possibilities.select do |poss|
vertex.requirements.uniq.all? { |req| requirement_satisfied_by?(req, activated, poss) }
end
end

# Attempts to activate the current {#possibility} (given that it hasn't
# already been activated)
# @return [void]
def attempt_to_activate_new_spec
if new_spec_satisfied?
activate_spec
else
create_conflict
unwind_for_conflict
end
end

# @return [Boolean] whether the current spec is satisfied as a new
# possibility.
def new_spec_satisfied?
unless requirement_satisfied_by?(requirement, activated, possibility)
debug(depth) { 'Unsatisfied by requested spec' }
return false
end

locked_requirement = locked_requirement_named(name)
# TODO: ideally we should ensure the possibilities set is unique here

locked_spec_satisfied = !locked_requirement ||
requirement_satisfied_by?(locked_requirement, activated, possibility)
debug(depth) { 'Unsatisfied by locked spec' } unless locked_spec_satisfied
PossibilitySet.new(vertex.payload.dependencies, (filtered_old_values + filtered_new_values))
end

locked_spec_satisfied
# Checks whether a `PossibilitySet` contains any pre-release versions. These
# need special treatment, as calling `search_for` on a previous, non-pre-release
# requirement won't have returned any pre-release versions
# @param [PossibilitySet] the possibility set to check
# @return [Boolean] whether or not the given `PossibilitySet` contains any
# pre-release version
def possibility_set_contains_prereleases(possibility_set)
# TODO: figure out a way to check for pre-release versions that doesn't
# call `.version` on user-provided possibilities objects
possibility_set.possibilities.any? { |poss| poss.version.prerelease? }
end

# @param [String] requirement_name the spec name to search for
Expand All @@ -436,22 +416,22 @@ def locked_requirement_named(requirement_name)
# Add the current {#possibility} to the dependency graph of the current
# {#state}
# @return [void]
def activate_spec
def activate_new_spec
conflicts.delete(name)
debug(depth) { "Activated #{name} at #{possibility}" }
activated.set_payload(name, possibility)
require_nested_dependencies_for(possibility)
end

# Requires the dependencies that the recently activated spec has
# @param [Object] activated_spec the specification that has just been
# @param [Object] activated_possibility the PossibilitySet that has just been
# activated
# @return [void]
def require_nested_dependencies_for(activated_spec)
nested_dependencies = dependencies_for(activated_spec)
def require_nested_dependencies_for(possibility_set)
nested_dependencies = dependencies_for(possibility_set.latest_version)
debug(depth) { "Requiring nested dependencies (#{nested_dependencies.join(', ')})" }
nested_dependencies.each do |d|
activated.add_child_vertex(name_for(d), nil, [name_for(activated_spec)], d)
activated.add_child_vertex(name_for(d), nil, [name_for(possibility_set.latest_version)], d)
parent_index = states.size - 1
parents = @parents_of[d]
parents << parent_index if parents.empty?
Expand Down Expand Up @@ -481,16 +461,48 @@ def push_state_for_requirements(new_requirements, requires_sort = true, new_acti
# @return [Array] possibilities
def possibilities_for_requirement(requirement, activated = self.activated)
return [] unless requirement
locked_requirement = locked_requirement_named(name_for(requirement))
if locked_requirement_named(name_for(requirement))
return locked_requirement_possibility_set(requirement, activated)
end

group_possibilities(search_for(requirement))
end

# @param [Object] the proposed requirement
# @return [Array] possibility set containing only the locked requirement, if any
def locked_requirement_possibility_set(requirement, activated = self.activated)
all_possibilities = search_for(requirement)
return all_possibilities unless locked_requirement
locked_requirement = locked_requirement_named(name_for(requirement))

# Longwinded way to build a possibilities array with either the locked
# requirement or nothing in it. Required, since the API for
# locked_requirement isn't guaranteed.
all_possibilities.select do |possibility|
locked_possibilities = all_possibilities.select do |possibility|
requirement_satisfied_by?(locked_requirement, activated, possibility)
end

group_possibilities(locked_possibilities)
end

# Build an array of PossibilitySets, with each element representing a group of
# dependency versions that all have the same sub-dependency version constraints.
# @param [Array] an array of possibilities
# @return [Array] an array of possibility sets
def group_possibilities(possibilities)
possibility_sets = []
possibility_sets_index = {}

possibilities.reverse_each do |possibility|
dependencies = dependencies_for(possibility)
if index = possibility_sets_index[dependencies]
possibility_sets[index].possibilities.unshift(possibility)
else
possibility_sets << PossibilitySet.new(dependencies, [possibility])
possibility_sets_index[dependencies] = possibility_sets.count - 1
end
end

possibility_sets.reverse
end

# Pushes a new {DependencyState}.
Expand Down
11 changes: 5 additions & 6 deletions spec/resolver_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,12 @@ def run(index_class, context)
end

def ignore?(index_class)
if index_class == BerkshelfIndex &&
name == 'can resolve when two specs have the same dependencies and swapping happens' &&
Gem.ruby_version < Gem::Version.new('2.3')
if [BerkshelfIndex, ReverseBundlerIndex].include?(index_class) &&
name == 'can resolve when two specs have the same dependencies and swapping happens'

# That index doesn't do a great job sorting, and segiddins has been
# unable to get the test passing with the bad sort (on Ruby < 2.3)
# without breaking other specs
# These indexes don't do a great job sorting, and segiddins has been
# unable to get the test passing with the bad sort without breaking
# other specs
return true
end

Expand Down
29 changes: 22 additions & 7 deletions spec/spec_helper/index.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,31 @@ def requirement_satisfied_by?(requirement, activated, spec)
requirement
end

existing_vertices = activated.vertices.values.select do |v|
v.name.split('/').first == requirement.name.split('/').first
end
existing = existing_vertices.map(&:payload).compact.first
if existing
existing.version == spec.version && requirement.requirement.satisfied_by?(spec.version)
shared_possibility_versions =
existing_possibility_versions_for_namespace(requirement, activated).
inject { |agg, set| agg & set }

if existing_possibility_versions_for_namespace(requirement, activated).any?
shared_possibility_versions.include?(spec.version) &&
requirement.requirement.satisfied_by?(spec.version)
else
requirement.requirement.satisfied_by? spec.version
requirement.requirement.satisfied_by?(spec.version)
end
end

private

def existing_possibility_versions_for_namespace(requirement, activated)
activated.vertices.values.map do |vertex|
next unless vertex.payload
next unless vertex.name.split('/').first == requirement.name.split('/').first
if vertex.payload.respond_to?(:possibilities)
vertex.payload.possibilities.map(&:version)
else
[vertex.payload.version]
end
end.compact
end
end

class BerkshelfIndex < TestIndex
Expand Down

0 comments on commit efd40f6

Please sign in to comment.