jhawthorn / pub_grub

🍔 A ruby implementation of the PubGrub CDCL-based version solver
MIT License
151 stars 11 forks source link

Fix crash when dealing with circular requirements #19

Closed deivid-rodriguez closed 1 year ago

deivid-rodriguez commented 1 year ago

When dealing with a situation like this one, PubGrub would previously raise an unexpected error like the following:

...................................F

Failure:
PubGrub::VersionSolverTest#test_circular_dependency [/Users/deivid/Code/jhawthorn/pub_grub/test/pub_grub/version_solver_test.rb:372]:
[PubGrub::SolveFailure] exception expected, not
Class: <ArgumentError>
Message: <"a dependency Incompatibility must have exactly two terms. Got [#<PubGrub::Term circular-dependency < 0>]">
---Backtrace---
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/incompatibility.rb:22:in `initialize'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/basic_package_source.rb:185:in `new'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/basic_package_source.rb:185:in `block in incompatibilities_for'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/basic_package_source.rb:140:in `each'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/basic_package_source.rb:140:in `map'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/basic_package_source.rb:140:in `incompatibilities_for'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/version_solver.rb:136:in `choose_package_version'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/version_solver.rb:41:in `work'
/Users/deivid/Code/jhawthorn/pub_grub/lib/pub_grub/version_solver.rb:58:in `solve'
/Users/deivid/Code/jhawthorn/pub_grub/test/pub_grub/version_solver_test.rb:373:in `block in test_circular_dependency'
---------------

Now it raises a more PubGrub-like user friendly error:

Because every version of circular-dependency depends on itself
  and root depends on circular-dependency >= 0,
  version solving has failed.
deivid-rodriguez commented 1 year ago

@jhawthorn Does this seem ok to you? I merged the Bundler PR but happy to do any amendments as follow ups!

deivid-rodriguez commented 1 year ago

Yeah, I figured we'd need more work if we wanted to handle more complex cycles, but I focused on just detecting this simple case and doing something reasonable.

deivid-rodriguez commented 1 year ago

Thanks for having a look!