-
Notifications
You must be signed in to change notification settings - Fork 696
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
Build plan only succeeds with --reorder-goals #1855
Comments
/cc @kosmikus |
I'm also having trouble with |
This looks worrying. To summarize: it shouldn't happen that a solution is I suspect a subtle bug in the computation or interpretation of conflict I will look into this. |
I encountered the same issue. The following cabal file seems to reproduce the problem:
|
Ok, I cannot tell yet whether all of this is the same issue. But I can say that I've tracked down the cause of what @bergey describes to a combination of two issues that, once solved, may or may not fix the others, too. One issue is that manual flags may under certain circumstances cause parts of the search tree to be discarded. The flag The other issue is that flag dependencies are not properly taken into account when computing conflict sets. Here, |
@kosmikus This sounds like an issue @cartazio opened against text-stream-decode (fpco/text-stream-decode#1). I'm not sure if he ever turned it into a cabal-install issue, so linking to it here in case it helps. |
@snoyberg @cartazio From a quick look, it seems as if fpco/text-stream-decode#1 is more related to haskell/aeson#177 and #1831 than this one. |
This was an innocent-looking but severe bug that was triggered for manual flags in certain circumstances: if both choices of a manual flag are already invalidated by other decisions, then the old code was removing the nodes underneath the flag completely. As a result, there's a node with no children rather than a node with two failure-children. The fail-nodes carry important information that is used to compute how far we can backtrack. If this information is removed, strange things happen.
Package flags generate (Boolean) goals. Package flags can be dependent on one another, in situations such as this one: if flag(a) ... else if flag(b) ... else ... In such a scenario, it's important to record that flag b depends on flag a. This affects conflict set generation. If something fails due to the choice of flag b, we should not backjump beyond flag a. While the code handling the proper insertion of goals with their correct dependencies was always there, it was accidentally overridden by another piece of code that created flag goals (without dependencies) for all flags defined in a package. The reason I add flag goals separately is because not all paths in the decision tree may contain choices for all flags of a package. For example, if a is chosen to be True in the example above, b does not occur at all. But flag choices may still affect other things that aren't visible to the solver (directory choices, exposed modules, ...), so all flags declared should always be chosen explicitly. So we want to keep adding all flags declared in a package as dummies, but we have to make sure to do so *before* adding the actual dependency tree. This way, while traversing the dependency tree, the ones occurring in dependencies will be added again, overriding the dummies, rather than the other way round (which we used to have before, where the dummies were overwriting the more informative versions).
Some more comments: The problem by @bergey should be fixed by the pull request I just sent. The test case provided by @twittner, too (thanks!). It should also fix the related problem reported on the cabal-devel mailing list today by Björn Peemöller (http://www.haskell.org/pipermail/cabal-devel/2014-May/009795.html). I cannot check the original test case provided by @snoyberg easily, but at least it should no longer report "Dependency tree exhaustively searched." It could still be that The keter problem reported by @snoyberg in #1877 is not fixed by this. This looks like it's caused by "strong flag handling" instead, which is related to haskell/aeson#177 and #1831 (and, as @snoyberg pointed out earlier, probably to fpco/text-stream-decode#1, too). I'll say more about this soon. |
I am also seeing something like this with See outputs for these two cases here: |
@gibiansky This is just exactly the same issue, even triggered by the same package again ( |
@kosmikus Alright, cool, glad there's a fix on the way. |
This was an innocent-looking but severe bug that was triggered for manual flags in certain circumstances: if both choices of a manual flag are already invalidated by other decisions, then the old code was removing the nodes underneath the flag completely. As a result, there's a node with no children rather than a node with two failure-children. The fail-nodes carry important information that is used to compute how far we can backtrack. If this information is removed, strange things happen. (cherry picked from commit 3e33a0f)
Package flags generate (Boolean) goals. Package flags can be dependent on one another, in situations such as this one: if flag(a) ... else if flag(b) ... else ... In such a scenario, it's important to record that flag b depends on flag a. This affects conflict set generation. If something fails due to the choice of flag b, we should not backjump beyond flag a. While the code handling the proper insertion of goals with their correct dependencies was always there, it was accidentally overridden by another piece of code that created flag goals (without dependencies) for all flags defined in a package. The reason I add flag goals separately is because not all paths in the decision tree may contain choices for all flags of a package. For example, if a is chosen to be True in the example above, b does not occur at all. But flag choices may still affect other things that aren't visible to the solver (directory choices, exposed modules, ...), so all flags declared should always be chosen explicitly. So we want to keep adding all flags declared in a package as dummies, but we have to make sure to do so *before* adding the actual dependency tree. This way, while traversing the dependency tree, the ones occurring in dependencies will be added again, overriding the dummies, rather than the other way round (which we used to have before, where the dummies were overwriting the more informative versions). (cherry picked from commit a51b837)
Practically, if I were to remove the |
@ekmett It would probably fix the issue without people having to upgrade |
Thanks. I need to get this to work for users with older cabals as well, because of the very purpose of the package, so if I'm forced into releasing a suboptimal version of the package simply to fix the issue, then I guess that is just the price I have to pay. |
@ekmett It's a maintenance nightmare, but... What about releasing three versions of transformers-compat:
And then cabal will simply select which of those three versions it uses based on the version of transformers. I think that will address the current problem. |
@ekmett What @snoyberg suggests should work. Another option I could think of is to go to two separate transformers-compat packages, each with just one flag. One (let's call it |
I could go for that solution if it is the only thing that works. That said, it'd be a deliberately breaking @mightybyte's "implicit http://softwaresimply.blogspot.com/2014/05/implicit-blacklisting-for-cabal.html If we did that on the minor version instead then his notion would still -Edward On Tue, May 20, 2014 at 10:47 AM, Michael Snoyman
|
Minor version instead of patch seems fine to me. I didn't have any strong reason for one vs the other. |
This was an innocent-looking but severe bug that was triggered for manual flags in certain circumstances: if both choices of a manual flag are already invalidated by other decisions, then the old code was removing the nodes underneath the flag completely. As a result, there's a node with no children rather than a node with two failure-children. The fail-nodes carry important information that is used to compute how far we can backtrack. If this information is removed, strange things happen. (cherry picked from commit 3e33a0f)
Package flags generate (Boolean) goals. Package flags can be dependent on one another, in situations such as this one: if flag(a) ... else if flag(b) ... else ... In such a scenario, it's important to record that flag b depends on flag a. This affects conflict set generation. If something fails due to the choice of flag b, we should not backjump beyond flag a. While the code handling the proper insertion of goals with their correct dependencies was always there, it was accidentally overridden by another piece of code that created flag goals (without dependencies) for all flags defined in a package. The reason I add flag goals separately is because not all paths in the decision tree may contain choices for all flags of a package. For example, if a is chosen to be True in the example above, b does not occur at all. But flag choices may still affect other things that aren't visible to the solver (directory choices, exposed modules, ...), so all flags declared should always be chosen explicitly. So we want to keep adding all flags declared in a package as dummies, but we have to make sure to do so *before* adding the actual dependency tree. This way, while traversing the dependency tree, the ones occurring in dependencies will be added again, overriding the dummies, rather than the other way round (which we used to have before, where the dummies were overwriting the more informative versions). (cherry picked from commit a51b837)
@mietek Ok, unfortunately I'm now certain that there's still a bug related to the original issue, and that the fix in |
Now that it is clear that the elegant version, which was going to be a third of the maintenance effort will not work; yes. ;) |
I am not fully convinced that a further patch to transformers-compat is |
Fair enough. I'll hold back and let you come up with a fix. On Thu, May 29, 2014 at 5:51 PM, kosmikus [email protected] wrote:
|
Just to throw in some more data:
And:
I just ran into this when trying to update a customer's codebase to newer package versions. My current workaround is patching transformers-compat in Stackage to avoid any conditionals (since in Stackage, we're pegged at transformers-0.3.0.0). |
@snoyberg This latest behaviour you're reporting is correct. In |
Something like
should work |
Yes, my mistake with the overly restrictive bounds. The transformers-compat issue does seem to be resolved now. |
How is this resolved? I try installing ad in a fresh sandbox and it fails with
But all is well if I do
|
What version of |
I am using stackage not hackage
|
There are two things which, together, are causing this problem:
Why don't we follow up privately about the best way to address this issue (either moving to a new snapshot, or upgrading cabal-install). Can you email me? |
Ok following up privately. |
I think only 1.20.0.3 and 1.18.0.5 are truly ok as far as cabal-install is concerned. |
@kosmikus I think you're right, but for this specific bug, I think 1.18.0.4 does not reproduce the issue. |
@snoyberg Quite possible. I'm just saying that if anyone is taking the effort to upgrade |
More data:
While with |
@neothemachine you should test that with |
Well, after 15min of 100% CPU usage and 3.5GiB memory usage I killed On 02/09/2015 06:35 PM, Adam Bergmark wrote:
|
This is a workaround for haskell/cabal#1855
This is a workaround for haskell/cabal#1855
This is a workaround for haskell/cabal#1855
This is a workaround for haskell/cabal#1855
Sorry for not following up properly on this. I actually believe this is fixed since 5b63fce. I'm closing this bug. If after that there are still problems, please reopen or better, open a new issue. (And because several things are mixed up in this thread, let me just reiterate: Adding or removing |
I apologize if this is somewhat off-topic. Why not have cabal automatically fall back to reordering goals in the event of the dependency tree being exhaustively searched? If one is supposed to try adding the |
@altaic because it is a bug (that needs to be fixed rather than workarounded). The set of solutions is supposed to be equal whether you give |
In my Stackage builds, I use --max-backjumps=-1 --reorder-goals. Given the recent discussion about reorder-goals both being slow, and possibly causing non-termination in build plan construction, I decided to try out running the build without --reorder-goals. However, doing so caused the build plan to fail. Following is the command I ran, and the output from cabal.
Note that there are some local, patched versions of libraries I'm using here. These patches are all available in the Stackage repo, and I can additionally provide them directly if that would be useful.
The text was updated successfully, but these errors were encountered: