hpdeifel / hledger-iadd

A terminal UI as drop-in replacement for hledger add.
BSD 3-Clause "New" or "Revised" License
77 stars 8 forks source link

'Duplicate instance declarations' build failure with `text-ansi` 0.2.1.1 and `text-builder-dev` 0.3.4.1 #81

Closed Vekhir closed 6 months ago

Vekhir commented 7 months ago

Description

When building hledger-iadd 1.3.20 with GHC 9.8 on Arch, the following error occurs:

[3 of 5] Compiling DateParserSpec   ( tests/DateParserSpec.hs, dist/build/spec/spec-tmp/DateParserSpec.dyn_o )

tests/DateParserSpec.hs:201:10: error: [GHC-59692]
    Duplicate instance declarations:
      instance Arbitrary Day -- Defined at tests/DateParserSpec.hs:201:10
      instance [safe] Arbitrary Day
        -- Defined in ‘quickcheck-instances-0.3.30:Test.QuickCheck.Instances.Time’
    |
201 | instance Arbitrary Day where
    |          ^^^^^^^^^^^^^

Analysis

This occurs due to text-builder-dev adding quickcheck-instances as a dependency in 0.3.4.1 and later. As tests/DateParserSpec uses import Test.QuickCheck, that also imports Test.QuickCheck.Instances leading to the duplication.

This issue is even explained in the description of quickcheck-instances:

Since all of these instances are provided as orphans, I recommend that you do not use this library within another library module, so that you don't impose these instances on down-stream consumers of your code.

Further, the text-builder-dev library is used subsequently until it reaches hledger-iadd, except that text-ansi switched to text-builder-linear with version 0.3.0, and so it doesn't depend on text-builder-dev anymore (transitively). As Arch still ships text-ansi 0.2.1.1, this problem became visible.

Potential solutions

Not sure what to do from a dev perspective. hledger-iadd could add quickcheck-instances themselves, using the new definition, or somehow try to hide it. Perhaps the better solution would be in text-builder-dev to remove it again as explained in the description above. Since newer text-ansi isn't affected by this issue, perhaps adding a requirement for that is most appropiate. Though the dependency arises by the use of hledger-lib, so ideally they would restrict text-ansi to not export the orphans themselves. Could be a tough sell though for something that only affects downstream. The quickest solution for users and packagers is to simply update text-ansi to 0.3.0 or above. This requires GHC 9.2 or above.

System information

OS: Arch Linux Kernel: 6.7.2-arch1-1 GHC: 9.8.1 hledger-iadd: 1.3.20 hledger-lib: 1.32.3 text-ansi: 1.2.1.1 text-builder: 0.6.7.2 text-builder-dev: 0.3.4.2

A lot of the analysis came about during the writing of it. If anything, this report is useful when Arch officially updates the affected packages or if someone else does a weird half upgrade of packages. I intend to close this issue when haskell-text-ansi gets updated to 0.3.0 or higher. -- Vekhir

hpdeifel commented 6 months ago

Thanks for the detailed report! I opted for the pragmatic solution and replaced the custom Arbitrary instance with a dependency on quickcheck-instances. It's a simple fix and helps users (not only on arch) that are stuck on text-ansi <0.3