Solver DSL improvements #4028
Solver DSL improvements #4028
Conversation
This commit modifies the tests so that they disable executables instead of libraries. Disabling executables with 'Buildable: False' is a more realistic test case.
Previously, the solver DSL ignored some types of dependencies when they appeared in executables or under flags. This commit uses one function, mkBuildInfoTree, to create all BuildInfos, so that any dependency can be used in any location.
…lver DSL. The checks might detect some errors in the DSL. The commit also adds non-default values to some fields to make the checks pass, such as adding a default-language to each component.
|
@grayjay, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edsko, @ezyang and @jdnavarro to be potential reviewers. |
| pkgCheckErrors = | ||
| let ignore = ["Unknown extensions:", "Unknown languages:"] | ||
| in [ err | err <- C.checkPackage (packageDescription package) Nothing | ||
| , not $ any (`isPrefixOf` C.explanation err) ignore ] |
There was a problem hiding this comment.
I filtered out these two warnings because some unit tests test that the solver allows unknown extensions/languages when the compiler supports them. I don't know whether the unit tests or package checks are correct.
There was a problem hiding this comment.
Cabal hardcodes a list of known extensions and warns if it sees something it doesn't understand.
I think the reason we hardcode is because some extensions require special handling: e.g., TemplateHaskell. But the vast majority of extensions don't get any special handling. This raises an interesting question: for an extension we've never seen before, should we assume that it does or does not need special handling? If the former, the warning is appropriate; if the latter, we really ought to only define extensions which we do handle specially. But maybe this is too much for a simple PR like this.
There was a problem hiding this comment.
I don't know. If most extensions don't need special handling, and we can get their names directly from the compiler, then it seems like it would be simpler to not define all of them.
I'll just add a comment to the line above for now.
|
I haven't looked too closely, but any refactoring of the solver DSL is fine by me. But the quickchecks are failing! (Let me know if there is anything in particular you'd like me to look at.) |
grayjay
force-pushed
the
buildable-solver-tests-2
branch
from
0055173
to
e52e16e
Compare
|
Go for it :) |
grayjay
force-pushed
the
buildable-solver-tests-2
branch
from
71841c6
to
e5bf467
Compare
grayjay
force-pushed
the
buildable-solver-tests-2
branch
from
e5bf467
to
cb6603a
Compare
|
Thanks! I'll merge it now. |
Three commits:
Update solver unit tests for Buildable field.
This commit modifies the tests so that they disable executables instead of
libraries. Disabling executables with
Buildable: Falseis a more realistictest case.
See #3988.
Refactor 'UnitTests.Distribution.Solver.Modular.DSL.exAvSrcPkg'.
Previously, the solver DSL ignored some types of dependencies when they appeared
in executables or under flags. This commit uses one function, mkBuildInfoTree,
to create all BuildInfos, so that any dependency can be used in any location.
Run package checks on the GenericPackageDescription created by the solver DSL.
The checks might detect some errors in the DSL. The commit also adds non-default
values to some fields to make the checks pass, such as adding a default-language
to each component.
Related to #3784
/cc @ezyang
I don't know if you need the first commit for #3988, since #4023 was merged, but I still think it's an improvement.