cdepillabout / pretty-simple

pretty-printer for Haskell data types that have a Show instance
https://hackage.haskell.org/package/pretty-simple
BSD 3-Clause "New" or "Revised" License
243 stars 29 forks source link

Correct version bounds to avoid compile failure #1

Closed hvr closed 7 years ago

hvr commented 7 years ago

This avoids cabal running into the compile failure below:

Configuring component lib from pretty-simple-1.0.0.3...
Preprocessing library pretty-simple-1.0.0.3...
[1 of 7] Compiling Text.Pretty.Simple.Internal.Output ( src/Text/Pretty/Simple/Internal/Output.hs, /tmp/matrix-worker/1484457210/dist-newstyle/build/x86_64-linux/ghc-7.8.4/pretty-simple-1.0.0.3/build/Text/Pretty/Simple/Internal/Output.o )
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading package array-0.5.0.0 ... linking ... done.
Loading package deepseq-1.3.0.2 ... linking ... done.
Loading package bytestring-0.10.4.0 ... linking ... done.
Loading package transformers-0.3.0.0 ... linking ... done.
Loading package mtl-2.1.3.1 ... linking ... done.
Loading package containers-0.5.5.1 ... linking ... done.
Loading package binary-0.7.1.0 ... linking ... done.
Loading package text-1.2.2.1 ... linking ... done.
Loading package parsec-3.1.11 ... linking ... done.
Loading package hashable-1.2.5.0 ... linking ... done.
Loading package pretty-1.1.1.1 ... linking ... done.
Loading package template-haskell ... linking ... done.
Loading package nats-1.1.1 ... linking ... done.
Loading package transformers-compat-0.5.1.3 ... linking ... done.
Loading package tagged-0.8.5 ... linking ... done.
Loading package unordered-containers-0.2.7.2 ... linking ... done.
Loading package semigroups-0.18.2 ... linking ... done.
Loading package split-0.2.3.1 ... linking ... done.
Loading package primitive-0.6.2.0 ... linking ... done.
Loading package vector-0.12.0.0 ... linking ... done.
Loading package vector-algorithms-0.7.0.1 ... linking ... done.
Loading package mono-traversable-1.0.1 ... linking ... done.
Loading package base-orphans-0.5.4 ... linking ... done.
Loading package stm-2.4.4.1 ... linking ... done.
Loading package StateVar-1.1.0.4 ... linking ... done.
Loading package void-0.7.1 ... linking ... done.
Loading package contravariant-1.4 ... linking ... done.
Loading package distributive-0.5.1 ... linking ... done.
Loading package comonad-5 ... linking ... done.
Loading package bifunctors-5.4.1 ... linking ... done.
Loading package exceptions-0.8.3 ... linking ... done.
Loading package filepath-1.3.0.2 ... linking ... done.
Loading package prelude-extras-0.4.0.3 ... linking ... done.
Loading package profunctors-5.2 ... linking ... done.
Loading package semigroupoids-5.1 ... linking ... done.
Loading package free-4.12.4 ... linking ... done.
Loading package generic-deriving-1.11.1 ... linking ... done.
Loading package adjunctions-4.3 ... linking ... done.
Loading package kan-extensions-5.0.1 ... linking ... done.
Loading package parallel-3.2.1.0 ... linking ... done.
Loading package reflection-2.1.2 ... linking ... done.
Loading package lens-4.15.1 ... linking ... done.
Loading package old-locale-1.0.0.6 ... linking ... done.
Loading package time-1.4.2 ... linking ... done.
Loading package unix-2.7.0.1 ... linking ... done.
Loading package ansi-terminal-0.6.2.3 ... linking ... done.

src/Text/Pretty/Simple/Internal/Output.hs:27:1: Warning:
    The import of ‘Control.Applicative’ is redundant
      except perhaps to import instances from ‘Control.Applicative’
    To import instances alone, use: import Control.Applicative()
[2 of 7] Compiling Text.Pretty.Simple.Internal.OutputPrinter ( src/Text/Pretty/Simple/Internal/OutputPrinter.hs, /tmp/matrix-worker/1484457210/dist-newstyle/build/x86_64-linux/ghc-7.8.4/pretty-simple-1.0.0.3/build/Text/Pretty/Simple/Internal/OutputPrinter.o )

src/Text/Pretty/Simple/Internal/OutputPrinter.hs:78:29:
    Not in scope: ‘mappend’

src/Text/Pretty/Simple/Internal/OutputPrinter.hs:93:12:
    Not in scope: ‘mconcat’
    Perhaps you meant ‘concat’ (imported from Prelude)

src/Text/Pretty/Simple/Internal/OutputPrinter.hs:113:27:
    Not in scope: type constructor or class ‘Monoid’
    Perhaps you meant ‘Monad’ (imported from Prelude)

src/Text/Pretty/Simple/Internal/OutputPrinter.hs:113:37:
    Not in scope: type constructor or class ‘Traversable’

src/Text/Pretty/Simple/Internal/OutputPrinter.hs:242:30:
    Not in scope: ‘mappend’
cdepillabout commented 7 years ago

Thanks for this PR. I've merged it in.

However, I'm having trouble uploading the package when using pvp-bounds: both. When I run stack upload, I get the following error:

$ stack upload .
Getting file list for /home/me/git/pretty-simple/
Building sdist tarball for /home/me/git/pretty-simple/
Unable to parse cabal file /tmp/stack13425/pretty-simple-1.0.0.4/pretty-simple.cabal: FromString "'then' branch of 'if' is empty" (Just 55)

It looks like it is failing on the if / else that I'm using to determine whether or not to build the example program.

So I wasn't able to actually upload a version to Hackage yet that uses pvp-bounds: both.

cdepillabout commented 7 years ago

Actually I had one more question about pvp-bounds: both. I've never used the pvp-bounds config option before, so let me know if this question doesn't make sense.

Doesn't it make the version bounds too restrictive on Hackage? In Travis CI, I'm testing against Stackage LTS 5 (GHC 7.10.3), LTS 6 (GHC 7.10.3), LTS 7 (GHC 8.0.1), and nightly (GHC 8.0.1). So, for instance, the lower version bound for the lens dependency would be the version in LTS 5 (which is lens-4.13), and the upper bound for the lens dependency would be the version in nightly (which is lens-4.15).

However, pvp-bounds: both would give the package bounds like lens >= 4.14 && < 4.15 (since the stack.yaml file is pointing to LTS 7 which is lens-4.14).

Does this not cause a problem for users of cabal?

hvr commented 7 years ago

@cdepillabout as to the stack upload error, you should file an issue @ https://github.com/commercialhaskell/stack/issues

And you're right: if you test against multiple stackage snapshots then we'd actually need a variant of thepvp-bound-feature that allows to generate a union of version ranges inferred from multiple stackage snapshots. Iirc there's an issue about that in Stack's issue tracker, and I've also seen an external tool being worked on that may help with that as well... so in your case pvp-bounds:both may not be optimal yet.

cdepillabout commented 7 years ago

@hvr I will file an issue on stack's issue tracker.

It looks like https://github.com/commercialhaskell/stack/issues/2262 is the issue about pvp-bounds.

So, if I don't use the pvp-bounds feature, do you have a suggestion on what I should do to make it easier for cabal users? Just manually specify version bounds?

Also, in your original error, it looks like you're trying to use pretty-simple with GHC 7.8? Do you need pretty-simple to support older versions of GHC?

pretty-simple is currently only supporting GHC 7.10+, but that decision is pretty arbitrary. I just didn't want to deal with CPP'ing around Control.Applicative while doing development. But now that pretty-simple is working well, I have no problem with supporting older GHC versions.

hvr commented 7 years ago

@cdepillabout It's totally fine to support only GHC 7.10 or later, especially if this avoids CPP. I noticed this issue because the Hackage matrix CI builder stumbled over it. :-)

cdepillabout commented 7 years ago

Oh, okay, I see. I guess the Hackage matrix CI builder just needs the base version bounds specified correctly?