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

Encourage top-level build-depends when it occurs in all branches of a conditional #7768

Open
andreasabel opened this issue Oct 23, 2021 · 2 comments
Labels
cabal-install: cmd/check re: conditional About conditional declarations in cabal files(`if`) type: enhancement

Comments

@andreasabel
Copy link
Member

andreasabel commented Oct 23, 2021

See e.g. jvranish/Lenses#3.
Currently hackage does not allow revision of conditionals, and it does not allow new build-depends.
In the situation (1)

build-depends: base
  , A
if cond
  build-depends: B >= 0.1 && < 0.4
else
  build-depends: B >= 0.2 && < 0.5

the constraints for B cannot be revised.
Thus, the better practice is to put a top-level B with extra constraints in the conditional (2):

build-depends: base
  , A
  , B >= 0.1 && < 0.5
if cond
  build-depends: B < 0.4
else
  build-depends: B >= 0.2

If cabal check would suggest a adding a top-level build-depends that appears in all branches, that would help with revisions on hackage.

Of course, this could also be fixed on the hackage side; in case it is fixed there by rejecting packages that could be rewritten from (1) to (2), cabal check should also warn about possible complaints by the hackage server.

@phadej
Copy link
Collaborator

phadej commented Oct 23, 2021

constraints for B definitely can be revised. See eg. https://hackage.haskell.org/package/aeson-0.8.1.1/revisions/ which revised

  if flag(old-locale)
    build-depends: time < 1.5, old-locale
  else
    build-depends: time >= 1.5

to

  if flag(old-locale)
    build-depends: time >= 1.1.3 && < 1.5, old-locale
  else
    build-depends: time >= 1.5

I assume you use hackage-cli add-bound feature, which isn't smart enough to deal with these.


EDIT: doesn't seem to be a problem with lenses either (that's a preview of a possible revision):

Screenshot from 2021-10-23 15-47-27

@andreasabel
Copy link
Member Author

Thanks for the swift feedback, @phadej!

I assume you use hackage-cli add-bound feature, which isn't smart enough to deal with these.

Mmh, weird, I cannot remember how I came to the conviction that conditional bounds cannot be revised.
Maybe I mixed this up with the fact that you cannot "add" a top-level dependency even if it is already mentioned in conditionals. I don't think I used hackage-cli add-bound ...

In the case of lenses, if there was a top-level build-depends: template-haskell, it could be equipped with < 2.17 rather than doing it in all of the branches. So maybe there is still some value to my suggestion, but certainly less need. It could be a warning or style suggestion (linting).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cabal-install: cmd/check re: conditional About conditional declarations in cabal files(`if`) type: enhancement
Projects
None yet
Development

No branches or pull requests

3 participants