search

haskell-hvr / cassava

A CSV parsing and encoding library optimized for ease of use and high performance
http://hackage.haskell.org/package/cassava
BSD 3-Clause "New" or "Revised" License
223 stars 107 forks source link

building cassava with text-short-0.1.3 breaks #174

Closed recursion-ninja closed 5 years ago

recursion-ninja commented 5 years ago

I tried to build our program which depends on cassava and text-short >= 0.1.3. I don't understand why it fails to build. I got this error:

$ cabal new-build --with-compiler=ghc-8.6.5
...
...
...
Configuring library for cassava-0.5.1.0..
Preprocessing library for cassava-0.5.1.0..
Building library for cassava-0.5.1.0..

Data/Csv/Conversion.hs:26:3: error:
     error: #error **INVARIANT BROKEN** Detected invalid combination of `text-short` and `bytestring` versions. Please verify the `pre-bytestring-0.10-4` flag-logic in the .cabal file wasn't elided.
     # error **INVARIANT BROKEN** Detected invalid combination of `text-short` and `bytestring` versions. Please verify the `pre-bytestring-0.10-4` flag-logic in the .cabal file wasn't elided.
       ^~~~~
   |
26 | # error **INVARIANT BROKEN** Detected invalid combination of `text-short` and `bytestring` versions. Please verify the `pre-bytestring-0.10-4` flag-logic in the .cabal file wasn't elided.
   |   ^

Data/Csv/Conversion.hs:99:0: error:
     warning: "MIN_VERSION_text_short" is not defined, evaluates to 0 [-Wundef]
     #if MIN_VERSION_text_short(0,1,0)

   |
99 | #if MIN_VERSION_text_short(0,1,0)
   | ^

Data/Csv/Conversion.hs:99:0: error:
     error: missing binary operator before token "("
   |
99 | #if MIN_VERSION_text_short(0,1,0)
   | ^

Data/Csv/Conversion.hs:1010:0: error:
     warning: "MIN_VERSION_text_short" is not defined, evaluates to 0 [-Wundef]
     #if MIN_VERSION_text_short(0,1,0)

     |
1010 | #if MIN_VERSION_text_short(0,1,0)
     | ^

Data/Csv/Conversion.hs:1010:0: error:
     error: missing binary operator before token "("
     |
1010 | #if MIN_VERSION_text_short(0,1,0)
     | ^
`gcc' failed in phase `C pre-processor'. (Exit code: 1)

Failed to build perfect-vector-shuffle-0.1.1. The failure occurred during the
final install step.

It looks similar to #154 . It mentions new versions of cabal with ^>= will resolve the need for the CPP nonsense. Have we crossed that threshhold yet? How do I build our code with text-short >= 0.1.3?

RyanGlScott commented 5 years ago

I think I'm hitting the same (or at least a very similar) issue. If you build cassava-0.5.1.0 with --allow-newer=bytestring (which is what haskell-ci does when building things with head.hackage), then Cabal will pick a build plan that enables the bytestring--LT-0_10_4 flag. But if you look at how it's used in cassava.cabal:

https://github.com/haskell-hvr/cassava/blob/341963b005b4d283cd862127c9831e707e68c30f/cassava.cabal#L109-L114

Enabling bytestring--LT-0_10_4 only makes sense when you're using an old version of bytestring. For whatever reason, passing --allow-newer=bytestring seems to make Cabal's constraint solver ignore what version of bytestring you're using (which, in the case of recent GHCs, is much newer than bytestring-0.10.4). I'm not sure if this is a Cabal constraint solver bug or not, but one thing is clear: passing --allow-newer=bytestring inevitably causes +bytestring--LT-0_10_4 to be chosen, which leads to the **INVARIANT BROKEN** error reported above.

It seems like a simple solution to this problem would be to make bytestring--LT-0_10_4 disabled by default (i.e., add default: False to its stanza in cassava.cabal). Since +bytestring--LT-0_10_4 only makes sense for older versions of bytestring, this seems like the less common code path anyways.

RyanGlScott commented 5 years ago

Indeed, it looks like adding default: False was exactly the workaround implemented in hvr/head.hackage@7107ad3221c80f652dffe791b1bf7cc884abb19b. Is there any reason not to adopt a similar fix upstream?

recursion-ninja commented 5 years ago

This is currently preventing our entire project from building and additionally preventing us from benchmarking our project as criterion depends on cassava.

Can we make fixing this high priority?

recursion-ninja commented 5 years ago

Can a proper fix of this defect be released? Still breaking our entire project from building.

hvr commented 5 years ago

@recursion-ninja unfortunately I couldn't reproduce this; I did the following

git clone https://github.com/amnh/PCG
cd PCG/
cabal v2-build -w ghc-8.6.5

and it succeded without any issue

recursion-ninja commented 5 years ago

Some change made (or PR merged) in the last 35 days appears to have resolved the issue.