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: Reexport all of Cabal-syntax #8064

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Conversation

bgamari
Copy link
Contributor

@bgamari bgamari commented Mar 25, 2022

To preserve compatibility for downstream users Cabal will reexport all
of Cabal-syntax for the time being. In the future we may deprecate these
reexports.

This requires that we bump the cabal-version of Cabal.cabal to 1.22 and
drops support for GHC < 7.10.

Closes #7974.


Please include the following checklist in your PR:

Please also shortly describe how you tested your change. Bonus points for added tests!

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@jneira
Copy link
Member

jneira commented Mar 25, 2022

Minor? caveat, reexported-modules, like other backpack related features are not well supported by stack repl. That in turn supposes you could not load the package en hls using stack.
I tried to use them in the hlint plugin of hls: haskell/haskell-language-server#166 (comment)
Upstream issue still unresolved (and i guess it wont) in stack: commercialhaskell/stack#5077 (comment)
And had to fall back to PackageImports instead.

Not sure if someone would want use stack repl or load the project in hls using stack, though.

@jneira
Copy link
Member

jneira commented Mar 25, 2022

Reviewing other prs in hls about it seems there was a fix in hls itself about: haskell/haskell-language-server#2468 and maybe it would work. I had to test it to confirm. But still stack repl would not work.

@jneira
Copy link
Member

jneira commented Mar 25, 2022

We are testing Cabal the lib for several ghcs < 7.10.3 and the tests did pass (https://github.com/haskell/cabal/runs/5687246403?check_suite_focus=true). And afaiu we are using cabal head at runtime for tests 🤔

@jneira jneira mentioned this pull request Mar 28, 2022
@jneira
Copy link
Member

jneira commented Mar 28, 2022

Ok, at build time cabal build Cabal -w D:\ghcup\ghc\7.8.4\bin\ghc --disable-tests --disable-benchmarks --project-file cabal.project.libonly fails with:

Warning: Cabal.cabal:1:22: Packages with 'cabal-version: 1.12' or later should specify a specific version of the Cabal spec of the form 'cabal-version: x.y'.
Use 'cabal-version: 1.22'.
Configuring library for Cabal-3.7.0.0..
Error: cabal: Your compiler does not support module re-exports. To use this
feature you must use GHC 7.9 or later.

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, reexportsi s the cleaner way and i hope the stack repl issue will not impact most cabal developer, thanks!
As we are gonna drop support for build Cabal < 8.0 and this only restricts it to < 7.10.3 i think it would be fine.
But the changelog should note it (and it should be noted in the release in a more relevant way if the next version does not drop support for ghc < 8.0, but it would be in another pr)

@Mikolaj
Copy link
Member

Mikolaj commented Apr 7, 2022

We did drop the support finally, right? #8079

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mikolaj thanks for the reminder, yeah, this is not reducing support over master anymore

@Mikolaj Mikolaj added the merge me Tell Mergify Bot to merge label Apr 8, 2022
To preserve compatibility for downstream users Cabal will reexport all
of Cabal-syntax for the time being. In the future we may deprecate these
reexports.

This requires that we bump the cabal-version of Cabal.cabal to 1.22 and
drops support for GHC < 7.10.

Closes haskell#7974.
@mergify mergify bot merged commit 9ca9891 into haskell:master Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cabal should perhaps reexport all the modules from Cabal-syntax to reduce breakage
3 participants