-
Notifications
You must be signed in to change notification settings - Fork 712
[RFC] Split modular solver into separate package #2928
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
[RFC] Split modular solver into separate package #2928
Conversation
Using explicit export lists allows for trivial detection of dead code, and this commit also removes quite a bit of said dead code.
Oh, yes: This represents a much more conservative approach than #2768, obviously. I figure this is a reasonable first step towards figuring out which bits (if any) should be integrated into Cabal, which bits (if any) could/should be moved to an entirely separate package (that doesn't depend on Cabal), and which bits should remain in the "realm" of the Cabal/cabal-install project. My hunch would be that most projects that are interested in the solver are interested in depending on Cabal too, so we'll see, I guess... I do want to land this + #2924 as soon as possible to avoid further conflicts with @kosmikus 's work, so review and comments would be much appreciated -- I don't want this to linger too long in the "patch queue" and bitrot beyond repair. :) |
Hopefully the conflicts shouldn't actually be too bad -- this really is mostly just moving stuff around, but I'm OK with that, I guess as long as it doesn't mean that this has to wait for several weeks (or something like that). However, I'd really like #2924 to get in first, if possible. Have you tried to see if that one conflicts with your stuff? |
@BardurArantsson Git compared the moved files properly, but there were large blocks of conflicting code. Merging #2924 doesn't look too difficult. There are large conflicts in |
Yeah, for this PR, I've sprinkled the "loc" type parameter all over the place in the "extracted" solver code. Essentially PackageLocation has become a type parameter to PackageSource and (transitively) a few other types. It's pretty mechanical, but still requires a bit of care to make sure the types line up. If you have a diff viewer which can show only the bit of each line that changed, it should all be pretty simple. No logic whatsoever has changed, only type signatures. As I say, I'm OK with redoing it -- it's just a bit annoying. As for #2924 : Yup, no changes to any logic whatsoever. An easy way to resolve the conflicts would be to just keep all your changes except grabbing the export lists (+ perhaps any additions you've made) from my branch and then deleting any code which GHC says is dead. Still, I'm not sure who is going to review and how we can manage this in an orderly fashion... maybe ping the mailing list? |
I'll just close this in favor of #2768 which I'll try to follow up on in the next few weeks. No need to spam the PR list. |
This is just an RFC, it's not meant for merging.
Notes:
setup-dev.sh
has been updated, so please use that if you want to try building for yourself.I think that's it for notes.
[1] As with the modular solver it revolves around the use of PackageLocation. I originally moved PackageLocation into the
cabal-solver
package, but gradually migrated it back tocabal-install
by making type signatures incabal-solver
more parametric.