-
Notifications
You must be signed in to change notification settings - Fork 231
Activate the new version solver #1851
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.
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This cleans up some existing code, but more importantly it makes some algorithms for the new version solver more efficient.
This splits up the monolithic version_solver.dart file into one class per file, and renames BacktrackingSolver to VersionSolver since there's only one solver implementation.
This introduces a new algorithm I call "Pubgrub" which is based on cutting-edge techniques for NP-hard search problems. It should substantially reduce the number of cases where the version solver goes exponential, and make it much easier to provide thorough error messages when no solution is available. This commit adds the core of the algorithm, but it's not yet feature- complete with the old version solver. I intend to add support for features like locked dependencies, downgrading, and so on in follow-up CLs. It also doesn't have any kind of error handling yet. As such, pub's test suite is not expected to pass. See #912
This allows the solver to just throw an error on failure, rather than wrapping it in an object to eventually get rethrown.
This will help us explain version lock to users
Previously, we stopped conflict resolution as soon as we knew that no solution could be found. Now we keep deriving new incompatibilities until we reach one that says the root package can't be selected. This allows us to provide a clearer line of reasoning about why version solving failed to the end user.
The root package is now displayed without a version number or constraint, and PackageRef now properly omits the source name for hosted packages and not for other sources instead of vice versa.
This ensures that constraints constructed by the version solver (rather than authored by humans) that look like ^ constraints are displayed tersely.
Unlike the previous SolveFailure architecture, which had separate subclasses for each type of failure, this includes a single IncompatibilityCause that transitively describes the full reason that the root package couldn't be selected successfully.
Non-derived incompatibilities now indicate their causes, and terms with any constraints no longer list their constraints explicitly.
Various refactors in preparation for adding error reporting
This improves some complex error output
This is useful for formatting human-readable error messages.
This was just the root package's directory, which is always the current working directory anyway.
This makes it possible to produce terse output by default while also providing enough detail to diagnose incompatible sources or descriptions.
This helps allow tables conserve horizontal space without affecting readability too drastically.
Add error reporting for the new version solver
Rather than assigning the root package version as a decision, we add an incompatibility indicating that it must be selected. This allows it to work with conflict resolution, so when we derive an incompatibility like {not root >=2.0.0}, we can merge that with {not root 1.0.0} to get an empty incompatibility that indicates solve failure.
This means that if we backtrack into a failure without actually trying a second solution, we don't count an additional attempt.
Most of these are skipped, since they test features that aren't currently supported by the new solver, but at least now we can make sure we don't regress any behavior that's already working.
Now that we're tracking decisions in PartialSolution, we no longer need to distinguish decision-like terms from derivation-like terms.
Refactor some solver stuff
Since these are included in parentheticals, their formatting should be different than usual.
This is used directly, as opposed to through the version solver, in "pub cache add".
We now emit an exit code based on the wrapped exception.
Gracefully handle exceptions during version solving
This test's behavior was never actually stable; which path takes precedence is entirely dependent on the details of the solver, such as which packages sorts alphabetically first. Since it's so implementation-sensitive, it doesn't make much sense to crystallize in a test.
Triage more solver tests
munificent
approved these changes
Mar 27, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉🎉🎉🎉🎉
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've tested this against a sampling of packages on pub.dartlang.org and it seems to be fully compatible. There are a few cases where it produces slightly different output, but all of them are situations where there are multiple valid solutions that differ between choosing a more recent version of package A and an older version of package B, versus choosing an older version of package A and a more recent version of package B. I believe it's safe to enable by default.
There is one outstanding feature that I haven't added support for yet to the new solver: package features. However, these aren't allowed on pub.dartlang.org and in practice I don't believe anyone is using them yet, so I don't want to let them block the roll-out of the new solver.
Closes #912