darcs

Patch 2017 support ghc-8.8 and ghc-8.10

Title support ghc-8.8 and ghc-8.10
Superseder Nosy List bf
Related Issues
Status accepted Assigned To
Milestone

Created on 2020-04-28.10:25:31 by bf, last changed 2020-07-25.23:17:28 by ganesh.

Files
File name Status Uploaded Type Edit Remove
loosen-upper-bound-for-haskeline-to-_0_9.dpatch bf, 2020-04-28.10:25:27 application/x-darcs-patch
patch-preview.txt bf, 2020-04-28.10:25:27 text/x-darcs-patch
unnamed bf, 2020-04-28.10:25:27 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg22006 (view) Author: bf Date: 2020-04-28.10:25:27
This makes screened build cleanly (no warnings) with ghc-8.8 and ghc-8.10.

I started by selectively pulling patches from branch-2.14. As expected, most
of them conflicted. I would have liked to rebase them to avoid the
conflicts, but since rebase currently does not work correctly with
conflicted patches I could not do that. Therefore the many conflict
resolution patches. If you happen to find the time: it would be a valuable
real-world test of the new fullUnwind-based rebase implementation if you
were able to squash the conflict resolutions with the original patches
(please add a "[rebased]" marker to the patch name if you do that). I won't
immediately screen this bundle to give you some time to consider whether you
want to try that.

It turned out that shelly-1.9 breaks our shell tests even on Linux: the
harness no longer finds any of them. Since for Windows we already have a
special case here, requiring <1.7.2, I decided to import shelly-1.7.1 into
our source tree and fix the build problems, rather than keep waiting any
longer for upstream to get their homework done (which may in fact never
happen). This worked out remarkably well. However, it currently works only
when building from a darcs clone, since the files in the shelly sub
directory are not included in the cabal source dist. In the long run we
should consider switching to turtle, which is actually maintained by the
author (a well known and quite diligent member of the community).

Running the shell tests confirmed my suspicion that merely replacing fail
with error to avoid proliferating MonadFail constraints all over the place
is a bad idea. In fact there was only one single place where this is the
correct fix. In most other places we actually want to replace it with (throw
. userError), or, where possible, with (liftIO . fail). I will backport
these changes and the shelly patches to make a new 2.14.4 release.

33 patches for repository http://darcs.net/screened:

patch 46bebd0a1511232b216d387392d964cae6fcac2c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 21 10:12:58 CEST 2020
  * loosen upper bound for haskeline to <0.9
  
  This is required to find a valid build plan for ghc >= 8.8.

patch 5d63e665634c964d7b23dcf9c28efc72d6a6f947
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 21 10:14:29 CEST 2020
  * loosen upper bound for shelly to <1.10
  
  This is required to find a build plan with ghc >= 8.8.
  
  A comment in the cabal file claims we cannot use shelly-1.9 because of two
  open issues. However, these issues only concern windows; but for windows we
  use an even stricter bound in a separate build-depend stanza, where we
  require < 1.7.2. I have moved the comment to this other stanza. The upshot
  is that building against ghc versions >= 8.8 is currently not supported on
  windows.

patch bc5ff9f3c122b590a26226091ab8717a6ba06abc
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 14:52:16 CEST 2020
  * resolve conflicts after 5d63e665634c964d7b23dcf9c28efc72d6a6f947

patch 2dbe7ff7042a2d909640f468c2d1dd39a00ca38b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Apr 26 20:32:02 CEST 2020
  * darcs.cabal: clean up extra-source-files
  
  Also added C source files in addition to header files.

patch 9c2e42144da778426b09f6c3b8ec03d285a106b7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 15:15:40 CEST 2020
  * resolve conflicts after 2dbe7ff7042a2d909640f468c2d1dd39a00ca38b

patch 353a1cfd0a42393b568c32f328b8a7d626a327f0
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 15:16:27 CEST 2020
  * fix license in darcs.cabal

patch 447f754fd0613f221cdf6174e67a2e020d71bdb6
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 21 08:45:05 CEST 2020
  * for base >=4.13 define fail in MonadFail instances

patch 8322b50f998b2f2244937d629ad66becf0f4df1e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 21 15:00:04 CEST 2020
  * add some elementary build instructions to the README.md

patch cacc6c44c3e8ae94e3aa3456e3f0ded9d00371f5
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 21 12:47:18 CEST 2020
  * adapt release/release.sh for use with modern cabal

patch 82fe27cc6295a2e55936b3b1ff10fcb4966e1e5e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Apr 24 10:45:44 CEST 2020
  * mitigate issue2643 with a better error message
  
  If the patch index is corrupt, output the name of the currupt file and
  suggest its removal.

patch 838279091622f16b331cef25c0b96aac01ad71d4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 15:32:18 CEST 2020
  * resolve conflicts after 447f754fd0613f221cdf6174e67a2e020d71bdb6

patch cf2e7bcff2195e03410305ffc25f2282e35f32a4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 15:34:05 CEST 2020
  * resolve conflicts after 82fe27cc6295a2e55936b3b1ff10fcb4966e1e5e

patch 347aeb4b5c1eccfe00956ac318a2123bca9ef9ca
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Apr 26 20:26:07 CEST 2020
  * clean up Darcs.Test.Patch.Check
  
  There was a comment in the code about the strangeness of how in this test
  module returning a Boolean was used to communicate a failure, and how this
  should be replaced with a more conventional way of error handling. This is
  now done, using the MaybeT monad transformer.

patch f85f4c41eeb116483655b6cd6c337845e63226ce
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 15:54:37 CEST 2020
  * resolve conflicts after 347aeb4b5c1eccfe00956ac318a2123bca9ef9ca

patch 64b5842ba3e32290d9eb479c8281eff4cfcea443
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Apr 24 15:27:26 CEST 2020
  * add changelog entry for release 2.14.3

patch 31c1837213befdef6b8ff513ad20ea7aaebbca1b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 16:03:42 CEST 2020
  * darcs.cabal: remove redundant version constraints

patch 3c481cfde19965f004913f1798a3036b4437bf59
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 16:22:29 CEST 2020
  * remove a redundant import from Setup.hs

patch 5f6825daaa5a145f827c83d0e1db826c45ee09e3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Apr 26 21:04:14 CEST 2020
  * no longer export fromJust from Darcs.Prelude [amended from branch-2.14]
  
  While used in many places, fromJust is a partial function and we should not
  encourage its unfettered use in Darcs.

patch 80223edc7108090f9c3231e36d4bdb94e1f52f6a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Apr 26 20:40:58 CEST 2020
  * remove redundant imports
  
  This mostly concerns (<>) imported from Darcs.Util.Printer and (<$>) from
  Control.Applicative. These are now consistently exported from Darcs.Prelude.

patch 68d2ba7bfe8b65168b2be95017ab5209521eccbd
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 21:03:00 CEST 2020
  * import shelly-1.7.1 as a new subdirectory

patch 241fee554ecef415e3c16e1147cf5c8d47b0af6b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 22:01:25 CEST 2020
  * make shelly build w/o warning for all supported compilers

patch e24a22c1c41b52bb0f1436abaf24704ea7bdc985
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Apr 27 22:02:30 CEST 2020
  * remove the special shelly upper bound for windows
  
  This is no longer necessary as cabal will now always use our (fixed) copy of
  shelly-1.7.1, at least when built from a clone. The cabal source distribution
  won't be able to run the shell tests.
  This is stop-gap-measure. We should eventually get rid of shelly.

patch ef62aa290cb72fdc8b06a11307cdebaad2a8f321
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:03:17 CEST 2020
  * resolve conflicts after 80223edc7108090f9c3231e36d4bdb94e1f52f6a

patch 466f1ac0b9623d0fa8c3ddf19f338af504d0b89c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:25:30 CEST 2020
  * unwind tests: use condition compilation for MonadFail instances and imports

patch 670806d16f3fd429b009d826c01c7199c1e3c745
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:28:22 CEST 2020
  * avoid use of fail in Darcs.Patch.Depends.unwrapOneTagged
  
  We specialize to monad Maybe and directly use Nothing.

patch ac8509387d1f8b87d3c8aeb758bd492b1a5ebfda
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:30:00 CEST 2020
  * more consequently throw MatchFailure exception
  
  This avoids two uses of fail in Darcs.Patch.Match in the context of a monad
  tranformer that is not necessarily IO based.

patch 5e2b79a3fc963ca1b8cfcd72b5ccf2f0d39d6837
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:32:54 CEST 2020
  * Darcs.Util.Prompt: add liftIO when calling fail 

patch 4ff9cb1b4352e2a447cead067ea5cc1a34929c9c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:38:38 CEST 2020
  * replace fail with error in Darcs.Repository.Diff
  
  This is the /only/ case where the "quick fix" of replacing fail with error
  is semantically valid. Here we clearly have a partial pattern match and it
  would be a bug if we hit that case.

patch d2c7b97789f538ac80a9547ed86db06ad57aa209
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:41:27 CEST 2020
  * replace fail with throw.userError when applying a patch
  
  There are several uses of apply (and its variants) where we catch
  IOExceptions e.g. maybeApplyToTree. This makes sense: if we apply in the
  context of IO (or a transformer over IO), then there may occur any kind of
  IOException, not only the ones explicitly raised from our code, and we want
  to catch them all.

patch faaceaffbf4c60a73c0c5643ea107ad8a2f8c753
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:49:16 CEST 2020
  * replace fail with throw.userError in Tree monads and index matching
  
  The situation here is different from that of patch application. Here we want
  the exception to percolate all the way up to the top level handler. In other
  words, these are regular failures similar to IO exceptions.

patch 9e9c1c5e2c6b23632605a5885d958b3cd3e20bb1
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 08:54:21 CEST 2020
  * replace runhaskell with runghc in tests/renames.sh
  
  It seems runghc is better supported by ghc packagers than runhaskell. This
  is not a complete solution, though. If cabal is used with
  --with-compiler=/path/to/ghc, there is no guarantee that we have a runghc or
  a runhaskell in the PATH.

patch 5e216ccf2ae019a95d16a9830185ced1720c9de8
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 09:43:25 CEST 2020
  * darcs.cabal: remove containers from setup-depends

patch c7fb6002aa234d1462bbf70c32334497f17e0fa7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Apr 28 11:00:37 CEST 2020
  * raise upper bounds for base and Cabal dependencies
  
  This allows to build with ghc-8.8 and ghc-8.10.
Attachments
msg22018 (view) Author: bf Date: 2020-05-18.00:15:32
I would like to get this into screened soon. I have another set of
patches that replace shelly with a different library, but that needs a
bit of discussion as there may be better alternatives.

Can we come to a decision what to do with the conflicts?
msg22019 (view) Author: ganesh Date: 2020-05-18.06:22:38
I don't mind either option, but personally would probably go with 
merging+conflict resolutions rather than rebase given that it is possible.
msg22174 (view) Author: ganesh Date: 2020-07-18.16:03:06
Sorry for the duplicate, it's been so long since I reviewed I had
forgotten I need to send email reviews to bugs@. I'm resending so it
appears on the bug tracker but feel free to reply to either one.

This should be the same as the message already sent direct to
darcs-devel modulo some reformatting/snipping where quoting seems to
have gone wrong.

>   * loosen upper bound for haskeline to <0.9

OK.

>   * loosen upper bound for shelly to <1.10
>   * resolve conflicts after 5d63e665634c964d7b23dcf9c28efc72d6a6f947

OK.

>   * darcs.cabal: clean up extra-source-files
>   * resolve conflicts after 2dbe7ff7042a2d909640f468c2d1dd39a00ca38b

OK.

>   * fix license in darcs.cabal

OK (had to stop and check that we have always been GPL-2.0-or-later, I
guess these things are easy to forget!)

>   * for base >=4.13 define fail in MonadFail instances
>   * resolve conflicts after 447f754fd0613f221cdf6174e67a2e020d71bdb6

OK. Though :-( to CPP, of course it's pretty much unavoidable for the
range of GHC versions wanted.

>   * add some elementary build instructions to the README.md

OK (I learnt a few things from that!)

>   * adapt release/release.sh for use with modern cabal

OK - I assume you will have battle-tested this.

>   * mitigate issue2643 with a better error message
>   
>   If the patch index is corrupt, output the name of the currupt file
>   and suggest its removal.
> 
>   * resolve conflicts after 82fe27cc6295a2e55936b3b1ff10fcb4966e1e5e

Looks good. The user message might be clearer still if it said that it
is always safe to remove the index but it's no big deal.

>   * clean up Darcs.Test.Patch.Check
>   
>   There was a comment in the code about the strangeness of how in this 
>   test module returning a Boolean was used to communicate a failure,
>   and how this should be replaced with a more conventional way of
>   error handling.
>   This is now done, using the MaybeT monad transformer.
> 
>   * resolve conflicts after 347aeb4b5c1eccfe00956ac318a2123bca9ef9ca

OK.

One (optional) comment:

[in Darcs.Test.Patch.Check]
> -- | PatchCheck is a state monad with a simulated repository state

> newtype PatchCheck a = PatchCheck { runPatchCheck :: MaybeT (State
>                                          ValidState) a }

It would help a bit to indicate in the haddock what the MaybeT
signifies, although it is visible by reading down just a little bit to
the throw/catch code. (Just = things are ok, Nothing = check failed)

>   * add changelog entry for release 2.14.3

OK - I shall assume it's accurate!

>   * darcs.cabal: remove redundant version constraints
>   * remove a redundant import from Setup.hs

Nice cleanups.

>   * no longer export fromJust from Darcs.Prelude [amended
>     from branch-2.14]
>   
>   While used in many places, fromJust is a partial function and we
>   should not encourage its unfettered use in Darcs.

OK.

>   * remove redundant imports
>   
>   This mostly concerns (<>) imported from Darcs.Util.Printer and (<$>) 
>   from Control.Applicative. These are now consistently exported from
>   Darcs.Prelude.

>   * resolve conflicts after 80223edc7108090f9c3231e36d4bdb94e1f52f6a

OK.


>   * import shelly-1.7.1 as a new subdirectory

[quoting from main body of submission:]
> This worked out remarkably well. However, it currently works only
> when building from a darcs clone, since the files in the shelly sub
> directory are not included in the cabal source dist.

So that means someone downloading from hackage and running the tests
will get an automatic failure? This doesn't seem ideal in case people
would want to do some kind of CI over all of hackage or something like
that. This does already happen though I suspect it wouldn't include
darcs' tests anyway.


>   * unwind tests: use condition compilation for MonadFail instances 
>     and imports

OK

>   * avoid use of fail in Darcs.Patch.Depends.unwrapOneTagged

OK

>   * more consequently throw MatchFailure exception
>   
>   This avoids two uses of fail in Darcs.Patch.Match in the context of
>   a monad tranformer that is not necessarily IO based.

OK (so I guess MatchFailure is an exception that signifies a user error
rather than an internal error)

>   * Darcs.Util.Prompt: add liftIO when calling fail 

OK. So now we don't care about the current Monad being MonadFail, just
that it is MonadIO.

>   * replace fail with error in Darcs.Repository.Diff
>   
>   This is the /only/ case where the "quick fix" of replacing fail with
>   error is semantically valid. Here we clearly have a partial pattern
>   match and it would be a bug if we hit that case.

OK.


>   * replace fail with throw.userError when applying a patch
>   
>   There are several uses of apply (and its variants) where we catch
>   IOExceptions e.g. maybeApplyToTree. This makes sense: if we apply in 
>   the context of IO (or a transformer over IO), then there may occur
>   any kind of IOException, not only the ones explicitly raised from
>   our code, and we want to catch them all.

So we're saying that if this code is called in a context that catches
IOExceptions, then these cases are normal/accepted. If it's not called
in a such a context then hitting them is an internal error in darcs
(like calling error - of course the Haskell name 'userError' is
confusing here)?

>   * replace fail with throw.userError in Tree monads and index 
>     matching
>   
>   The situation here is different from that of patch application.
>   Here we want the exception to percolate all the way up to the
>   top level handler.
>   In other words, these are regular failures similar to IO exceptions.

And these ones are always internal errors or at least unexpected
situations that are not errors in what the user requested?

>   * replace runhaskell with runghc in tests/renames.sh
>   
>   It seems runghc is better supported by ghc packagers than 
>   runhaskell.
>   This is not a complete solution, though. If cabal is used with
>   --with-compiler=/path/to/ghc, there is no guarantee that we have a 
>   runghc or a runhaskell in the PATH.

OK.

>   * darcs.cabal: remove containers from setup-depends

OK.

>   * raise upper bounds for base and Cabal dependencies
>   
>   This allows to build with ghc-8.8 and ghc-8.10.

OK. Though I am concerned about trying to support this many GHCs (8.2 to
8.10 inclusive) on an ongoing basis. Can we bump the minimum on HEAD as
soon as we release 2.16?
msg22230 (view) Author: bf Date: 2020-07-21.13:59:00
I could swear I did reply to the review but now I can see it neither in
my email nor on the tracker nor in my drafts folder. Strange.

>>   * fix license in darcs.cabal
> 
> OK (had to stop and check that we have always been GPL-2.0-or-later, I
> guess these things are easy to forget!)

I asked you about that one before making the change ;-)

>>   * for base >=4.13 define fail in MonadFail instances
>>   * resolve conflicts after 447f754fd0613f221cdf6174e67a2e020d71bdb6
>
> OK. Though :-( to CPP, of course it's pretty much unavoidable for the
> range of GHC versions wanted.

Addressed by

patch d559b09f9516251c3ad5bf9cb331f65767bab152
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul 19 16:34:54 CEST 2020
  * remove an unneeded LANGUAGE CPP pragma

and

patch 368451e02778df65951b7f8a024c2564e71426f7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul 19 16:35:54 CEST 2020
  * remove all MonadFail instances in darcs lib

> OK - I assume you will have battle-tested this.

Yes, I released 2.14.3 and 2.14.4 with it. Took quite some fiddling
until it worked to my satisfaction, but i think it's okay now.

>>   * mitigate issue2643 with a better error message
>>   
>>   If the patch index is corrupt, output the name of the currupt file
>>   and suggest its removal.
>>
>>   * resolve conflicts after 82fe27cc6295a2e55936b3b1ff10fcb4966e1e5e
> 
> Looks good. The user message might be clearer still if it said that it
> is always safe to remove the index but it's no big deal.

I felt this was implied, but feel free to re-word to make it more explicit.

>>   * clean up Darcs.Test.Patch.Check
>
> One (optional) comment:
> 
> [in Darcs.Test.Patch.Check]
>> -- | PatchCheck is a state monad with a simulated repository state
> 
>> newtype PatchCheck a = PatchCheck { runPatchCheck :: MaybeT (State
>>                                          ValidState) a }
> 
> It would help a bit to indicate in the haddock what the MaybeT
> signifies, although it is visible by reading down just a little bit to
> the throw/catch code. (Just = things are ok, Nothing = check failed)

Addressed by

patch cbf8512f7107db7f82579b448ccd7d7825e3ddd3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul 19 15:35:17 CEST 2020
  * improve docs for PatchCheck

>>   * import shelly-1.7.1 as a new subdirectory
> 
> [quoting from main body of submission:]
>> This worked out remarkably well. However, it currently works only
>> when building from a darcs clone, since the files in the shelly sub
>> directory are not included in the cabal source dist.
> 
> So that means someone downloading from hackage and running the tests
> will get an automatic failure? This doesn't seem ideal in case people
> would want to do some kind of CI over all of hackage or something like
> that. This does already happen though I suspect it wouldn't include
> darcs' tests anyway.

It is not ideal, which is why I tried to get rid of shelly in favour of
turtle. Unfortunately that failed to work on Windows.

The obvious "solution" is to add every source file from the bundled
shelly to our darcs.cabal. This would be a very ugly.

>>   * more consequently throw MatchFailure exception
>>   
>>   This avoids two uses of fail in Darcs.Patch.Match in the context of
>>   a monad tranformer that is not necessarily IO based.
> 
> OK (so I guess MatchFailure is an exception that signifies a user error
> rather than an internal error)

The general idea is that /all/ exceptions are user errors, except
ErrorCall. If an exception, for instance an IOError, should not lead to
failure of the command, then it must be caught.

>>   * Darcs.Util.Prompt: add liftIO when calling fail 
> 
> OK. So now we don't care about the current Monad being MonadFail, just
> that it is MonadIO.

See below.

>>   * replace fail with throw.userError when applying a patch
>>   
>>   There are several uses of apply (and its variants) where we catch
>>   IOExceptions e.g. maybeApplyToTree. This makes sense: if we apply in 
>>   the context of IO (or a transformer over IO), then there may occur
>>   any kind of IOException, not only the ones explicitly raised from
>>   our code, and we want to catch them all.
> 
> So we're saying that if this code is called in a context that catches
> IOExceptions, then these cases are normal/accepted. If it's not called
> in a such a context then hitting them is an internal error in darcs
> (like calling error - of course the Haskell name 'userError' is
> confusing here)?
> 
>>   * replace fail with throw.userError in Tree monads and index 
>>     matching
>>   
>>   The situation here is different from that of patch application.
>>   Here we want the exception to percolate all the way up to the
>>   top level handler.
>>   In other words, these are regular failures similar to IO exceptions.
> 
> And these ones are always internal errors or at least unexpected
> situations that are not errors in what the user requested?

The long comments for these two patches are pretty confusing. Sorry
about that. Please ignore them.

The idea here was to make the minimally invasive changes that kept the
behavior the same it was before, across different compiler/base-library
versions. The instance MonadFail IO throws a UserError and i believe
most monad transformers just lifted the fail method. The monads in this
case are Apply and TreeMonad, for which instances in Darcs proper (i.e.
outside of the test harness) are usually based on IO. The replacement
'throw . userError' is the closest approximation to the IO monad's
'fail' that I could think of.

Note that UserError is an IOException, which means that if code at lower
levels (like apply) throws a UserError, we can (and in some places do)
catch it, along with any other IOExceptions. If we do not catch them, it
is is a normal user error.

Throwing UserErrors in the instances of Apply and TreeMonad is not
ideal. This should be reviewed but that is pretty much out of scope for
this patch bundle.

>>   * raise upper bounds for base and Cabal dependencies
>>   
>>   This allows to build with ghc-8.8 and ghc-8.10.
> 
> OK. Though I am concerned about trying to support this many GHCs (8.2 to
> 8.10 inclusive) on an ongoing basis. Can we bump the minimum on HEAD as
> soon as we release 2.16?

I'd say we postpone that decision. Feel free to remind me when we have
done the release. I am personally not inconvenienced by this. I
occasionally run a loop like

for g in /opt/ghc/8.10.1/bin/ghc /opt/ghc/8.2.2/bin/ghc
/opt/ghc/8.4.4/bin/ghc /opt/ghc/8.6.5/bin/ghc /opt/ghc/8.8.2/bin/ghc; do
cabal build --disable-optimisation --enable-tests -fwarn-as-error
--with-compiler=$g || break
done

and less often also build with optimisations and run the test suite. I
haven't yet encountered any breakage with the newer compilers (after
applying this bundle, I am still using ghc-8.2.2 by default).
msg22257 (view) Author: ganesh Date: 2020-07-25.22:41:44
> The idea here was to make the minimally invasive changes that kept the
> behavior the same it was before, across different compiler/base-library
> versions. The instance MonadFail IO throws a UserError and i believe
> most monad transformers just lifted the fail method. The monads in this
> case are Apply and TreeMonad, for which instances in Darcs proper (i.e.
> outside of the test harness) are usually based on IO. The replacement
> 'throw . userError' is the closest approximation to the IO monad's
> 'fail' that I could think of.
> 
> Note that UserError is an IOException, which means that if code at lower
> levels (like apply) throws a UserError, we can (and in some places do)
> catch it, along with any other IOExceptions. If we do not catch them, it
> is is a normal user error.
> 
> Throwing UserErrors in the instances of Apply and TreeMonad is not
> ideal. This should be reviewed but that is pretty much out of scope for
> this patch bundle.

Thanks for the explanation. As you say with the immediate upgrade
problem fixed we can think about improving the overall situation in future.
History
Date User Action Args
2020-04-28 10:25:31bfcreate
2020-05-18 00:15:33bfsetmessages: + msg22018
2020-05-18 06:22:39ganeshsetmessages: + msg22019
2020-05-18 06:45:50bfsetstatus: needs-screening -> needs-review
2020-07-18 15:44:02ganeshsetstatus: needs-review -> review-in-progress
2020-07-18 16:03:09ganeshsetmessages: + msg22174
2020-07-21 13:59:04bfsetmessages: + msg22230
2020-07-25 22:41:45ganeshsetmessages: + msg22257
2020-07-25 22:41:58ganeshsetstatus: review-in-progress -> accepted-pending-tests
2020-07-25 23:17:28ganeshsetstatus: accepted-pending-tests -> accepted