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 complains about TypeAbstractions #9496

Closed
LeventErkok opened this issue Dec 6, 2023 · 15 comments
Closed

Cabal complains about TypeAbstractions #9496

LeventErkok opened this issue Dec 6, 2023 · 15 comments

Comments

@LeventErkok
Copy link

As I understand it GHC 9.8.1 should work well with cabal 3.10.2.1, and indeed I've those versions:

$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 9.8.1
$ cabal --version
cabal-install version 3.10.2.1
compiled using version 3.10.2.0 of the Cabal library

But I'm getting:

$ cabal check
Warning: The following warnings are likely to affect your build negatively:
Warning: Unknown extensions: TypeAbstractions, TypeAbstractions,
TypeAbstractions, TypeAbstractions, TypeAbstractions, TypeAbstractions
...

The cabal file under question is: https://github.com/LeventErkok/sbv/blob/master/sbv.cabal

TypeAbstractions is in a conditional stanza (https://github.com/LeventErkok/sbv/blob/48f39ce0666b976ebba559cc27a55a3c41d2fcc0/sbv.cabal#L79-L81), but I don't think that matters. Cabal still complains the same if I move it out of the conditional part.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 6, 2023

I thought TypeAbstractions will only land in GHC 9.10, but indeed it's already there in 9.8.1: https://downloads.haskell.org/ghc/latest/docs/users_guide/exts/type_abstractions.html?highlight=typeabstractions#type-abstractions

@bgamari, @Kleidukos, we missed that in #9203. Do we want to fix the omission in cabal 3.10.3.0? Are there any additional new extensions? I think somebody told me there are none, but nobody took responsibility for that by ticking the box in the ticket.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 7, 2023

Let me mark it as 3.10 backport so that we don't forget to decide.

@andreabedini
Copy link
Collaborator

Unfortunately adding a known extension (represented by a type constructor) is a breaking change and cannot be backported to 3.10.

@ffaf1
Copy link
Collaborator

ffaf1 commented Dec 8, 2023

Depending on how important this is re: blessed-by-ghcup, this can be backported to 3.10, without adding type constructors, just modifying the logic of check.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 8, 2023

Andrea, you are right we can't just add a new type constructor. This is a shame and one more reason to review why we missed the extension. IMHO, this is worth a hack that Francesco suggests. GHC warns to add that extension, so more and more people are going to have it, so the warning at each cabal build, repeated for each component, is going to be annoying.

@andreabedini
Copy link
Collaborator

Depending on how important this is re: blessed-by-ghcup, this can be backported to 3.10, without adding type constructors, just modifying the logic of check.

Right. W.r.t to ghcup, ghcup only ships the executable binary so there's no issues of compatibility there. But we need to be careful not to add a breaking change to the 3.10 branch, otherwise we risk forgetting about it and mixing it up with other backports.

@Mikolaj yeah, unfortunately it is. From https://pvp.haskell.org/

Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes or classes were changed, or orphan instances were added or any instances were removed, then the new A.B MUST be greater than the previous A.B. Note that modifying imports or depending on a newer version of another package may cause extra orphan instances to be exported and thus force a major version change.

image

The reason is simple, if a dependent package case splits on the data type, it will either fail to build or worse get an exception at runtime.

I don't have any issue with modifying how cabal check works.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 8, 2023

Right. I think @ffaf1 might have meant that if the "recommended" status bestowed by ghcup, which we so eagerly seek, depends on handling the extension, that's one more argument to make the extra effort and hack this in 3.10.

ffaf1 added a commit to ffaf1/cabal that referenced this issue Dec 8, 2023
See haskell#9496.

“Backport” of haskell#9502, notice that we are not adding a new constructor,
just tweaking `check` behaviour.  This is improve UX without breaking
changes.
ffaf1 added a commit to ffaf1/cabal that referenced this issue Dec 8, 2023
See haskell#9496.

“Backport” of haskell#9502, notice that we are not adding a new constructor,
just tweaking `check` behaviour.  This is improve UX without breaking
changes.
@ffaf1
Copy link
Collaborator

ffaf1 commented Dec 8, 2023

Why did we miss TypeAbstractions?

  • Making a release has a tick for that:

    Update Language.Haskell.Extension list, if there are new GHC extensions

  • When we check Release notes for GHC 9.8.1, first line starts with

    There is a new extension ExtendedLiterals

    So, a new extension. There is a mention of TypeAbstractions, but it is not immediately clear that it is an extension

    This feature is guarded behind TypeAbstractions.

    so most likely an unfortunate and understandable oversight.

In the future, if we want to automatically test this, ghc --supported-languages could be useful:

f@x270:~/media/vcs/ghc/9.8.1/bin$ ./ghc --supported-languages | grep TypeAbstractions
TypeAbstractions
NoTypeAbstractions

(but note that cabal knows more GHC extensions that GHC itself does).

@Mikolaj
Copy link
Member

Mikolaj commented Dec 8, 2023

Re 3.10 postmortem: and how about the GHC test https://gitlab.haskell.org/ghc/ghc/-/blob/master/testsuite/tests/driver/T4437.hs? How come it did not detect the missing extension? Is it not run by GHC CI? @bgamari, @mpickering?

@Mikolaj
Copy link
Member

Mikolaj commented Dec 8, 2023

Wow, TypeAbstractions is marked there as expectedGhcOnlyExtensions! Is cabal not supposed to support it? What a mess!

@philderbeast
Copy link
Collaborator

philderbeast commented Dec 8, 2023

Something I was looking into for #9435 might help:

-- | Options added in GHC-8.8, to generate:
--
-- @
-- ghc-8.6.5 --show-options | sort > 8.6.5.txt
-- ghc-8.8.1 --show-options | sort > 8.8.1.txt
-- diff -u 8.6.5 8.8.1
-- @
--
-- - remove -W(no-)error=, -W(no-)warn flags.
-- - split into all and flags which may affect artifacts

Here's a sequence of commands that shows -XTypeAbstractions as added:

$ ghc-9.6.3 --show-options | sort > 9.6.3.txt
$ ghc-9.8.1 --show-options | sort > 9.8.1.txt
$ diff -u 9.6.3.txt 9.8.1.txt | grep -E "^\+-X" | sed -E 's/\+(.*)$/, "\1"/'
, "-XExtendedLiterals"
, "-XNoExtendedLiterals"
, "-XNoTypeAbstractions"
, "-XTypeAbstractions"

Are option changes only ever shipped with the first of a GHC series, in the ghc-x.y.1 version?

@mpickering
Copy link
Collaborator

@philderbeast There is precedent for shipping extensions with later versions of GHC in exceptional circumstances (see DeepSubsumption).

@int-index
Copy link
Collaborator

int-index commented Dec 8, 2023

Wow, TypeAbstractions is marked there as expectedGhcOnlyExtensions! Is cabal not supposed to support it? What a mess!

I added it there because it's the first step of the documented process

https://gitlab.haskell.org/ghc/ghc/-/blob/ca7510e4477fc37749a79dd6f77019684abbf140/compiler/GHC/Driver/Session.hs#L340-352

--    The recommended workflow is,
--
--     1. Temporarily add your new language extension to the
--        expectedGhcOnlyExtensions list in T4437 to ensure the test doesn't
--        break while Cabal is updated.
--
--     2. After your GHC change is accepted, submit a Cabal pull request adding
--        your new extension to Cabal's list (found in
--        Cabal/Language/Haskell/Extension.hs).
--
--     3. After your Cabal change is accepted, let the GHC developers know so
--        they can update the Cabal submodule and remove the extensions from
--        expectedGhcOnlyExtensions.

The problem is that I somehow forgot to do the other two steps.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 9, 2023

@int-index: thank you very much for the clarification. It seems we missed to double-check from the cabal side, as well. I've now added to our release checklist a hint to use ghc --supported-languages to double check cabal code vs GHC output. It should be now more obvious what to do for the 3.12.1 release.

Also, it seems the communication with GHC devs that grace our cabal dev meetings failed in this respect --- I'm being told we got distracted by long term automation and decoupling plans discussion and never got back to, e.g., cross-checking #9203 and a similar integration ticket on the GHC side, if there was any this time (there was for previous GHC releases).

@bgamari says "I think all we can do is to potentially make the T4437 fail during release builds" --- that sounds great. Let's not close this here cabal ticket before it's resolved.

All in all, the postmortem up to this point indicates it's not a major process or communication break-down, but just an unfortunate combination of little problems. Thank you all for keeping at it and let's also work on the long term automation/decoupling solutions!

@ffaf1
Copy link
Collaborator

ffaf1 commented Jan 10, 2024

This is done and backported.

@ffaf1 ffaf1 closed this as completed Jan 10, 2024
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