-
Notifications
You must be signed in to change notification settings - Fork 697
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
Reject solver solutions involving un-buildable libraries. #3988
Conversation
Now you get something like this: Resolving dependencies... cabal: Could not resolve dependencies: trying: p-0.1 (user goal) unknown package: unbuildable (dependency of p-0.1) fail (backjumping, conflict set: p, unbuildable) Dependency tree exhaustively searched. The error message is not the best. Fixes haskell#3978 Signed-off-by: Edward Z. Yang <[email protected]>
@ezyang, thanks for your PR! By analyzing the history of the files in this pull request, we identified @grayjay, @kosmikus and @BardurArantsson to be potential reviewers. |
Solver tests fail but I think that is just a matter of the semantics having changed. I'd appreciate opinions on whether or not this semantics change is right, though. |
I don't think this is the best solution to #3978. The error message isn't accurate, and it won't work with build-tools or top-level executables that have unbuildable libraries. I can see why we need to solve for components in the dependency solver now. That would fix #3978, and then we would probably want to treat an unbuildable component like a failure, not something to ignore. If we need to work around the problem with
I agree that the tests should change. They should test disabling an unbuildable executable instead of a library. I can update them. |
Your suggested alternate fix sounds reasonable. Maybe I can also just fix the assert failure directly in cabal-install's code by handling the case when a package has no components. |
@grayjay It's not clear to me that solving for components would help here (not to say it's not, I just don't see it). Another approach that I was thinking of was to keep using the syntactic transformation for executables (so the core solver never sees it, it just gets transformed away), whereas for libraries we would treat it directly. Here's the idea. The
Or for the unix example
And then So this failure might be added directly like other kinds of dependency (pkg, extension, pkg-config etc), except that it's a dependency that is never satisfied, and carries a failure reason (or a built-in failure reason in the case of So concretely my suggestion was to add this fail dep kind (alongside pkg, extensions etc) and in the index conversion, convert |
@dcoutts If I understand @grayjay correctly, then it would be natural for the solver to reject non-buildable components if we'd solve on a per-component basis. But because we solve for packages instead, we currently just disable the non-buildable components yet still consider the package choice valid. The idea to remove packages that have no buildable components left sounds reasonable to me. |
Ok, but this is the correct behaviour for exes (according to the slightly weird behaviour of |
I'm not sure I understand.
Exactly. As I see it, when cabal solves for whole packages,
I thought that the behavior should be similar for libraries and exes, now that packages can depend on build tools. The solver should try to avoid disabling the exe if another package depends on it. |
I started updating the tests (https://github.com/grayjay/cabal/tree/buildable-solver-tests), but I realized that the solver DSL doesn't handle languages and extensions in executables. I'll have to look into that later. |
@grayjay ok yes it's possible to fix by looking at things on a component level, but it's also possible to fix at the package level still. I say this because I assume that going to a component level is not going to be a trivial change we can make quickly (as well as likely having performance implications).
We just have to adjust that logic slightly. We say instead that |
@dcoutts What about a package that has an unbuildable library that is only needed |
@grayjay historically the Cabal spec has not supported that, since it works at the package level. The whole unbuildable thing was an ill-thought-out convenience hack that we now have to interpret in vaguely sane ways. I don't think we need to care too much about corner cases of a bad feature that are not in practice used. |
Okay. Do you think we should work towards deprecating the field? I'm not sure it's a corner case now, though. I just checked hackage, and there are 74 packages and 521 package versions that have at least one |
I don't think it makes sense to deprecate the field until we have per-component solving. |
I was wondering if we might still need Buildable for other things, even if we had per-component solving, such as allowing authors to create flags that control the set of executables that is built when a user installs a package. |
Closing, since #5337 fixed the same issue. |
Now you get something like this:
The error message is not the best.
Fixes #3978
Signed-off-by: Edward Z. Yang [email protected]