Skip to content

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 39 commits into from
Apr 23, 2016
Merged

Conversation

23Skidoo
Copy link
Member

See #3357.

Iñaki García Etxebarria and others added 30 commits April 23, 2016 23:16
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)
This commit is a pure refactoring, no semantic changes. Submitting as separate
PR at @kosmikus request.

Tihs moves `ConflictSet` and `Var` into their own modules.

(cherry picked from commit 1a1f1f9)
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)
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 81b1f1a)
(cherry picked from commit 6835115)
(cherry picked from commit 6617ca6)
(cherry picked from commit bafcd96)
(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 0a2e772)
(cherry picked from commit 04bc698)
edsko and others added 9 commits April 23, 2016 23:16
(cherry picked from commit 119f3e6)
(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)
@23Skidoo
Copy link
Member Author

This also includes the pkg-config config patch by @garetxe and @dcoutts's patch for monitoring the pkg-config db for changes.

@23Skidoo 23Skidoo merged commit 8f57b3e into haskell:1.24 Apr 23, 2016
@23Skidoo 23Skidoo deleted the solver-1.24 branch April 23, 2016 22:13
@ezyang
Copy link
Contributor

ezyang commented Apr 23, 2016

If so, you need c4e4aed too

@23Skidoo
Copy link
Member Author

@ezyang Thanks, cherry-picked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants