search

haskell / fgl

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

Fixing build with MonadFailDesugaring #80

Closed recursion-ninja closed 5 years ago

recursion-ninja commented 5 years ago

This resolves #79.

I had to use CPP for conditional imports and constraints, which is ugly, but not too intrusive.

ivan-m commented 5 years ago

I don't mind CPP; I've had to use it enough times myself :/

If nothing else, this PR has also pointed out to me that I may need to re-implement the benchmarks away from microbench as it hasn't been touched in 10 years and looks like it won't work with 8.8 :(

recursion-ninja commented 5 years ago

I try to avoid CPP if I can and like to let people know when I've added it.

I applied your code review suggestions and squashed the commits.

recursion-ninja commented 5 years ago

Should be ready to merge.

peti commented 5 years ago

Ping?

ivan-m commented 5 years ago

Sorry, work has been rather busy so I haven't had a chance to look at it. Hopefully next week.

On mobile; please excuse any tpyos.

On Thu, 27 Sep. 2018, 10:57 pm Peter Simons, notifications@github.com wrote:

Ping?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell/fgl/pull/80#issuecomment-425124800, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwXc_F1KmibGRiqf2MiYYknipypmkicks5ufOdngaJpZM4WFFNo .

phadej commented 5 years ago

@recursion-ninja if you think breaking change is better way, could you depend on fail to provide uniform API.

It would be very surprising for downstream if when upgrading to GHC-8.6 they need to write MonadFail instance for their monad (if someone is so silly that doesn't use IO or ST directly).

ivan-m commented 5 years ago

I prefer to keep FGL depending solely on boot packages.

recursion-ninja commented 5 years ago

@ivan-m, so we'll stick with the CPP and not depend on the fail package.

Since that's the case, this should be ready to merge.

recursion-ninja commented 5 years ago

Is there a time line for merging this? I'd like it to be able to use the fgl and libraries that transitively depend of fgl with GHC-8.6.1. Is there anything else required for this to be merged?

martijnbastiaan commented 5 years ago

@recursion-ninja as a temporary workaround you could use:


source-repository-package
  type: git
  location: https://github.com/recursion-ninja/fgl.git
  tag: d49cd7b0cd766d9f4647e140e263de1b94ace0c5

in your cabal.project. I believe this only works if you're already using new-*, though. I've tested this for https://github.com/clash-lang/clash-compiler/ which seems to work fine. It would be great if this patch could be merged soon though!

recursion-ninja commented 5 years ago

@martijnbastiaan that is a nice, temporary solution. However, this pull request should be merged and a new version of fglupload sooner rather than later.

peti commented 5 years ago

Does someone have the time to initiate a non-maintainer upload request as per https://wiki.haskell.org/Hackage_trustees?

ivan-m commented 5 years ago

Sorry, this slipped my mind; I'll do it this weekend.

ivan-m commented 5 years ago

I'm not sure why Travis-CI didn't catch this, but this actually fails with cabal-install-2.4 due to the deprecation of cabal install.

grumbles

phadej commented 5 years ago

@ivan-m, cabal-install-2.4 print the warning, but it doesn't fail.

Sent from my iPhone

On 13 Nov 2018, at 17.11, Ivan Lazar Miljenovic notifications@github.com wrote:

I'm not sure why Travis-CI didn't catch this, but this actually fails with cabal-install-2.4 due to the deprecation of cabal install.

grumbles

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

ivan-m commented 5 years ago

Sorry, you're right; I fail at reading error messages.