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

Solver "round trip" / "freeze" property requires more expressive constraints #3502

Open
dcoutts opened this Issue Jun 20, 2016 · 34 comments

Comments

Projects
None yet
6 participants
@dcoutts
Copy link
Member

dcoutts commented Jun 20, 2016

Here is a property that we would quite like the solver to have:

given any solver result/solution we should be able to construct a set of solver input constraints that uniquely determine the same result (in any environment in which that solution exists)

Or to put it another way, we should be able to freeze the result of the solver and feed it back into the solver later to get the same result. The solver almost has this property but not quite. In particular the introduction of setup deps and qualified goals means that simple "pkg == version" constraints are no longer expressive enough to describe all the solutions the solver can produce. If the solver picks two versions of package foo (e.g. one a "main" lib package and another as a setup dep) then we cannot just constrain "foo == 1.0", we would need to specify the scope of the constraint. Notice also that "primary" vs "setup" dep scope is not enough, since we could pick two versions of foo both as setup deps. Probably what is required to express all the solutions is something corresponding to the internal goal qualifications.

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Jun 20, 2016

@kosmikus @grayjay @edsko opinions? ideas?

dcoutts added a commit to dcoutts/cabal that referenced this issue Jun 20, 2016

Add a new-freeze command
This is ok, but not perfect since freezing is now more tricky with setup
deps. See haskell#3502

dcoutts added a commit to dcoutts/cabal that referenced this issue Jun 20, 2016

Add a new-freeze command
This is ok, but not perfect since freezing is now more tricky with setup
deps. See haskell#3502
@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Jun 21, 2016

Does freeze need to handle changes in the set of installed packages?

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Jun 25, 2016

@grayjay I'm not sure I totally understand the question. Want to grab me on IRC in #hackage for a discussion? Or ask again with more details.

The new freeze command (in PR #3503) takes the current solution (having made sure that was up to date) and translates it into a set of constraints and then writes it out. The new build/repl/configure/freeze all will re-run the solver if the solver inputs would change, so this includes changes in the set of installed packages in the global package db (but not in the user or store dbs) and changes in the set of source pakages from hackage. In the new scheme we never tell the solver about installed packages in the store, just source packages and the global installed packages. I'm not sure if that answers the question.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Jun 25, 2016

@dcoutts I was wondering what should happen when a user runs new-freeze, removes a globally installed package, and then runs new-configure again. Should new-freeze just freeze the version numbers, so that cabal can try to rebuild the previously installed package? Or does it need to ensure that the dependencies are exactly the same, including the flag assignments? I joined #hackage, if you want to discuss it there.

dcoutts added a commit to dcoutts/cabal that referenced this issue Jun 25, 2016

Add a new-freeze command
This is ok, but not perfect since freezing is now more tricky with setup
deps. See haskell#3502
@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Jun 26, 2016

Ah ok. So if at the point when we ran new-freeze the solution involved an installed package from the global db, (e.g. the copy of containers that is bundled with ghc) then yes we only know the version and not flags, so we can only generate version constraints, not flag constraints. For all packages outside of the global db we do know all the flags and do produce flag constraints with new-freeze.

So you're right to point out that this is a slight hole, however it's not one I'm really worried about. While at the moment we use the whole global package db, our assumption is that these days it only really includes a small set. We can make that assumption properly true by using a "core" package db that really only contains the core packages. Also, of the existing packages typically in the global db, I'm not aware of any with automatic flags that significantly affect things.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Jun 29, 2016

@dcoutts Thanks for explaining. I'm still not very familiar with the overall design for new-build, but I think that using goal qualifiers in freeze constraints makes sense. The only problem that I can think of is that we might add flags to modify goal qualification, such as --independent-goals. It's probably not a big deal, though, since we could just specify the goal qualification scheme in the freeze file.

@kosmikus

This comment has been minimized.

Copy link
Contributor

kosmikus commented Jun 29, 2016

@dcoutts In principle, I agree. But for the use case in question, wouldn't it be even better if we'd have a dedicated new solver (we could call it --explicit-solver or --trivial-solver) that won't solve at all, but simply deserialize an install plan that is passed via some mechanism. The sanity check would still run, of course.

Or are there particular reasons why you want to have this feature in connection with being able to make some modifications, and need the actual solver machinery in the background?

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jun 29, 2016

We actually already have something like --trivial-solver in Cabal, it's called --explicit-configuration. Perhaps we only need to port this feature to cabal-install?

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Jul 1, 2016

@grayjay I'm not too worried about things like --independent-goals since we're not going to turn those on by default.

@kosmikus yes, the purpose is the same as the current cabal freeze, to give you a starting point but that you can manually tweak later.

@kosmikus

This comment has been minimized.

Copy link
Contributor

kosmikus commented Jul 1, 2016

@dcoutts Ok, I get it. The only thing that worries me is that I still don't consider the whole setup dependency / qualified goals stuff to be extremely stable. If we allow constraint syntax for these kinds of constraints, we may expose rather fragile solver internals to the end user.

So, syntax proposals welcome.

@ezyang

This comment has been minimized.

Copy link
Contributor

ezyang commented Sep 2, 2016

I need this feature for other reasons; specifically, when I build cabal-install's test suite from source with cabal-install HEAD, I accidentally pick the in-tree Cabal library to build the custom setup for happy with (which causes it to be built inplace). It would be far better if I could specify a constraint that the custom setup should NOT use the inplace Cabal. Syntax here (along with a way to distinguish between local and remote) would go a long way.

@ezyang

This comment has been minimized.

Copy link
Contributor

ezyang commented Oct 26, 2016

In #3932 we wanted to add a constraint that all versions of Cabal which are brought in from setup dependencies must be >= 1.20. Since we don't have qualified constraints, the best we can do is just say that Cabal, ANYWHERE, must have >= 1.20. That's suboptimal; a more precise constraint would be better. Once we fix this, we can make the constraint in @dagit's patch more precise.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Oct 27, 2016

A constraint that applies to all packages' setup scripts doesn't correspond to a qualifier in the solver, but it probably wouldn't be much harder to implement.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Oct 27, 2016

@robjhen Are you still interested in working on this issue?

@ghost

This comment has been minimized.

Copy link

ghost commented Oct 27, 2016

@grayjay Yes I'd be interested but I've been trying with no success to get the latest Haskell Platform and hence the Cabal project to build on my current operating system, Debian 7.

I've uploaded the code from the hackathon: 2d9c460

To install Debian 8 and then complete this feature would likely take me about two months as I have to fit it into limited spare time. I guess that may be too long to wait? Let me know if either a) you would prefer that someone else who has a working build environment take over development or b) you don't mind it taking that amount of time. Sorry about this!

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Oct 28, 2016

No problem! I just wanted to make sure that we don't duplicate work. Thanks for working on it. I don't know how soon this feature will be needed, though. @dcoutts, @ezyang Do you know if this is needed for 2.0?

@ezyang ezyang added this to the Default nix-local-build (3.0) milestone Oct 28, 2016

@ezyang

This comment has been minimized.

Copy link
Contributor

ezyang commented Oct 28, 2016

I don't think it's a blocker for 2.0. Feels important though; I'd put it on the default list (indeed I just have.)

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Oct 28, 2016

Thanks. So two months would be soon enough?

@ezyang

This comment has been minimized.

Copy link
Contributor

ezyang commented Oct 28, 2016

Yes, if software development time estimates ever meant anything ;)

@ghost

This comment has been minimized.

Copy link

ghost commented Oct 30, 2016

Okay, great! I'll go ahead with the work, and get back to you.

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 5, 2017

Just to let you know I'm close to finishing this. I'm currently splitting my changes up into separate commits for ease of review and I should have a pull request ready by tomorrow.

@ezyang

This comment has been minimized.

Copy link
Contributor

ezyang commented Jan 5, 2017

Awesome, looking forward to it.

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 6, 2017

I've made pull request #4211 - this is some refactoring prior to my main submission.

ghost pushed a commit that referenced this issue Jan 7, 2017

Rob Henderson
Merge pull request #4211 from robjhen/issue-3502
Preliminary refactoring in preparation for issue #3502
@ghost

This comment has been minimized.

Copy link

ghost commented Jan 8, 2017

Okay, this is ready: pull request #4219. @grayjay, @ezyang Would be great to know your opinion on the code.

ghost pushed a commit that referenced this issue Jan 9, 2017

Rob Henderson
Merge pull request #4219 from robjhen/issue-3502-part2
Qualified constraints (issue #3502)

ghost pushed a commit that referenced this issue Jan 13, 2017

Rob Henderson
Merge pull request #4228 from robjhen/issue-3502-part3
Qualified constraints (issue #3502) part 2
@BardurArantsson

This comment has been minimized.

Copy link
Collaborator

BardurArantsson commented Jan 15, 2017

Can this be closed, seeing as #4219 was merged?

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 15, 2017

See PR #4236

@BardurArantsson

This comment has been minimized.

Copy link
Collaborator

BardurArantsson commented Jan 15, 2017

I see what you did there :). I'm probably not qualified, but I'll see if I get time to review that PR tomorrow.

If that concludes the changes you should probably just add a "Fixes: #4236" to one of the commit messages. That'll auto-close this when it gets merged :).

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 15, 2017

@BardurArantsson I have another PR pending review (see previous comment), and then I believe @grayjay will want to upgrade the solver to act on the the new qualified constraints before we close this issue.

@BardurArantsson

This comment has been minimized.

Copy link
Collaborator

BardurArantsson commented Jan 15, 2017

Oh, ok, fair enough. :)

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 15, 2017

Sorry, didn't refresh my browser window there so I missed your last comment before making mine :) Anyway, I would check with @grayjay before closing the issue as she may or may not want to leave it open.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Jan 16, 2017

Yes, this issue isn't finished, because the solver doesn't use the constraint qualifiers yet. Another thing that needs to be done is printing the qualified constraints in the freeze file.

ghost pushed a commit that referenced this issue Jan 17, 2017

Rob Henderson
Merge pull request #4236 from robjhen/issue-3502-part4
Qualified constraints: documentation and unit tests (issue #3502)
@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Feb 26, 2017

cabal now has qualified constraints that apply to setup dependencies, top-level dependencies, and any dependency. We'll probably need to implement per-component dependency solving before we add qualified constraints for build tools.

Related pull requests: #4211, #4219, #4228, #4236, #4252, #4257, #4342

We should be able to add setup qualified constraints to freeze files now. We can continue to constrain build tool dependencies with unqualified constraints that allow multiple versions.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Dec 16, 2017

Should this ticket also cover Hackage revision constraints, or should there be a different one? There's also the idea of allowing index-state in freeze files.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Dec 17, 2017

I think both of those features could be implemented separately from qualified constraints, so they should probably be separate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment