Skip to content
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

Better fix for #19 #20

Closed
hvr opened this issue Aug 21, 2015 · 3 comments
Closed

Better fix for #19 #20

hvr opened this issue Aug 21, 2015 · 3 comments

Comments

@hvr
Copy link
Member

hvr commented Aug 21, 2015

I noticed #19, and currently fgl.cabal contains

    build-depends:    base < 5
                    , transformers
                    , containers
                    , array

    if impl(ghc >= 7.4)
       build-depends:
            deepseq >= 1.1.0.0 && < 1.5.0.0

However this underspecified, as in the ghc>=7.4 you clearly require also containers >= 0.4.2, so a better fix would be e.g

    build-depends:    base < 5
                    , transformers
                    , containers >= 0.4.2
                    , array
                    , deepseq >= 1.1.0.0 && < 1.5

or alternatively (but this leaves us with an API that conditionally provides the NFData feature depending on the containers version):

flag containers042
   manual: false
   default: true

library
-- ....
    build-depends:    base < 5
                    , transformers
                    , containers
                    , array

    if flag(containers042)
       build-depends:
            containers >= 0.4.2
            deepseq >= 1.1.0.0 && < 1.5
    else
       build-depends: containers < 0.4.2

PS: it's unsound to write an upper bound with trailing zeroes like <1.5.0.0 since 1.5 < 1.5.0 < 1.5.0.0

@ivan-m
Copy link
Contributor

ivan-m commented Aug 22, 2015

You added a minimum bound to transformers in your second code block; was that meant to be for containers instead?

I'm not sure I like the flag approach (though I can see now why the version of containers should depend upon whether deepseq is being used or not; I just blindly went by what GHC has rather than being compiler-agnostic).

I suppose the upper bound (if any) on containers would then go in the main build-depends block (or should it be taken out of there and only mentioned within the flag bit)?

@hvr
Copy link
Member Author

hvr commented Aug 22, 2015

You added a minimum bound to transformers in your second code block; was that meant to be for containers instead?

Yes, sorry... I've fixed the example

I'm not sure I like the flag approach (though I can see now why the version of containers should depend upon whether deepseq is being used or not; I just blindly went by what GHC has rather than being compiler-agnostic).

Just because GHC ships with a specific containers version doesn't guarantee cabal will pick that version for you (the solver has a soft preference for installed packages though). In fact, here's a compatibilty matrix:

containers

I suppose the upper bound (if any) on containers would then go in the main build-depends block (or should it be taken out of there and only mentioned within the flag bit)?

I'd put a global upper bound in the main build-depends as that's less error-prone
(and btw, the build-depends property is a conjunctive Monoid)

ivan-m added a commit that referenced this issue Sep 8, 2015
@ivan-m ivan-m closed this as completed in 3b23dcb Sep 8, 2015
ivan-m added a commit that referenced this issue Sep 8, 2015
ivan-m added a commit that referenced this issue Sep 8, 2015
ivan-m added a commit that referenced this issue Sep 8, 2015
@ivan-m
Copy link
Contributor

ivan-m commented Sep 8, 2015

Whilst this works, it results in older GHC's having newer containers built on them on Travis-CI; making the default False results in containers being downgraded on 7.4.2.

Any idea how this can be resolved?

psteiger pushed a commit to psteiger/fgl that referenced this issue Nov 8, 2015
psteiger pushed a commit to psteiger/fgl that referenced this issue Nov 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants