Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Auto merge of #6650 - greysteil:dont-mutate-original-trees, r=segiddins
Browse files Browse the repository at this point in the history
Don't mutate original error trees when determining version_conflict_message

*I know I'm messing with private APIs here, so have no right to expect this PR merged!*

Currently, when Bundler generates its error message for a version conflict, it mutates the `requirement_trees` array on the underlying `Molinillo::VersionConflict` error.

I would really like it if it didn't. Dependabot taps into that error to understand what went wrong with resolution and unlock any dependencies that are blocking resolution. The code where we do so is [here](https://github.com/dependabot/dependabot-core/blob/9abcb85ba69b408b11b56572dd13ab89d0c55b8c/lib/dependabot/file_updaters/ruby/bundler/lockfile_updater.rb#L142-L167).

I don't think that there's any downside to not mutating here. Resolution has finished (and failed) at this stage, so trying to reduce memory usage doesn't really matter.

🙏

(cherry picked from commit fc60fe4)
  • Loading branch information
bundlerbot authored and colby-swandale committed Aug 16, 2018
1 parent b98b558 commit c93b998
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/bundler/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ def version_conflict_message(e)
:solver_name => "Bundler",
:possibility_type => "gem",
:reduce_trees => lambda do |trees|
# called first, because we want to reduce the amount of work required to find maximal empty sets
trees = trees.uniq {|t| t.flatten.map {|dep| [dep.name, dep.requirement] } }

# bail out if tree size is too big for Array#combination to make any sense
return trees if trees.size > 15
maximal = 1.upto(trees.size).map do |size|
Expand Down

0 comments on commit c93b998

Please sign in to comment.