Add InstallPlan invariant assertion checking, revealing cyclic dep problem #4014
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
For the moment we cannot use ghc-options at the top level in the cabal.project since it applies to all packages not just local ones. Until that is fixed, use a workaround of specifying it for the two local packages of interest directly.
plus some minor related tidying up, such as fixing a few stray cases where we still talked about "index" rather than "graph".
It turns out that the install plan elaboration is constructing cyclic plans in some cases. The effect is that executing the plan simply misses out anything that depends on the packages involved in the cycle. This is probably the cause of haskell#3996 With this patch such cases will fail with an assertion such as: internal error in InstallPlan.fromSolverInstallPlanWithProgress: The following packages are involved in a dependency cycle hspec-discover-2.3.1-da63d0b4e952e7949a113646e4af0aac925a4d864a1db650.. The cause is clearly in the caller of fromSolverInstallPlanWithProgress and the only caller of that is ProjectPlanning.elaborateInstallPlan. The problem appears to be to do with the intra-package dependencies.
|
Travis failure is due to DNS outage. |
|
LGTM. |
ezyang
approved these changes
Oct 21, 2016
| @@ -223,17 +223,21 @@ type InstallPlan = GenericInstallPlan | |||
|
|
|||
| -- | Smart constructor that deals with caching the 'Graph' representation. | |||
| -- | |||
| mkInstallPlan :: Graph (GenericPlanPackage ipkg srcpkg) | |||
| mkInstallPlan :: (IsUnit ipkg, IsUnit srcpkg) | |||
There was a problem hiding this comment.
We can get more accurate location tracing by requiring a CallStack (WithCallStack)
There was a problem hiding this comment.
I thought of that but couldn't be bothered with the CPP required given the call stack stuff only works with newer ghcs.
There was a problem hiding this comment.
That's what Distribution.Compat.Stack is for ;)
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.
It turns out that the install plan elaboration is constructing cyclic plans in some cases. The effect is that executing the plan simply misses out anything that depends on the packages involved in the cycle. This is probably the cause of #3996
With this patch such cases will fail with an assertion such as:
The cause is clearly in the caller of
fromSolverInstallPlanWithProgressand the only caller of that isProjectPlanning.elaborateInstallPlan. The problem appears to be to do with the intra-package dependencies. The hspec-discover-2.3.1 example is one where it has a lib and an exe and the exe depends on the lib. The resulting elaborated plan ends up with the lib component with a self-dependency which is clearly wrong.@ezyang would you mind having a look?