Add support for base shims to the modular solver #2530
Merged
Conversation
This file contains 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
It turns out not to be the right solution for general private dependencies and is just complicated. However we keep qualified goals, just much simpler. Now dependencies simply inherit the qualification of their parent goal. This gets us closer to the intended behaviour for the --independent-goals feature, and for the simpler case of private dependencies for setup scripts. When not using --independent-goals, the solver behaves exactly as before (tested by comparing solver logs for a hard hackage goal). When using --independent-goals, now every dep of each independent goal is qualified, so the dependencies are solved completely independently (which is actually too much still).
POption annotates a package choice with a "linked to" field. This commit just introduces the datatype and deals with the immediate fallout, it doesn't actually use the field for anything.
This is implemented as a separate pass so that it can be understood independently of the rest of the solver.
In particular, in the definition of dependencyInconsistencies. One slightly annoying thing is that in order to validate an install plan, we need to know if the goals are to be considered independent. This means we need to pass an additional Bool to a few functions; to limit the number of functions where this is necessary, also recorded whether or not goals are independent as part of the InstallPlan itself.
Since we didn't really have a unit test setup for the solver yet, this introduces some basic tests for solver, as well as tests for independent goals specifically.
This address @23Skidoo's comment #2500 (comment)
I don't know why we we constructed this graph manually here rather than calling `graphFromEdges`; it doesn't really matter except that we will want to change the structure of this graph somewhat once we have more fine-grained dependencies, and then the manual construction becomes a bit more painful; easier to use the standard construction.
This commit does nothing but rearrange the Modular.Dependency module into a number of separate sections, so that's a bit clearer to see what's what. No actual code changes here whatsoever.
The ComponentDeps datatype will give us fine-grained information about the dependencies of a package's components. This commit just introduces the datatype, we don't use it anywhere yet.
The modular solver has its own representation for a package (PInfo). In this commit we modify PInfo to keep track of the different kinds of dependencies. This is a bit intricate because the solver also regards top-level goals as dependencies, but of course those dependencies are not part of any 'component' as such, unlike "real" dependencies. We model this by adding a type parameter to FlaggedDeps and go which indicates whether or not we have component information; crucially, underneath flag choices we _always_ have component information available. Consequently, the modular solver itself will not make use of the ComponentDeps datatype (but only using the Component type, classifying components); we will use ComponentDeps when we translate out of the results from the modular solver into cabal-install's main datatypes. We don't yet _return_ fine-grained dependencies from the solver; this will be the subject of the next commit.
In this commit we modify the _output_ of the modular solver (CP, the modular's solver internal version of ConfiguredPackage) to have fine-grained dependency. This doesn't yet modify the rest of cabal-install, so once we translate from CP to ConfiguredPackage we still lose the distinctions between different kinds of dependencies; this will be the topic of the next commit. In the modular solver (and elsewhere) we use Data.Graph to represent the dependency graph (and the reverse dependency graph). However, now that we have more fine-grained dependencies, we really want an _edge-labeled_ graph, which unfortunately it not available in the `containers` package. Therefore I've written a very simple wrapper around Data.Graph that supports edge labels; we don't need many fancy graph algorithms, and can still use Data.Graph on these edged graphs when we want (by calling them on the underlying unlabeled graph), so adding a dependency on `fgl` does not seem worth it.
The crucial change in this commit is the change to PackageFixedDeps to return a
ComponentDeps structure, rather than a flat list of dependencies, as long with
corresponding changes in ConfiguredPackage and ReadyPackage to accomodate this.
We don't actually take _advantage_ of these more fine-grained dependencies yet;
any use of
depends
is now a use of
CD.flatDeps . depends
but we will :)
Note that I have not updated the top-down solver, so in the output of the
top-down solver we cheat and pretend that all dependencies are library
dependencies.
Although we don't use the new setup dependency component anywhere yet, I've replaced all uses of CD.flatDeps with CD.nonSetupDeps. This means that when we do introduce the setup dependencies, all code in Cabal will still use all dependencies except the setup dependencies, just like now. In other words, using the setup dependencies in some places would be a conscious decision; the default is that we leave the behaviour unchanged.
This patch adds it to the package description types and to the parser. There is a new custom setup section which contains the setup script's dependencies. Also add some sanity checks.
(and, therefore, also to the modular solver's output)
By chosing setup dependencies after regular dependencies we get more opportunities for linking setup dependencies against regular dependencies.
The only problematic thing is that when we call `cabal clean` or `cabal
haddock` (and possibly others), _without_ first having called `configure`, we
attempt to build the setup script without calling the solver at all. This means
that if you do, say,
cabal configure
cabal clean
cabal clean
for a package with a custom setup script that really needs setup dependencies
(for instance, because there are two versions of Cabal in the global package DB
and the setup script needs the _older_ one), then first call to `clean` will
succeed, but the second call will fail because we will try to build the setup
script without the solver and that will fail.
This happened independently in a number of places, which was bad; and was about to get worse with the base 3/4 thing.
Never consider flag choices as independent from their package.
|
How does this PR work now? 3/4 of it has been merged already but that's not reflected in the github ui. |
|
Ok, I've re-reviewed the extra patches. Lets find out what the merge does... |
dcoutts
added a commit
that referenced
this pull request
May 21, 2015
Add support for base shims to the modular solver
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.
NOTE: This PR depends on the previous ones and therefore looks big; it's really only the set of commits from "Abstract out qualification of goals"*.
GHC 6 comes with two versions of base: base 3.0 and base 4.0. Base 3.0 is a shim package which depends on base 4.0. Without special support for this in the solver this is a problem, because as soon as we try to configure a package A that depends on base 3, we have a dependency on base 3 and a transitive dependency on base 4, which is not normally possible. The top-down solver has special support for this, but until now it was hard to add special support for this in the modular solver.
However, now that we have a proper implementation of independent goals (PR #2500) it becomes relatively easy to add support for this in the modular solver as well. The idea is that we qualify all dependencies on base, but (unlike other qualifiers) this qualifier is not inherited.
Here's a somewhat idealized example where we have a package A depending on base-3, where base-3 depends on base-4 and syb, and syb also depends on base-4 (this is a cut-down version of the actual dependencies; in particular, in reality base-4 has some other dependencies of its own). To make the example somewhat more interesting we also assume that we have another version of
sybavailable, which also relies onbase-4. The final, pruned search tree looks likeA.base; we have two choices, but must pick version 3 because A wants version 3.base.base; we now have 3 choices: we can pick version 3.0 (not an option because base-3 relies on base-4), we can link it to A.base (also not an option for the same reason), or we can pick base-4.syb. Since A depends on base-3, and base-3 depends onsyb-1, and since we don't inherit the "base" qualifier, this means that A and base must be linked against the same version of syb (version 1); picking version 2 is not possible.syb's dependency onbase, qualified assyb.base. 4 choices now, only one of which is valid. (Picking the non-linked version base-4 is not possible because of the single instance restriction says that if we pick the same version of a package, it must be linked.)I've added some unit tests for this, and also tested it on some simple packages, as well as combinations of packages that use a mixture of base-3 and base-4. I've also ran it against Hackage, but more details on that in the next PR.