-
Notifications
You must be signed in to change notification settings - Fork 712
Port solver fixes from master into 1.24 #3373
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
When solving, we now discard plans that would involve packages with a pkgconfig-depends constraint which is not satisfiable with the current set of installed packages (as listed by pkg-config --list-all). This fixes haskell#3016. It is possible (in principle, although it should be basically impossible in practice) that "pkg-config --modversion pkg1 pkg2... pkgN" fails to execute for various reasons, in particular because N is too large, so the command line becomes too long for the operating system limits. If this happens, revert to the previous behavior of accepting any install plan, regardless of any pkgconfig-depends constraints. (cherry picked from commit c72aa8d) + Monitor the pkg-config db for changes We use the pkg-config db as an input for the solver, so we ought to monitor it for changes. The pkg-config tool makes this possible, if not totally trivial. (cherry picked from commit f968b41)
As it turns out we often get spurious test failures, and it's generally more useful to be able to actually build *everything* rather than having the tests pass while building. (We already have pre-merge Travis checks, so it's not that likely that something will slip through anyway.) (cherry picked from commit 2ce3fde)
(cherry picked from commit 2cee0fe)
In the original implemented we recorded only the two conflicting packages; in the "fix" in haskell#3221 we include the GRC for one of the two packages, but not the other. This is also wrong, although slightly less so and @kosmikus tells me that it's actually rather difficult to find an example that exposes the remaining inaccuracy. I'll have to take him at his word on that :) In this commit we use the GRC for both goals instead. (cherry picked from commit 3bc157a)
The cycle check was not constructing the correct conflict sets; when we found a cycle, we constructed a conflict set consisting of all choices that we made so far, as long as those choices directly involved a package in the cycle. But sometimes the dependencies are more subtle than that. Now instead we construct conflict sets from all packages _and their goal reason chains_ that were involved in the cycle and then union all those. (cherry picked from commit 503593c)
(cherry picked from commit 8ddf587)
(cherry picked from commit 91e2be1)
(cherry picked from commit 6a4aa92)
(cherry picked from commit 98c7fef)
(cherry picked from commit 8c22df4)
(cherry picked from commit 81b1f1a)
(cherry picked from commit 2f4c440)
(cherry picked from commit 0fe6258)
(cherry picked from commit 6835115)
(cherry picked from commit 2cfe4e3)
(cherry picked from commit 6617ca6)
(cherry picked from commit bafcd96)
(cherry picked from commit ed0f002)
(cherry picked from commit 7995ea2)
(cherry picked from commit 9a3e339)
and fix the error message in showMessages; this keeps the construction of conflict sets uniform. This still isn't quite right though. If we remove B from db21 as well, we don't see a line "unknown package B" appearing in the log. I have no idea why. Also, @kosmikus says: > ok, so first problem: we still need to add C to the conflict set > one way to do this now is to change the "initial" conflict set in the backjump calls to be the node + the goal reason again > I was assuming that the initial set is only ever added to an existing conflict set that we already have > but this is now no longer true I have no idea what that means :) (cherry picked from commit 5a4149f)
When we are choosing to link a package (`pickLink`), it is possible that that package was _already_ linked (because its reverse dependencies got linked). Thus, this requires a merge of link groups: it is not correct to simply add the package into the target link group. This fixes haskell#2842. (cherry picked from commit 34bfdf8)
(cherry picked from commit 440033d)
… in link group This commit replaces a call to 'lgBlame' with a call to 'lgConflictSet'. 'lgConflictSet' adds the members of the link group to the conflict set. The other members are required, for example, when a linked package's version doesn't match a constraint, and the solver must try other versions for the linked-to package. All tests now pass. (cherry picked from commit 7f48059)
(cherry picked from commit 26252b4)
(cherry picked from commit 0a2e772)
(cherry picked from commit 188e4f4)
(cherry picked from commit 04bc698)
(cherry picked from commit 49b422b)
(cherry picked from commit 119f3e6)
See haskell#3268 (comment) (cherry picked from commit 4ff4e2b)
(cherry picked from commit 162d258)
This is relevant for unknown packages. Without the current goal in the initial conflict set, the unknown package itself does not end up in its own conflict set. But we want it to, if only to make sure that the error slicing machinery keeps the message about the unknown goal around. It is also the correct thing to do. The initial conflict set corresponds to a virtual "avoid" choice underneath the package goal. That choice corresponds to avoiding assigning any proper value to the goal at all, which might be possible if the goal was not needed. The conflict set for this "avoid" goal is always between the current package and its immediate goal reason. I have added comments to the code explaining this as well. (cherry picked from commit 018e936)
In several places where we used to work with Goals mainly to maintain the goal reasons for constructing conflict sets, we can now do so without, and just use Vars instead. This also has the advantage that when creating dependencies during index conversion, we now do not have to invent goal reasons. We used to do that by having an "Unknown" goal reason. This is now no longer necessary. (cherry picked from commit 16cae8a)
Added this so that we can verify the unknown package error. (cherry picked from commit 268edad)
This commit removes the 'ExTest' constructor from 'ExampleDependency'. Test dependencies are now represented using the same 'ExampleDependency' constructors as library dependencies. The only difference between dependencies of different components is that they are placed under different keys in 'D.C.ComponentDeps.ComponentDeps'.
(cherry picked from commit ef050ee)
(cherry picked from commit 2fc1c14)
If so, you need c4e4aed too |
@ezyang Thanks, cherry-picked. |
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.
See #3357.