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

Improve algorithm for choosing flags with './Setup configure' #3082

Merged
merged 1 commit into from Jan 28, 2016

Conversation

Projects
None yet
2 participants
@grayjay
Copy link
Collaborator

grayjay commented Jan 27, 2016

Cabal previously tried all flag combinations, which was very slow. The new
algorithm assigns one flag at a time, and backtracks when a flag introduces a
dependency that is unavailable.

The new algorithm handles the Buildable field by adding an extra conditional at
the top level of each component that represents the condition for which the
component is buildable. Since all dependencies go under the "then" branch, they
are only required when a flag choice makes the component buildable. The buildable
logic is taken from the cabal-install dependency solver.

This commit also changes the error message that is shown when dependencies are
missing. Previously, Cabal printed the shortest list of missing dependencies
from a single flag assignment. Now it takes the union of all dependencies that
caused it to backtrack when trying different combinations of flags, which
requires less searching.

I updated #2925 to allow backtracking to work with the Buildable field. I moved the extractCondition function (added in #2731) from the modular solver to Distribution.PackageDescription.Configuration so that it can be used by Cabal and cabal-install. Since extractCondition is not a very general utility function, I was wondering if there is a better place to put it.

Improve algorithm for choosing flags with './Setup configure'
Cabal previously tried all flag combinations, which was very slow.  The new
algorithm assigns one flag at a time, and backtracks when a flag introduces a
dependency that is unavailable.

The new algorithm handles the Buildable field by adding an extra conditional at
the top level of each component that represents the condition for which the
component is buildable.  Since all dependencies go under the "then" branch, they
are only required when a flag choice makes the component buildable.  The buildable
logic is taken from the cabal-install dependency solver.

This commit also changes the error message that is shown when dependencies are
missing.  Previously, Cabal printed the shortest list of missing dependencies
from a single flag assignment.  Now it takes the union of all dependencies that
caused it to backtrack when trying different combinations of flags, which
requires less searching.
@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jan 28, 2016

Just checked that the containers test suite still works with this change.

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Jan 28, 2016

@23Skidoo What is different in the containers test suite?

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jan 28, 2016

The containers test suite depends on containers, so the cabal-install solver fails to find a solution when you run configure --enable-tests because of circular dependencies. The ./Setup configure solver works in this case, so it's important to make sure that it continues to work. See #1806, #1575, #935, #960 for details.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jan 28, 2016

LGTM.

23Skidoo added a commit that referenced this pull request Jan 28, 2016

Merge pull request #3082 from grayjay/setup-configure-backtracking
Improve algorithm for choosing flags with './Setup configure'

@23Skidoo 23Skidoo merged commit 1a01c92 into haskell:master Jan 28, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jan 28, 2016

Merged, thanks!

@grayjay

This comment has been minimized.

Copy link
Collaborator

grayjay commented Jan 28, 2016

@23Skidoo Thanks. I just realized that if I export addBuildableCondition instead of extractCondition from Distribution.PackageDescription.Configuration, I can reuse more code between cabal-install and Cabal. I'll make another PR. Sorry I didn't think of it sooner!

@grayjay grayjay deleted the grayjay:setup-configure-backtracking branch Jan 28, 2016

grayjay added a commit to grayjay/cabal that referenced this pull request Jan 29, 2016

grayjay added a commit to grayjay/cabal that referenced this pull request Jan 29, 2016

grayjay added a commit to grayjay/cabal that referenced this pull request Jan 29, 2016

grayjay added a commit that referenced this pull request Jan 29, 2016

Merge pull request #3091 from grayjay/refactor-3082
Refactor #3082 to reduce code duplication and improve Haddock comment.

garetxe added a commit to garetxe/cabal that referenced this pull request Mar 5, 2016

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