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

'cabal check' should warn when there are no upper bounds in setup-depends #4683

Closed
23Skidoo opened this issue Aug 12, 2017 · 9 comments
Closed

Comments

@23Skidoo
Copy link
Member

23Skidoo commented Aug 12, 2017

wxc has the following custom-setup section:

custom-setup
  setup-depends: 
    base, 
    Cabal,
    bytestring,
    split,
    process,
    directory,
    filepath

That is, it says that its setup.hs works with any version of Cabal. However, it is lying - it actually fails to build against Cabal 2. We should warn about such obviously incorrect setup-depends and maybe even auto-rewrite the constraint on Cabal to < 2.

@23Skidoo 23Skidoo changed the title cabal check should check that custom-setup 'cabal check' should warn when there are no upper bounds in setup-depends Aug 12, 2017
@23Skidoo
Copy link
Member Author

@23Skidoo 23Skidoo added this to the 2.0.1 milestone Aug 12, 2017
@hvr
Copy link
Member

hvr commented Aug 12, 2017

As discussed on IRC, it'd also make sense to validate that the lower bound on lib:Cabal is consistent with the spec-version specified in the .cabal file.

@grayjay
Copy link
Collaborator

grayjay commented Aug 12, 2017

As discussed on IRC, it'd also make sense to validate that the lower bound on lib:Cabal is consistent with the spec-version specified in the .cabal file.

#3751 is related to this.

@phadej
Copy link
Collaborator

phadej commented Aug 16, 2017

If you review&accept my #4701, then I'd like to tackle this as a follow-up ;)

(yet, if it's for 2.0.1, then situation is tricker :( )

@phadej
Copy link
Collaborator

phadej commented Aug 17, 2017

Note:

      +++ custom-setup
      +++     setup-depends: base -any
      +++                    Cabal -any
      +++                    cabal-doctest >=1.0.2 && <1.1

here I know that omitting upper bounds on Cabal is safe. Cabal dependency is needed because of #4288

@qnikst
Copy link
Collaborator

qnikst commented Jun 8, 2018

I'm going to work on this

mergify bot pushed a commit that referenced this issue Aug 26, 2021
* Introduce checks for upper dependencies in setup.

setup depends are now checked for having upper dependencies.
Upper bounds are mandatory for base and Cabal libraries, and
are optional (but emit warning) for other libraries.
Implicit dependencies are not being checked.

* Add tests.

* Fix tests.

* add changelog

* fix test for the newer cabal_raw'

* scale down alerts: no warnings on any package, only errors on base+Cabal

* address review comments

* typo

* improve changelog (add Cabal)

* remove boundedAbove and use Distribution.Version.hasUpperBound instead

Co-authored-by: Alexander Vershilov <alexander.vershilov@gmail.com>
Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
Co-authored-by: Mikolaj Konarski <mikolaj@well-typed.com>
@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Sep 6, 2021

Since #7409 has been merged, this probably can be closed. Cc @Mikolaj

@Mikolaj
Copy link
Member

Mikolaj commented Sep 6, 2021

Right. High time. :)

Thank you again, @ulysses4ever, @qnikst and everybody who took part in the struggle.

@Mikolaj Mikolaj closed this as completed Sep 6, 2021
@ulysses4ever
Copy link
Collaborator

Thanks a lot @Mikolaj for helping out with it!

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

No branches or pull requests

7 participants