defer build-tools-depends choices as well as setup choices #7532
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
whoops, accidentally pushed directly to master. i'm confident enough that i'm not going to revert, but just link the patch for posterity. sorry about bypassing the process. |
|
Wow, congratulations. This is 80% of the real world case, not 80% of the reproduction example for the issue? |
|
Amazing! |
|
Really excited for this. Thanks so much! |
|
@Mergifyio backport 3.6 |
|
Command
|
mergify bot
added a commit
that referenced
this pull request
Aug 13, 2021
…7532) (#7538) * defer build-tools-depends choices as well as setup choices (cherry picked from commit 3ac512e) * Update Preference.hs * Update Solver.hs Co-authored-by: Gershom Bazerman <gershom@arista.com> Co-authored-by: gbaz <gershomb@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This was referenced Aug 26, 2021
andreasabel
added
cabal-install: solver
re: build-tool
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is effectively a one line change, modulo renaming, that seems to fully resolve #7472 giving a speedup of roughly 80% on the tricky case detailed there (cardano-node). Arguably this was the single key change to resolve the issue all along (although the other fixes resulted in improvements along the way anyway). All it does is extend the existing solver pass that defers solving setup depends until top-level goals are solved to also defer build-tool-depends goals until top level goals are solved. This means that all the constraints at the top level (on versions, flags, stanzas, etc) are applied first and only then are attempts made to build the less-constrained qualified goals, significantly streamlining the production of valid build plans.
This change is in line with how setup depends are already constrained, so its simply extending an already tested and correct strategy to another instance of the same problem. Also, since it just reorders goals, it doesn't change the solution space. As such, hopefully it can be reviewed and merged quickly.