Implemented an iterative version of Bundler's dependency resolving algorithm

[Who828]

Hey folks,

I reimplemented the Bundler's dependency resolving algorithm iteratively as it was giving stack overflow errors for lot of people. Also this implementation is much more easier to understand than the previous one (catch and throw to unwind recursion!). The main crux of this algorithm is to rely on a stack and create a struct to handle the state of requirement and activated gems. (basically create a data structure to handle the call stack). It's still using the original BFS search, so no changes there.

I also took the time to fix bugs #2532 and #2464 and included the spec written by Hemant (@gnufied) to make sure its 100% fixed. I have also included a complex conflict spec because my algorithm passed all the bundler specs but failed this one (now its passing, of course).

My implementation is based on the Rubygems version of Resolver, if I didn't have that code as reference I don't think I would have been able to come up this algorithm. (Thank you so much @evanphx!), I have also deleted the unneeded code because of this algorithm. (mainly the safe_catch code and its specs)

I think I can still do the following,

1. Add more comments (if needed)
2. Refactor the code (now that I finally understand it)
3. Test the performance (it shouldn't have degraded)

However, I wanted to showcase this work to the Bundler's team. Since this algorithm is a core part of Bundler and the correctness of this algorithm triumphs everything else.

I just wanted to see if you folks are happy with the work and if I didn't miss any corner cases.

PS: This is my first non-trivial opensource patch, I am very nervous at the moment!

[xaviershay]

Thanks for the contribution! You certainly chose a tricky problem for your first patch :)

I'm not familiar enough with this part of the system to review it adequately, will let others chime in. I do approve of the net-negative amount of code.

[indirect]

This is amazing! We’ve been kicking this idea around for at least a few months, so it’s pretty amazing (and impressive) that you’ve tackled it. It will probably take me until the weekend to go through this, but I will be very happy to do so. Thanks.

On Nov 21, 2013, at 8:14 PM, Xavier Shay notifications@github.com wrote:

Thanks for the contribution! You certainly chose a tricky problem for your first patch :)

I'm not familiar enough with this part of the system to review it adequately, will let others chime in. I do approve of the net-negative amount of code.

—
Reply to this email directly or view it on GitHub.

[headius]

Way to go, @Who828 :-)

[arthurnn]

❤️ 💚 💜 💛

[simi]

@Who828 You're 🐉 ❤️.

[Who828]

Hey,

Just added a fix + spec for an issue I had missed out on, I hope there are not many of these.

[gnufied]

@Who828 Awesome work. <3

[gja]

Woah. Awesome work <3 @Who828

[radar]

This. Is. AMAZING.

On Fri, Nov 22, 2013 at 5:26 PM, Tejas Dinkar notifications@github.com
wrote:

Woah. Awesome work <3 @Who828

Reply to this email directly or view it on GitHub:
#2726 (comment)

[jaseemabid]

+1 @Who828

[jessicard]

yeaaaahhhh @Who828 ❤️

[kaiwren]

Awesome @Who828 - so proud!

[avinasha]

Great work @Who828! 👏 👏

[dblock]

I've dabbled with this thing before, and you, sir, get a standing ovation.

[skottler]

❤️ 🎧 🌟

[TimMoore]

Fantastic! This will make a lot of people very happy.

[ahacking]

Solving the version dependancies falls into a Constraint Satisfaction Problem (CSP). I have heard mention of backtracking in the bundler algorithm so I'm assuming there must be an established CSP algorithm being used?? I don't have time presently to investigate but I am wondering if the implementation has considered established algorithms for solving this kind of problem, namely AC 3 and more recent improvements AC 3.1 and AC|DC-2i ?

Introduction to constraint satisfaction problems: http://4c.ucc.ie/web/outreach/tutorial.html

The AC 3.1 algorithm with pseudo code:
http://www.dcs.gla.ac.uk/~pat/cpM/papers/ac2001b.pdf
http://www.academia.edu/1470263/Dynamic_Arc_Consistency_for_CSPs

The AC|DC-2i algorithm with pseudo code:
http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.90.3011&rep=rep1&type=pdf

[Who828]

@ahacking Yes, Bundler does make use of forward checking, It doesn't blindly try all possibles gem versions of a particular gem. It finds the gem versions based on the current requirement (the one which doesn't conflict with any active gems so far).

The only time we backtrack is in case that we activated a new gem later on and its requirements are conflicting with the current activated gems or whenever the search of gem versions is coming up empty because of the activated gems so far.

We also do form a preprocessing of gem requirements by sorting it in a order which is easiest to resolve.

Of course the algorithm can always be improved, I will check out arc consistency as well. Though from what I have read so far, arc consistency requires more time than forward checking though it's more effective at pruning the search spaces.

I should probably refer to "Artificial Intelligence: A Modern Approach", it had a complete section on Constraint Satisfaction Problem if I recall correctly. (Though it has the AC 3, unlike the newer versions you have mentioned).

Thanks for your input though, I learned something new today :)

[bf4]

When will @github create a 👍 ? 💯

[pawelchcki]

Great work @Who828. I've stumbled upon bug similar to #2596 and was going to report it but I've found your PR and it seems to resolve it.

Any news when this could be merged?

[evanphx]

This implementation has bugs that I fixed in Rubygems and I'm not sure are fixed in Bundler.

[indirect]

@evanphx could you provide more specifics? This implementation passes the entire Bundler test suite.

[Who828]

@evanphx umm do you mean this bug rubygems/rubygems@c3811c1 which you fixed recently? If so, I tried a similar spec in Bundler and it seems to be working!

Let me know if there is something else I missed out on.

[evanphx]

It's more
rubygems/rubygems@ea4529e#diff-607f8edf7b1be36adff54586257ce961actually.
All my commits on
https://github.com/rubygems/rubygems/commits/master/lib/rubygems/resolver.rbwe
need to be sure are in the bundler merged version. @drbrain had a test
case I used (though the specs in there should cover it...).

On Mon, Dec 23, 2013 at 10:18 PM, Smit Shah notifications@github.comwrote:

@evanphx https://github.com/evanphx umm do you mean this bug
rubygems/rubygems@c3811c1rubygems/rubygems@c3811c183521d65092d5924fc9e807f98a58bdfbwhich you fixed recently? If so, I tried a similar spec in Bundler and it
seems to be working!

Let me know if there is something else I missed out on.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2726#issuecomment-31159056
.