Skip to content
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

InstallPlan refactor v3 #3622

Merged
merged 17 commits into from
Jul 26, 2016
Merged

InstallPlan refactor v3 #3622

merged 17 commits into from
Jul 26, 2016

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented Jul 25, 2016

Rebased version of #3609. Not yet included fixes from #3611, but each commit does compile and pass tests on at least linux with one ghc version.

dcoutts added 13 commits July 25, 2016 01:09
The main thing this adds is infrastructure for generating random
InstallPlans (which is not totally trivial given their structure).
Needed for some new InstallPlan functions
Currently the way to traverse/execute an InstallPlan involves using the
InstallPlan itself as the processing state.

This adds a new Processing type and associated operations to help write
InstallPlan traversals that are separate from the InstallPlan itself.
This is a replacement for the existing linearizeInstallPlan utils.
It uses the new separate traversal approach.
Delete two different ugly versions of linearizeInstallPlan. Yay.
This is intended to replace a couple existing executeInstallPlan
implementations.

The tests check that execute visits packages in reverse topological
order, for both serial and parallel job control. There's also a check
that serial execute and executionOrder use exactly the same order.
Eliminate the local executeInstallPlan. The main change is that the new
execute returns BuildResults instead of an upated InstallPlan. This has
a few knock-on conseuqnces for the code that looks at the result of the
build execution.

Currently we don't actually do that much with the results in the
new-build code path (though we should) so there's less disruption than
one might imagine. The biggest change is in the integration tests which
do inspect the execution result to check things worked or didn't work as
expected.

The equivalent change for the old build code path will be more
disruptive since it does a lot of stuff with the execution results.
Redefine the local executeInstallPlan util in terms of
InstallPlan.execute. Of course now executeInstallPlan returns
BuildResults instead of an upated InstallPlan. This requires changes in
all the code that looks at the result of the build execution, including
console error reporting, build reports, symlinks etc.
These were the primitives used by executeInstallPlan to change the
package states to Processing, Installed, Failed etc.

The 'ready' is still used in a couple other places, so those need to be
modified before the old 'ready' can be removed.
There were two uses other than excuteInstallPlan. The new ready impl can
be used in both these cases, allowing the old one to be removed.
Rename from prime' versions to the normal names, now that the old ones
have been removed.
We now just have the initial states, PreExisting and Configured. The
Processing, Installed and Failed states are gone. We now do traversals
separately from the InstallPlan, so we no longer need these states.
These were used previously for the Installed and Failed package states,
but these states are now gone.

Importantly this now means that we can have a serialisable InstallPlan
without the failure types having to be serialisable. This means we can
use things like SomeException which is not serialisable. Since the
traversal is done separately, the result of the traversal contains the
failure values, but this result set does not have to be serialised.
@dcoutts dcoutts force-pushed the installplan-refactor branch from 595fb87 to 0bb80be Compare July 25, 2016 19:22
@dcoutts dcoutts changed the title InstallPlan refactor InstallPlan refactor v3 Jul 25, 2016
@phadej phadej mentioned this pull request Jul 26, 2016
@phadej phadej merged commit 62bfec9 into haskell:master Jul 26, 2016
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.

3 participants