Closed sjakobi closed 2 years ago
One common thing about (at least 6 first packages) is that they are importing mtl modules unqualified:
https://github.com/haskell-hvr/HsYAML/blob/c9cd9898f839c1f2108148649e8a127c23ba3f0f/src/Data/YAML/Loader.hs#L19 https://github.com/snowleopard/alga/blob/1623fae605503595ee331929157c61b5e5e0c1c7/src/Algebra/Graph/NonEmpty.hs#L57 https://github.com/chrisdone/lucid/blob/998ddcfa350134bcb92398bb8ccadfb32b6fc873/lucid1/src/Lucid/Base.hs#L49 https://github.com/haskell-hvr/regex-tdfa/blob/5919e9c1e23486b5a48f29989ec8898fbf74c643/lib/Text/Regex/TDFA/CorePattern.hs#L37 https://github.com/UnkindPartition/tasty/blob/master/core/Test/Tasty/Patterns/Eval.hs#L5
released happy:
> import Control.Monad.Writer
logict
is fixed in https://github.com/Bodigrim/logict/commit/7c217cdecdb84dcfc39b43171bb8f6d7827663d1#diff-e9956f814b9685cfcc940660acc036a864f1f52389476632361b19e25fab69d2
One common thing about (at least 6 first packages) is that they are importing mtl modules unqualified:
Yes, of course, this is established practice.
established bad practice.
Emphasis mine. But as I said in the PR #99, it's probably better to revert that and forget about the cleanup of #68.
Mmh, if migration is as simple as adding extra imports, like
import Control.Monad
import Data.Monoid (...)
I personally would not mind adding these. But it would feel unnatural to discontinue unqualified import of monad transformers, like
import Control.Monad.Except
import Control.Monad.Reader
import Control.Monad.State
import Control.Monad.Writer
because the names brought into scope are distinct enough (ReaderT
, runReaderT
, MonadReader
, evalState
, etc.).
@andreasabel Yes, you could continue to to use unqualified imports, and the #99 change makes that less dangerous, by making these modules export less (e.g. all of Data.Monoid
).
I'd still advise against. E.g. HLS makes it quite easy to add explicit import lists after initial development is done. (Or you may use internal prelude and import these modules once and re-export for your package, then you get both: the control of what is in scope and convenience).
IMO unqualified imports are a wart of Haskell. Nice for one-off little scripts, not good for "made to last" software. Unqualified imports make adding new names potentially breaking change. And PVP explicitly mentions and advices against: See Backwards compatible client specification. in https://pvp.haskell.org/. So if you do
import Control.Monad.Except
you should have
build-depends: mtl <2.2.3
but no one does that unfortunately. (e.g. lucid
and tasty
don't have upper bounds at all)
And I admit the defeat. Trying to make people follow PVP practices is just not gonna happen.
For reference, @phadej and I have discussed this issue a bit further in #99.
I've realized that I don't particularly care about this issue, because I don't need the new features in transformers-0.6
.
I do expect though, that if transformers-0.6
is used in GHC 9.4, the breaking changes in mtl
will probably inhibit adoption of GHC 9.4. I think the mtl
maintainers should take this into account.
cc @emilypi let's schedule a meeting to discuss this
On Wed, Dec 1, 2021, 09:39 Simon Jakobi @.***> wrote:
For reference, @phadej https://github.com/phadej and I have discussed this issue a bit further in #99 https://github.com/haskell/mtl/pull/99.
I've realized that I don't particularly care about this issue, because I don't need the new features in transformers-0.6.
I do expect though, that if transformers-0.6 is used in GHC 9.4, the breaking changes in mtl will probably inhibit adoption of GHC 9.4. I think the mtl maintainers should take this into account.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haskell/mtl/issues/101#issuecomment-983763914, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEOIX27GHVSWBXPQE4RE7YTUOY6RVANCNFSM5ILAYSLQ .
So, I suppose everyone is in favour of reverting the no re-export change and then just cutting a release? cc @emilypi
I'd note that until that reverting (i.e. un-fixing #68) probably means that Control.Monad
, Data.Monoid
, Control.Monad.Fix
in base
should be treated akin to Prelude
(i.e. changes in these are very controvercial). cc @Bodigrim
The explicit re-export lists as @sjakobi suggested is middle-ground. Less bad than open re-export.
It would help to have a clear strategy statement from maintainers. Do mtl
maintainers want this change to happen (now or in future) despite its impact? #74 suggests that the answer is yes, but that discussion might be outdated.
Here are my thoughts in a personal capacity, feel free to ignore.
I'd release mtl-2.3
without #99 and (if there is a consensus that it is a desired change to happen) immediately follow up with mtl-2.4
with #99. Update GHC submodule to mtl-2.3
, but not to mtl-2.4
yet.
The rationale is that it mtl-2.3
is an important release to catch up with transformers
, which is long overdue. It is already breaking enough, and people are unable to test it at the moment, because #99 breaks almost every consumer in a long chain of dependencies. Reverting #99 would give users a decent way to test and migrate over ListT
/ ErrorT
changes, so that mtl
is not a stumbling block by the time GHC 9.4 is released.
Having mtl-2.4
released, but not forced onto users (by bumping GHC submodule) would provide a gentle push and opportunity for maintainers to upgrade in a due course. For example, they'll be able to setup Haskell-CI workflow with mtl
pinned to 2.4, overrriding an installed boot package. Required changes are backward-compatible and does not need CPP, which is awesome, but it's worth to give time to the ecosystem to accomodate them. Once key packages are compatible, one can push mtl-2.4
as a submodule for GHC 9.6.
That feels like kicking everything twice, especially if a package needs changes due both, removed ErrorT
and removed re-exports.
If GHC-9.4 is to be released in 5 months at earliest (?), why wait with re-export removal breakage.
That feels like kicking everything twice, especially if a package needs changes due both
If both mtl-2.3
and mtl-2.4
are released at the same time, a motivated maintainer can do both changes at once. I am personally not a fan of cliff edges when you must upgrade half of the world just to accomodate transformers-0.6
. Five months are not that much, especially given that I do not see any migration guide or a loud announcement of this change except a tiny changelog line.
Essentially it's a judgement call, how much breakage in which period of time maintainers are comfortable with. I do not really want to interfere with their decision uninvited, but on the other hand I'd love to hear it rather sooner than later; delaying a release indefinitely does not make anything better at all.
I think I am leaning toward reverting and just being in violation of PVP for now. I never imagined so much of the ecosystem would use MTL imports in this way, which I consider poor form, but others don't seem to mind that much.
What do others think? @sjakobi @phadej @Bodigrim @emilypi
On Sun, Jan 9, 2022, 06:46 Bodigrim @.***> wrote:
That feels like kicking everything twice, especially if a package needs changes due both
If both mtl-2.3 and mtl-2.4 are released at the same time, a motivated maintainer can do both changes at once. I am personally not a fan of cliff edges when you must upgrade half of the world just to accomodate transformers-0.6. Five months are not that much, especially given that I do not see any migration guide or a loud announcement of this change except a tiny changelog line.
Essentially it's a judgement call, how much breakage in which period of time maintainers are comfortable with. I do not really want to interfere with their decision uninvited, but on the other hand I'd love to hear it rather sooner than later; delaying a release indefinitely does not make anything better at all.
— Reply to this email directly, view it on GitHub https://github.com/haskell/mtl/issues/101#issuecomment-1008291143, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEOIX2Z3746H7EAHIZ44QLTUVF7SNANCNFSM5ILAYSLQ . You are receiving this because you commented.Message ID: @.***>
I do like @Bodigrim's suggestion from https://github.com/haskell/mtl/issues/101#issuecomment-1005139255. Making a first release with #99 reverted would be a good first step in any case. :+1:
I understand technical merits of #99, and indeed things like replicate
being imported by Control.Monad.Reader
are fishy, but it is also a huge-huge breaking change (in the name of potentially less breaking changes in future). It's not my burden to facilitate migration and respond to user's feedback, so it would be wrong for me to advice on strategy. Otherwise my take is in https://github.com/haskell/mtl/issues/101#issuecomment-1005139255.
I want to remind about the middle-ground: At least make the re-exports explicit, and fixed.
It's not that easy to do as just reverting: someone have to write code "new" code, but it's not much work either: e.g. make a other-modules
named MonadReexports
with an explicit re-export list, and re-export that one instead of Control.Monad
, Data.Monoid
etc.
(If you ask me to do it, I'll ask to be become mtl
maintainer...)
@chessai @emilypi GHC 9.4 feature freeze is upcoming in February. It would be nice to release mtl-2.3
with transformers-0.6
support, so that both can be updated in GHC source tree.
I'm happy to report that all of the packages I mentioned in the issue description build without errors with mtl-2.3-rc4
. Some may already have been patched, but the majority seem to have been fixed via #103.
Need this issue be reopened, because of https://github.com/haskell/mtl/pull/103#issuecomment-1024522023 ?
Tracking breakage and fixes:
[x] HTTP-4000.3.16
https://github.com/haskell/HTTP/issues/139 (https://github.com/haskell/HTTP/pull/142)
[ ] random-1.2.1
https://github.com/haskell/random/issues/126 (https://github.com/haskell/random/issues/125)
[4 of 4] Compiling System.Random.Stateful ( src/System/Random/Stateful.hs, dist/build/System/Random/Stateful.o, dist/build/System/Random/Stateful.dyn_o )
src/System/Random/Stateful.hs:289:22: error:
• Variable not in scope: replicateM :: Int -> m0 a0 -> m [a]
• Perhaps you meant ‘replicate’ (imported from Prelude)
|
289 | uniformListM n gen = replicateM n (uniformM gen)
| ^^^^^^^^^^
Error: cabal: Failed to build random-1.2.1 (which is required by
test:shelly-testsuite from shelly-1.10.0).
[ ] Agda-2.6.2.1
https://github.com/agda/agda/pull/5798 (agda/agda@92c871c
(#5798) 77 files changed +305 −132 lines)
Took me 5 hours to fix the imports in 77 files (305 loc).
@Bodigrim's take (https://github.com/haskell/mtl/issues/101#issuecomment-1005139255) seems best: Decouple transformers-0.6
adoption (uncontroversial) from the big breakage (controversial).
Yes, there is some breakage also due to the removal of ErrorT
and ListT
, but there have been deprecation warnings for years, so this should come to no-ones surprise.
@sjakobi : Will you reopen this one or shall I create a new issue to track breakage and fixes with RC3?
Yeah, feel free to use this issue for tracking breakage and fixes.
@emilypi @chessai Given the merging of #108, is this issue stale?
Yes, thanks @kozross
@kozross, @emilypi What do you mean with "stale"? Did you find a way to reduce the breakage reported in this issue?
I can actually reproduce many of the compilation errors I reported with the mtl-2.3 release.
I've used my Hackage trustee powers to add mtl < 2.3
bounds to all the packages listed above that needed them.
Many don't have patches to add support for mtl-2.3 yet though:
lucid
(bounds issue): https://github.com/chrisdone/lucid/commit/10b93cff4b2d447a552b583e8914170250be35a6happy
(bounds issue): https://github.com/haskell/happy/pull/237pattern-arrows
(bounds issue)aeson-better-errors
(bounds issue)mfsolve
(bounds issue)monad-par
(bounds issue)EDIT: More packages that need patches:
microlens-mtl
(https://github.com/monadfix/microlens/issues/151)tasty-silver
(https://github.com/phile314/tasty-silver/issues/35)resourcet
(https://github.com/snoyberg/conduit/issues/484), PR: https://github.com/snoyberg/conduit/pull/485x509
(https://github.com/haskell-tls/hs-certificate/issues/129)conduit-parse
(https://github.com/k0ral/conduit-parse/issues/4)lsp
(https://github.com/haskell/lsp/issues/430)Another broken package that is blocking many other packages is x509
: https://github.com/haskell-tls/hs-certificate/issues/129
To put it bluntly, bounds problems on other packages aren't my problem. they should use upper bounds. I'm not going to stop you from doing work you feel you need to do, but maintainers should take responsibility for their packages and do the patch work themselves. I don't want you to burn out because they don't want to add an upper bound or maintain their packages.
No worries, @emilypi! :) I know I don't have to fix bounds on other people's packages.
Here are some build errors that I've noticed:
HsYAML
(PR):algebraic-graphs
(note: bounds seem sufficient):happy
(bounds are fixed, needs patch):lucid
(bounds are fixed, needs patch):regex-tdfa
(bounds are sufficient, no compatible version yet):tasty
(patch):EDIT: More breakage
pattern-arrows
(bounds are fixed):aeson-better-errors
(bounds are fixed, needs patch):mfsolve
(bounds are fixed, needs patch):bytes
(bounds seem sufficient, has compatible release):logict
(bounds seem sufficient, no compatible version yet):monad-par
(bounds are fixed, needs patch):EDIT 2: Even more breakage
repline
(bounds are sufficient, PR):I wonder whether it might be better to continue to re-export a selected (stable) set of combinators and possibly also classes like
MonadIO
andMonadFix
.For combinators like
liftM2
,when
or(<=<)
, I don't see how this could do any harm by causing compatibility issues down the road.In the case of classes like
MonadIO
andMonadFix
I'm less sure, but even for these it might be no worse to cause the breakage once it's warranted by upstream changes on these classes themselves.What do you think?