Cabal library API breakage in 3.14.0.0

[ulidtko]

Up until version 3.14, the public module Distribution.Simple of Cabal-the-library has been exporting these names:

• GlobalPackageDB
• UserPackageDB
• SpecificPackageDB

as constructors of PackageDB data type (indirectly, via reexport of Distribution.Simple.Compiler).

The patchset #10256, specifically commit 90fbf08, having shuffled the definitions a little bit, is now causing compile errors in either of two forms:

src/Distribution/Extra/Doctest.hs:481:8: error: [GHC-76037]
    Not in scope: data constructor ‘GlobalPackageDB’
    Suggested fix:
      Perhaps you want to add ‘GlobalPackageDB’
      to one of these import lists:
        ‘Distribution.Simple’ (src/Distribution/Extra/Doctest.hs:(66,1)-(68,23))
        ‘Distribution.Simple.Compiler’ (src/Distribution/Extra/Doctest.hs:(69,1)-(70,74))
    |
481 |       (GlobalPackageDB:dbs)
    |        ^^^^^^^^^^^^^^^

with import Distribution.Simple.Compiler (PackageDB(..)) being already there;

or

src/Distribution/Extra/Doctest.hs:70:59: error:
    In module ‘Distribution.Simple.Compiler’:
      ‘GlobalPackageDB’ is a data constructor of ‘PackageDBX’
    To import it use
      import Distribution.Simple.Compiler( PackageDBX( GlobalPackageDB ) )
    or
      import Distribution.Simple.Compiler( PackageDBX(..) )
   |
70 |        (CompilerFlavor (GHC), CompilerId (..), PackageDB, GlobalPackageDB, UserPackageDB, SpecificPackageDB, compilerId)
   |                                                           ^^^^^^^^^^^^^^^

Please at least document this in release notes, as it's a breaking change.

(BTW there's no 3.14.0.0 on https://github.com/haskell/cabal/releases )

[ulidtko]

One more with BuildCommonFlags (bundled, record) pattern synonym: #9718 (comment)

[ulidtko]

One more with SymbolicPathX vs FilePath:

src/Distribution/Extra/Doctest.hs:279:40: error: [GHC-83865]
    • Couldn't match type: [Char]
                     with: Distribution.Utils.Path.SymbolicPathX
                             Distribution.Utils.Path.AllowAbsolute
                             Distribution.Utils.Path.Pkg
                             (Distribution.Utils.Path.Dir Distribution.Utils.Path.PkgDB)
      Expected: PackageDBX
                  (Distribution.Utils.Path.SymbolicPath
                     Distribution.Utils.Path.Pkg
                     (Distribution.Utils.Path.Dir Distribution.Utils.Path.PkgDB))
        Actual: PackageDBX FilePath
    • In the expression:
        SpecificPackageDB $ distPref </> "package.conf.inplace"
      In the second argument of ‘(++)’, namely
        ‘[SpecificPackageDB $ distPref </> "package.conf.inplace"]’
      In the expression:
        withPackageDB lbi
          ++ [SpecificPackageDB $ distPref </> "package.conf.inplace"]
    |
279 |   let dbStack = withPackageDB lbi ++ [ SpecificPackageDB $ distPref </> "package.conf.inplace" ]
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[ulidtko]

... The BuildCommonFlags one puzzles me for good 💭

Seems like the post-refactor API is incomplete; the new pattern synonym BuildCommonFlags should be bi-directional... Yet, with its ViewPatterns definition, it can't.

Hi @sheaf, can I ask you as the implementor of #9718 — am I missing a way to rewrite snippets that use Cabal-the-library like so

miniRepro :: BuildFlags
miniRepro = defaultBuildFlags {buildVerbosity = undefined}

against Cabal 3.14?..

That (I think, correctly) yields:

src/Minirepro.hs:245:13: error: [GHC-16444]
    • non-bidirectional pattern synonym ‘BuildCommonFlags’ used in an expression
    • In a record update at field ‘buildVerbosity’
      and with pattern synonym ‘BuildCommonFlags’.
      In the expression: defaultBuildFlags {buildVerbosity = undefined}
      In an equation for ‘miniRepro’:
          miniRepro = defaultBuildFlags {buildVerbosity = undefined}
    |
245 | miniRepro = defaultBuildFlags {buildVerbosity = undefined}
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Under spoiler, I've minimized a bare ghci-ready model test, to spare us all of traversing Cabal codebase for smoother discussion:

minified example

Foo.hs:

{-# LANGUAGE PatternSynonyms, ViewPatterns #-}

module Foo
  ( Sauce (BuildPun, buildDolor, buildSitamet)
  , defaultSauce
  ) where

-- exported without constructor
data Sauce = Sauce
  { lorem :: Int
  , ipsum :: Bool
  , buildSauce :: CommonSauce
  }

data CommonSauce = CommonSauce
  { dolor :: String
  , sitamet :: Maybe ()
  }

pattern BuildPun :: String -> Maybe () -> Sauce
pattern BuildPun
  { buildDolor
  , buildSitamet
  } <- ( buildSauce -> CommonSauce { dolor=buildDolor, sitamet=buildSitamet })

defaultSauce = Sauce 42 False defaultCommonSauce
defaultCommonSauce = undefined

Bar.hs:

module Bar where

import Foo (Sauce (..), defaultSauce)

example :: Sauce
example = defaultSauce { buildDolor = "WAT" }

Same error message:

Bar.hs:6:11: error: [GHC-16444]
    • non-bidirectional pattern synonym ‘BuildPun’ used in an expression
    • In a record update at field ‘buildDolor’
      and with pattern synonym ‘BuildPun’.
      In the expression: defaultSauce {buildDolor = "WAT"}
      In an equation for ‘example’:
          example = defaultSauce {buildDolor = "WAT"}
  |
6 | example = defaultSauce { buildDolor = "WAT" }
  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Not a heavy user of pattern synonyms myself, so apologies for perhaps obvious question, and thanks in advance.

[sheaf]

... The BuildCommonFlags one puzzles me for good 💭

To set the verbosity field of BuildFlags in Cabal 3.14 and above, you can do the following:

setBuildFlagsVerbosity :: Verbosity -> BuildFlags  -> BuildFlags
setBuildFlagsVerbosity v flags@(BuildFlags { buildCommonFlags = common )} =
  flags { buildCommonFlags = common { setupVerbosity = v } }

Probably the most backward-compatible way to fix this would be to use lenses that would be provided by the Cabal library, but at the moment the Cabal library is very inconsistent about which lenses it provides. IMO Cabal should expose a Lens' BuildFlags Verbosity that would be usable across versions.

[ulidtko]

That's quite awkward 🫤 Also realize it must be wrapped into CPP version checks (which I see you did in cabal tests).

Wasn't the point of refactor to simplify?..

Anyhow, thanks for the reply, I'll try to adapt to this change as well.

[ulysses4ever]

hey @ulidtko ! thanks for raising this issue! just wanted to make it explicit here: do you expect Cabal to document all API changes between major versions? does any other package track these? I saw how some packages do highlight some "notable" API changes in the changelog, and we certainly could retroactively fix the notes for 3.14.0.0, and 3.14.1.0 can include these as well. But it's not clear to me that anyone has a reliable and exhaustive reporting of all API changes. I'd be curious to know how people set this up especially in case of big packages like Cabal+Cabal-syntax with ~60K LOC and several dozen modules and active contributors community.

[ffaf1]

But it's not clear to me that anyone has a reliable and exhaustive reporting of all API changes.

If migrating is not trivial — this seems to be the case — as a user I would be quite happy to see a migration guide.
Cabal devs/managers resources are finite, so PR authors need to help with foresight on what might throw a spanner in our users’ codebases.

[ulidtko]

Hi @ulysses4ever, thanks for the acknowledgement and response.

Per my standards, this issue itself is "documentation" already; of course I won't be demanding exhaustive listings of all changes in release notes. That's what git commit logs are for; and everyone wants release notes to be short read, too.

But nevermind my opinion; try to keep in mind the wider haskellers community, including less experienced and "primarily using another language" members. You upgrade a dependency (potentially a distant transitive one) — your thing breaks — you go check changelogs, immediately. That's muscle memory, that I hope all sides would agree is nice to cultivate.

Not documenting breaking changes simply erodes that culture of keepachangelog. Then things get that sticky fame of "breaks all the time" 😒 Consider e.g. that I don't have a good link to give to a teammate to point out precisely what happened; in a hurry, they'll just come out with the wrong, oversimplified conclusion.

As a haskeller myself, I can of course "enjoy" static breakage, at compile time. It should be said: that's lightyears better than runtime breakage. However, it's a nontrivial "haha, funny talk" idea to grasp; it takes experience to internalize it. So here, I'm appealing at the community again.

... And while at it, let me spell out these from fresh memory, for posterity.

Migration steps

Constructors of PackageDB: GlobalPackageDB, UserPackageDB, SpecificPackageDB

have moved to an auxiliary datatatype PackageDBX. Change imports:

import Distribution.Simple.Compiler
       (PackageDB, PackageDBX (GlobalPackageDB, UserPackageDB, SpecificPackageDB))

Fields of ConfigFlags, BuildFlags, InstallFlags, TestFlags, BenchmarkFlags, HaddockFlags, HscolourFlags, SDistFlags, CopyFlags, RegisterFlags, CleanFlags, ReplFlags

have partially moved to *CommonFlags. Use corresponding pattern synonyms and/or Monoid instance of CommonSetupFlags. Example for haddock command:

 import Distribution.Simple.Setup (HaddockFlags(..))
+import Distribution.Simple.Setup.Common (CommonSetupFlags(..))

 example =
   someHaddockFlags
-    { haddockVerbosity = a
-    , haddockTargets  = b
     }
+    { haddockCommonFlags = mempty
+      { setupVerbosity = a
+      , setupTargets = b
+      }
     }

Additions to SymbolicPath, RelativePath

Check the module Distribution.Utils.Path, it now provides more nuanced API that Cabal uses to keep track of filepath locations. (Hopefully, avoiding confusion which things should go where and how.)

In your specific circumstance, you'll need to decide how much of that nuance to keep. The function getSymbolicPath discards all of it, getting back the raw FilePath; but see the linked module's haddocks for caveats and less drastic options.

It may help to introduce a CompatSymPath typeclass.

[ffaf1]

This is fantastic and I will incorporate this via a PR as soon as the weekend kicks in.
Thanks @ulidtko!