-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support repline-0.3 (and haskeline-0.8 properly) #1717
Conversation
Building dhall-nix with the new constraints will require haskell-nix/hnix#540. |
dontCrash m = | ||
Haskeline.catch | ||
Control.Monad.Catch.catch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not very keen on this, but it might be possible to preserve compatibility with older versions of haskeline and repline with a bit of CPP.
If the hnix release takes a long time, that might be worth looking into…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sjakobi: You will need to support older versions of haskeline
in order to support older versions of GHC. haskeline
is one of GHC's "boot libraries" (https://gitlab.haskell.org/ghc/ghc/-/wikis/commentary/libraries/version-history)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haskeline
is upgradeable – I can compile it fine with e.g. 8.6.5. Do you have different compatibility issues in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind. For some reason I thought stack
could not upgrade boot libraries, but apparently haskeline
is an exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sjakobi: It looks like you might need to support older versions of Haskeline due to Nix. It appears to misbehave in the following way when upgrading haskeline
:
Warning:
This package indirectly depends on multiple versions of the same package. This is very likely to cause a compile failure.
package hnix (hnix-0.7.1) requires haskeline-0.7.4.2
package repline (repline-0.3.0.0-Apjg8sKpeyJ3V54YeFv2Wr) requires haskeline-0.8.0.0-2fxBQp1muOsBHoaEfLruXE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gabriel439 Shouldn't that error go away when we update hnix
once haskell-nix/hnix#540 is merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sjakobi: I'm not sure, but my guess is: probably not. My rough understanding of what is going on is:
- The GHC built by Nix comes with boot libraries (e.g.
haskeline-0.7.4.2
) - You can still build a newer
haskeline
(e.g.haskeline-0.8.0.0
) and even build packages that depend on that newerhaskeline
(e.g.repline-0.3.0.0
), but somehow things go awry when the newerhaskeline
becomes a transitive dependency because then the newerhaskeline
conflicts with the olderhaskeline
included with GHC
So I think the problem would only disappear if we can figure out to prevent Nix from using the older haskeline
at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that sounds tricky.
I guess the easiest way to backward compatibility would be to avoid the MonadException
/ MonadCatch
constraints, and to use our concrete Repl
type instead:
type Repl = Repline.HaskelineT (State.StateT Env IO)
Do you think that could work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sjakobi: Yeah, that seems fine
@Gabriel439 This might be a nice addition for the minor release. Please take a look if you've got a minute! :) |
@sjakobi: For future reference, I believe if you use GitHub's support for draft pull requests then I'll get an e-mail notification when the pull request leaves the draft status |
Oh, I'll give that a try then! :) |
4b8177c
to
5b3a0d5
Compare
Relevant changelogs:
The changelog for
haskeline
fails to mention that itsMonadException
module was removed in favor ofMonadCatch
.