Cabal-syntax-3.10.2.0 is going to violate PVP

[phadej]

It adds constructors to LicenseId and LicenseListVersion, Language.Haskell.Extension.Extension. These are breaking changes.

[fgaz]

Looks like #9028 targeted the wrong branch

cc @Kleidukos

edit: and this is the extensions backport 9faa4db

[Mikolaj]

#9028 is my fault --- I decided to add new licences in a point release to lessen the load on 3.10.1.0. I forgot it doesn't just add data, but creates new constructors. Let's just revert and it's going to be published in 3.12.1.0. No harm done.

Re 9faa4db, this is a serious issue, because it's needed to fully handle GHC 9.8, so I think we may even consider breaking PVP and saying so explicitly in release notes. Another option is to revert the commit and include a snapshot of master branch in GHC 9.8 instead, to be published (with some more commits) as cabal 3.12. Are there really any other options?

@Kleidukos, @bgamari, everybody what do we do?

Edit: here's a backport with the Language.Haskell.Extension.Extension changes, but it's not been used: #9194. I can't find the actual backport PR containing the commit that is included.

[andreabedini]

Re 9faa4db, this is a serious issue, because it's needed to fully handle GHC 9.8

Are you sure about this? What would be the consequences of not including ExtendedLiterals in KnownExtension?

[Mikolaj]

I'm not sure at all. :) We'd need to consider the POV of tool creators, users that read GHC release notes and want to enable the extension in their cabal file, GHC devs writing tests that perhaps do the same, etc. Actually, is the new constructor used anywhere except in .cabal files in the two fields about extensions?

[andreabedini]

FWIW

ghci> simpleParsec "NewShinyThing" :: Maybe Extension
Just (UnknownExtension "NewShinyThing")

so everything will still parse. Also from a first cursory look, it looks like the solver does not make a distinction between a Known- or Unknown- extensions for its purpose.

cabal/cabal-install-solver/src/Distribution/Solver/Modular/Validate.hs

Lines 578 to 580 in fa54e31

	supportedExt = maybe (const True) -- if compiler has no list of extensions, we assume everything is supported
	(\ es -> let s = S.fromList es in \ x -> S.member x s)
	(compilerInfoExtensions cinfo)

Where supportedExt :: Extension -> Bool and

data Extension
  = -- | Enable a known extension
    EnableExtension KnownExtension
  | -- | Disable a known extension
    DisableExtension KnownExtension
  | -- | An unknown extension, identified by the name of its @LANGUAGE@
    -- pragma.
    UnknownExtension String

@grayjay might be able to correct me.

To be clear, I don't mind if we end up doing a major bump.

PS: Now I am wondering whether it is even worth having that list of known extensions.

[Mikolaj]

[Edit: @phadej explains that's a ghc-related error, so my example is bogus, see below. I just re-checked by adding ExtendedLiterals to my .cabal file and old cabals warn about ExtendedLiterals, but go through fine when compiling with GHC 9.8. I have no idea how that works. Is GHC being asked whether it supports an extension? What's the protocol? If so, keeping our own list really seems redundant, for the minor benefit of Hackage warnings.]

[Edit2: @phadej kindly offers a new bit of info: "Also e.g. uhcLanguageExtensions uses the list, as there isn't uhc --supported-extensions."]

I've added extension StrictDataZ to the default-extensions list in my .cabal file and I'm getting:

~/r/horde-ad$ cabal build
Resolving dependencies...
Error: cabal: Could not resolve dependencies:
[__0] next goal: horde-ad (user goal)
[__0] rejecting: horde-ad-0.1.0.0 (conflict: requires unknown extension
StrictDataZ; did you mean StrictData?)
[__0] fail (backjumping, conflict set: horde-ad)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: horde-ad

[phadej]

The

 cabalSpecVersionToSPDXListVersion :: CabalSpecVersion -> LicenseListVersion
-cabalSpecVersionToSPDXListVersion CabalSpecV3_8 = LicenseListVersion_3_16
+cabalSpecVersionToSPDXListVersion CabalSpecV3_8 = LicenseListVersion_3_20

change in particular changes the .cabal file format. That cannot be done at all, as old Cabals will fail to parse new cabal-version: 3.8 files.

Please, pay attention to the changes.

---

The extension changes affects what files may be uploaded on Hackage, i.e.

% cabal check
Warning: The following warnings are likely to affect your build negatively:
Warning: Unknown extensions: ExtendedLiterals
Warning: Hackage would reject this package.

that's probably the only noticeable issue of not including new extensions as KnownExtensions.

---

The haiku change can be better properly right: just add a constructor and make a major release.

       PackageDistInexcusable (UnknownCompiler unknownImpls)
   ]
   where
-    unknownOSs    = [ os   | OS   (OtherOS os)           <- conditions ]
+    unknownOSs    = [ os   | OS   (OtherOS os)           <- conditions, os /= "haiku" ]

which can be better done by adding a proper constructor (i.e. breaking change).

---

Make what you now have as 3.10.2.0 to be 3.12.1.0 (and add Haiku OS constructor). The major releases can be "small".

[phadej]

I've added extension StrictDataZ to the default-extensions list in my .cabal file and I'm getting:

That's because your compiler doens't support StrictDataZ, not because it's not KnownExtension.

[gbaz]

I think, as we had discussed in calls in the past, there's no reason the versions of cabal and cabal-install need to move in lockstep. because of reexports, at least for now, cabal-syntax and cabal do.

That said, given that the knownextensions thing is not that big a deal (hackage typically won't update to allow the latest extensions right away anyway), I would be happy with one of three solutions.

1. roll back the pvp-problematic changes
2. bump the release version of cabal-syntax and cabal but not cabal-install.
3. if the former makes people feel too confused, bump the release version of all three instead.

[bgamari]

We have discussed a few mitigations of this in the Stability Working Group. There are a few options here that we might decide to pursue in the future:

• Introduce something like Rust's non_exhaustive attribute, allowing us
• Split the KnownExtension type into a separate, independently-versioned package. This might even allow us to share this type with GHC.
• Provide the data constructors of KnownExtension via pattern synonyms, which would prevent GHC from concluding that matches are complete
• Possibly in conjuction with the previous suggestion, turn KnownExtension into a newtype around String

[phadej]

a newtype around String

Please verify that this is not a regression (memory usage nor performance) for applications like hackage-server which does (AFAIK) keep plenty of package descriptions (with extensions lists) in memory and also compares them. Changing from comparing a (machine word) tag to comparing String is non-trivial.

This is probably not as impactful as optimizing Version representation, but probably not completely negligible either.

[gbaz]

I think the non_exhaustive attribute would be a useful extension to ghc in general, even if the process means that it can't help with cabal for some time, and I really hope the SWG pursues it regardless of what other mitigations we may take.

[andreabedini]

I still am not convinced that promptly adding new extensions to Cabal-syntax is necessary.

I admit I only had a cursory look but I think everything would work as expected even an extension is unknown. UnknownExtensions are represented as strings which is exactly one of the options that @bgamari suggests.

I will dare further; the whole thing is unnecessary: cabal does not need to keep a list of known extensions at all. It does not do anything with it.

@phadej points out that hackage-server uses that list; in that case the list can be moved directly into hackage-server (perhaps not hardcoded but kept on a configuration file that can be easily changed).

[phadej]

@phadej points out that hackage-server uses that list;

hackage-server uses PackageDescription.Check, which uses the exception list.

[Kleidukos]

fixed by #9260 and #9259