haskell / fgl

A Functional Graph Library for Haskell
http://hackage.haskell.org/package/fgl
Other
184 stars 54 forks source link

Compile issue when FGL is used as third party dependency with GHC 7.0.4 and 7.2.2 #69

Closed danfran closed 6 years ago

danfran commented 6 years ago

I am using fgl as dependency in this project:

https://github.com/danfran/cabal-macosx/blob/master/cabal-macosx.cabal

On travis all the builds run successfully apart for GHC 7.0.4 and 7.2.2. The issue seems related to:

Downloading fgl-5.5.3.1...
Configuring fgl-5.5.3.1...
Building fgl-5.5.3.1...
Preprocessing library fgl-5.5.3.1...
[ 1 of 29] Compiling Paths_fgl        ( dist/build/autogen/Paths_fgl.hs, dist/build/Paths_fgl.o )
[ 2 of 29] Compiling Data.Graph.Inductive.Internal.Thread ( Data/Graph/Inductive/Internal/Thread.hs, dist/build/Data/Graph/Inductive/Internal/Thread.o )
[ 3 of 29] Compiling Data.Graph.Inductive.Graph ( Data/Graph/Inductive/Graph.hs, dist/build/Data/Graph/Inductive/Graph.o )
Data/Graph/Inductive/Graph.hs:132:3: Unrecognised pragma
[ 4 of 29] Compiling Data.Graph.Inductive.Basic ( Data/Graph/Inductive/Basic.hs, dist/build/Data/Graph/Inductive/Basic.o )
[ 5 of 29] Compiling Data.Graph.Inductive.PatriciaTree ( Data/Graph/Inductive/PatriciaTree.hs, dist/build/Data/Graph/Inductive/PatriciaTree.o )
Data/Graph/Inductive/PatriciaTree.hs:276:30:
    Not in scope: `IM.foldlWithKey''
Data/Graph/Inductive/PatriciaTree.hs:290:30:
    Not in scope: `IM.foldlWithKey''
Failed to install fgl-5.5.3.1

I assume that a way could be using "-f -containers042" for that versions but I have tried this on travis without any luck:

if [ $GHCVER = "7.0.4" ] || [ $GHCVER = "7.2.2" ];
     then
          cabal install --only-dependencies --enable-tests --enable-benchmarks -f -containers042;
     else
          cabal install --only-dependencies --enable-tests --enable-benchmarks;
     fi

as the flag is disabled only on the current cabal instance (not for dependencies).

Any idea how to use fgl as dependency with GHC 7.0.4 and 7.2.2?

ivan-m commented 6 years ago

This is odd, as I test GHC 7.0.4 and 7.2.2 support.

Can you please provide me with the full logs, especially containing the version of containers being used?

It looks like IM.foldlWithKey was only added in containers-0.4.2.0, so it might be that my Travis-CI builds are forcing a newer version of containers to be used.

ivan-m commented 6 years ago

@hvr you were the one that helped me add that flag in; do you have any suggestions as to why it succeeds in Travis-CI but fails here? Should I switch that flag to default: False (thus allowing Cabal to flip it for newer versions of GHC)?

danfran commented 6 years ago

@ivan-m you can find a full build log here for 7.0.4: https://travis-ci.org/danfran/cabal-macosx/jobs/261922077 and here for 7.2.2: https://travis-ci.org/danfran/cabal-macosx/jobs/261922078

The current travis conf I am using is: https://github.com/danfran/cabal-macosx/blob/master/.travis.yml

ivan-m commented 6 years ago

OK, setting default: False does the wrong thing (as in it tries to force that flag for newer versions of GHC).

I think I misunderstood the issue: the flag is working; it's just that the code doesn't support older versions of containers. It's just that for some reason Travis-CI didn't pick up on that; I think I need to configure it to disable that flag for older versions of GHC.

hvr commented 6 years ago

@ivan-m I'm investigating...

ivan-m commented 6 years ago

@hvr: I think it's that when building just fgl, there's nothing stopping it from installing a newer version of containers, so it keeps the flag enabled.

If, however, fgl is used as a dependency, then upgrading containers can break ghc, etc. breaking the install plan.

This is what happens if the flag defaults to False.

ivan-m commented 6 years ago

Looking at an earlier build: it says that the flag is chosen to be enabled.

hvr commented 6 years ago

Ok, so this is with older cabals and w/o new-build, right? What happens if --constraint "containers installed" is used?

hvr commented 6 years ago

@ivan-m I think we need to update your Travis CI w/ the latest travis-ci script generator... as the current one would have caught this problem

ivan-m commented 6 years ago

If I explicitly set the flag, it seems to work (in that the reported build failure occurs).

(And yes, older cabal-install, no new-build.)

Do you mean the new-build variant? I'd prefer to keep older cabal-install's being used with older GHCs.

hvr commented 6 years ago

Fair enough, but the older cabal-install is, the more bugs you'll encounter in its solver. cabal new-build has been designed to support all GHCs back to GHC 7.0, with the intent to phase out older cabal-install versions (note that old cabal versions also used to tell you when there was a new cabal-install release, and suggested to run cabal install cabal-install).

That being said, the function was added in haskell/containers@a631e10ce3a6634e0f85ed6f00c25baa960746f9, which judging from the git tags exists only w/ containers-0.4.2 and later (GHC 7.0.4 was bundled w/ containers-0.4.0; GHC 7.2.2 still shipped w/ containers-0.4.1; whereas GHC 7.4.2 ships w/ container-0.4.2.1), consequently the code in fgl is lacking some MIN_VERSION_containers(0,4,2) CPP compat layer to cope with the missing containers function.

Moreover, past releases of fgl will require its lower bound on containers tightened (I'm still investigating which releases were affected).

PS: I've confirmed that only 5.5.3.1 was affected, and I've already revised it as there's no way to use 5.5.3.1 with containers < 0.4.2

ivan-m commented 6 years ago

--constraint "containers installed" still seems to bring in containers-0.5.2.1 with GHC 7.0.4. This is using cabal-install 1.16 for both, which is odd (some change with how Cabal interacts with GHC?). So I think I'll go back to my hacky flag-based approach.

The main rationale I have for using as low a version of cabal-install as possible is that if people are still using an old version of GHC, then they might be using an old cabal-install as well and I want to make sure it still builds.

(The actual compilation problem is because I'm using a function not present in older versions of containers, but I want to make sure that this issue doesn't occur again by fixing my Travis CI configuration first.)

hvr commented 6 years ago

So I think I'll go back to my hacky flag-based approach.

what exactly was that? I'm just worried that it may break under new-build and that it's actually not even needed even for older cabals.

ivan-m commented 6 years ago
env:
  global:
    - CONF=""

matrix:
  include:
    - env: CABALVER=1.16 GHCVER=7.0.4 CONF="-f -containers042"
      compiler: ": #GHC 7.0.4"
      addons: {apt: {packages: [cabal-install-1.16,ghc-7.0.4], sources: [hvr-ghc]}}
    - env: CABALVER=1.16 GHCVER=7.2.2 CONF="-f -containers042"
      compiler: ": #GHC 7.2.2"
      addons: {apt: {packages: [cabal-install-1.16,ghc-7.2.2], sources: [hvr-ghc]}}
    # Other versions don't set CONF

I then append $CONF to calls to cabal install and cabal configure.

hvr commented 6 years ago

I see.

Btw, the reason of

--constraint "containers installed" still seems to bring in containers-0.5.2.1

is that you need to pass it to all commands which perform a configure; as otherwise it gets lost again.

and another reason may be, that the hacky cabal-caching logic in that pre-new-build script, cached a containers 0.5.2.1 version, which then satisfies the installed constraint too.

That being said, it makes sense to pass explicit flag settings for containers042 for that script; it's not that hacky.

ivan-m commented 6 years ago

I did have the --constraint bit everywhere, but yes, it seems there was some caching going on.

So if I deleted the caches would the --constraint variant work? (And would that be considered less hacky than using environment variables?)

hvr commented 6 years ago

IMO, both are reasonable ways. And both are only 99% robust in the old-build world :-)

Btw, the new-build script generator actually natively supports forcing installed constraints for all GHC bundled packages, and does so by default (with an escape hatch)

ivan-m commented 6 years ago

@danfran This should do it; it's getting late here though so I'll make the release tomorrow.

@hvr thanks for your help!

danfran commented 6 years ago

@ivan-m @hvr awesome! Just tested 7.0.4 and 7.2.2 builds and gone green :) thank you!

ivan-m commented 6 years ago

OK, I've just released 5.5.4.0 with this fix in it.