Guard PackageInfo behind cabal-version ≥ 3.12

[ffaf1]

Towards #9331.
I took the liberty to to run with @andreabedini #9374 as the Rubenesque #8427 got merged meanwhile, meaning the former PR needed to be updated.

Once we are fine with this patch, there is the question of whether — and how — to backport it to 3.10.

Include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.
• Manual QA notes have been included.
• Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

---

QA notes

• create a new folder, cd into it (e.g. mkdir prova && cd prova)

• cabal init -nm and then modify the file to have PackageInfo_pkg modules. Example follows:

    cabal-version: 2.4
    name: prova
    version: 0
    license: GPL-3.0-or-later
    maintainer: Someone
    category: Example
    synopsis: Foo
    description: FooBar
    build-type: Simple
    extra-doc-files: CHANGELOG.md
  
    library
        default-language: Haskell2010
        build-depends: base <5
        -- ☞ N.B.: PackageInfo packages must contain the same name of
        -- of the package! (In this example: `prova`).
        autogen-modules: PackageInfo_prova
        exposed-modules: PackageInfo_prova
  

• cabal v3.10.*: run cabal check, no error/warnings about PackageInfo modules is reported

• cabal master: run cabal check, you will be warned about having to use cabal-version 3.12 or higher

[Mikolaj]

Yes, the backport if crucial, but I appreciate how it can prove laborious.

[ffaf1]

To kickstart the discussion:

• this patch adds a CabalSpecV3_12 constructor, is that correct? Should it be CabalSpecV3_10 instead?
• for the eventual backport to 3.10, I don't think it is possible to introduce that constructor (it would be a breaking change, am I wrong?). My idea is to
  • have the relevant warning fire without checks on cabal-version (because, for the above-mentioned reasons, we cannot add a new CabalSpecV3_X.
  • have the program fail here if the user sets cabal-version to 3.12
    

    cabal/cabal-install/src/Distribution/Client/Init/Interactive/Command.hs

    Line 315 in ed3b269

    parseCabalVersion "3.12" = CabalSpecV3_12

    
    This way the feature is still usable locally, there is a warning about it not being suitable for Hackage, and a clear upgrade path (clear when 3.12 is released).

[Mikolaj]

To kickstart the discussion:

* this patch adds a `CabalSpecV3_12` constructor, is that correct? Should it be `CabalSpecV3_10` instead?

Yes, this seems correct to me.

* for the eventual backport to 3.10, I don't think it is possible to introduce that constructor (it would be a breaking change, am I wrong?). My idea is to

Yes, we shouldn't suddenly suddenly introduce a new cabal format version in a point release.

  * have the relevant warning fire without checks on cabal-version (because, for the above-mentioned reasons, we cannot add a new `CabalSpecV3_X`.

Right.

  * have the program fail here if the user sets `cabal-version` to 3.12
    https://github.com/haskell/cabal/blob/ed3b26946c5ed0d96f10be2160aba10bacf00ba6/cabal-install/src/Distribution/Client/Init/Interactive/Command.hs#L315

Sounds like a useful extra diagnostics for a particularly persistent user. Perhaps, in the general error message about the need to wait for cabal 3.12, also allude to the feature that we remove in 3.10.3 and that setting cabal-version won't remove the warning nor make it acceptable on Hackage, until cabal 3.12 is out.

    This way the feature is still usable locally, there is a warning about it not being suitable for Hackage, and a clear upgrade path (clear when 3.12 is released).

Perfect.

I see your PR handles both step 2 and 3 of the plan and the modified backport will cover step 5. I wasn't aware the checks in cabal build and in cabal check are so intertwined, but it's actually good, we want the two code paths to be in sync and not duplicated.

So, it seems, you solve all our problems :) except patching Hackage, which may benefit from your code as well.

[Mikolaj]

In an implementation of point 3 of the plan, I'd also expect cabal build to fail if cabal-version is too low and the new features are present in the .cabal file (the parser should complain, I guess, just as it does for many other features?). However, I don't see any test that checks that. E.g., with cabal-version: 2.2 and sublibraries, cabal complains

~/r/horde-ad$ cabal build -j1
Errors encountered when parsing cabal file ./horde-ad.cabal:

horde-ad.cabal:206:18: error:
unexpected Sublibrary dependency syntax used. To use this syntax the package needs to specify at least 'cabal-version: 3.0'. Alternatively, if you are depending on an internal library, you can write directly the library name as it were a package.

  205 |     build-depends:
  206 |       , horde-ad:horde-ad-simplified
      |                  ^

[ffaf1]

I am not sure I like this solution (using check logic is a bit of a hack, more principled would be using a parse error), but let us just see if test suite passes for the meantime.

[Mikolaj]

Oh, great, thank you for the cabal build error in cabal 3.12. OTOH, in cabal 3.10 the same should be a warning, because in 3.10 the user can't set cabal-version:3.12 and we don't want to block functionality in 3.10.3 that was there 3.10.2 and was not broken but merely not guarded enough (our fault, the user should not be punished). Right?

[ffaf1]

Yes, should definitely be just a warning in 3.10.

[Mikolaj]

@ffaf1: I probably missed this: is the PR ready for re-review?

[ffaf1]

Nope, CI is redder than a bowl of borscht. I will take a stab in the weekend!

[Mikolaj]

LGTM