-
Notifications
You must be signed in to change notification settings - Fork 696
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] default-package-bounds
#9569
Comments
What is wrong with common stanza exactly? Do we really need another feature doing the same thing? |
I find it quite confusing if the dependencies are declared in common stanzas. Also in that way of doing things, the naive action of adding another dependency to build-depends will result in a "common bound" not being used (as one should have used a common stanza instead), but with the default-package-bounds the naive action will imply the "common bound" already. It can be seen as as wrong-by-naive vs right-by-naive decision. For example tools that add new dependencies from the cli (I think it was named |
Cabal is a spec. I find that we are increasingly mudding the boundaries between good spec and good user interface. Common stanzas are the pattern for code reuse. If we're thinking purely in terms of user experience, we have to try harder first:
I think adding more ad-hoc features is going to degrade the spec. |
I would not have guessed that default-package-bounds
build-depends:
text > 5
library d
build-depends: text >4.5 is equivalent to library d
build-depends: text >4.5 and not to library d
build-depends: text >4.5 && >5 Cabal usually treats
There is already an error when a build plan for test executable differs from a build plant for a library. You need to override it manually (with |
@Bodigrim said:
I don't think it is unreasonable to have for example two benchmarks, one with some version of say aeson, and a different benchmark with some other version. In general I think there is freedom in using different bounds for different components. Although you say that "there is already an error if the build plan differs from the build plan of the library", cabal does not prevent such an error in any way. This feature (default-package-bounds) would be unnecessary if for example Cabal warned the user if bounds changed from enabling tests or not, but that is not the case. |
That is a very valid concern and the matter is perfectly up for debate. I could change the implementation to work this way and I also don't think it is unreasonable. |
if |
That is not my understanding. Cabal says the following:
But if I provide I recreated de above with |
That's just a lousy error message: |
The error message is not very clear, but the key point is "force the solver to include the test suites for all packages" = "use package bounds from test suites even when building the main library". See #5079 (comment) and #7883 (comment) |
I think the nature of my issue is slightly different to those you are linking. My concern is that there is no warning that enabling the tests changes versions of the dependencies in the build plan. If I upgrade my library to use a newer dependency but not my test-suite, I will always test the older version when I enable the tests. Yes, there is a warning that the build plan doesn't include the tests and that you have to explicitly enable them, but that doesn't warn you of the scenario above. In my example on the other comment I might think "ah my library supports aeson 2.2.3.0" and then a user comes and says "but it fails for me with aeson 2.2.3.0!" and my immediate thought is "that's weird, all my tests pass", but inadvertently my tests were forcing me to use 2.2.0.0. There is no current way to prevent this. Default-package-bounds makes this an opt-in. Unless you specify a bound manually, the same bound as the library will be picked if you enable the tests, always. |
I agree that However, I wonder whether it is worth adding and maintaining a new feature for that bit of extra convenience. If one wants more convenience, one could also use a wizard like |
Obsoletes #9477
Problem this solves
When describing a package in a
.cabal
file, multiple components can declare the same dependency in theirbuild-depends
sections. There can be two flavors of this:aeson
in one test-suite and a different version in another, in order to test backwards compatibility always.It is to case (2) that we turn our attention in this RFC. On the one hand it feels redundant and error-prone to repeat the version bound in all components. On the other hand, in order to make sure you are building with the same version regardless of what subset of those components is enabled one has to perform a topological-sort of the components and assign the version bound in the bottom-most component.
A different solution is to use
common
stanzas as done by some packages but this leads to a weird syntax, where theimport
section of a component is importing both configurations andbuild-depends
sections.Proposed solution
Implement a new section in the
.cabal
package file nameddefault-package-bounds
which contains a list of dependencies in the same format as abuild-depends
.For each of the unqualified dependencies in the
default-package-bounds
, whenever a dependency on the same package appears in abuild-depends
section of a component without a specified version bound, the version bound fromdefault-package-bounds
will be used. For each qualified dependencies in thedefault-package-bounds
, whenever a dependency on the same package appears in abuild-tool-depends
section of a component without a specified version bound, the version bound fromdefault-package-bounds
will be used.In particular
default-package-bounds: foo >2
will be applied to bothbuild-depends: foo, foo:bar
, anddefault-package-bounds: foo:baz >2
will be applied tobuild-tool-depends: foo:baz
.For specific cases like (1) above, the user should not include such a special dependency in these
default-package-bounds
section.This will be just a syntactic sugar and will have no influence in:
constraints
incabal.project
)pkgconfig-depends
Backwards compatibility
This change would be fully backwards-compatible as there existed no field with this name before and omitting it would work the same way as before this change, i.e. it is an optional field.
The text was updated successfully, but these errors were encountered: