Drop 7x #265

vdukhovni · 2020-08-23T07:53:12Z

With bytestring 0.11 no longer fully compatible with GHC prior to 8.0, drop support for GHC 7.x and older, by requiring base >= 4.9.
This also makes it possible to shed considerable CPP baggage.

sjakobi · 2020-08-23T21:37:04Z

The simplifications are nice, but I don't currently see sufficient motivation for removing support for all these GHC versions.

We could probably use this patch once a good reason for removing support for GHC < 8 comes up though.

vdukhovni · 2020-08-24T00:08:10Z

The simplifications are nice, but I don't currently see sufficient motivation for removing support for all these GHC versions.

We could probably use this patch once a good reason for removing support for GHC < 8 comes up though.

The "good reason" is that with #175 already merged we're no longer backwards compatible with GHC 7.x. Anything that uses Data.ByteString.Internal to access PS will fail to build. Since we don't know what packages do or don't do that, and don't have appropriate bounds, it is safer to just drop support for 7.x, and I think about time too.

sjakobi · 2020-08-24T07:55:27Z

The "good reason" is that with #175 already merged we're no longer backwards compatible with GHC 7.x. Anything that uses Data.ByteString.Internal to access PS will fail to build. Since we don't know what packages do or don't do that, and don't have appropriate bounds, it is safer to just drop support for 7.x, and I think about time too.

Lots of packages have features that are unavailable with certain older GHC versions. I've never heard that these would be a particular source of trouble. And any trouble can be safely detected at compile-time too!

In other packages that I have contributed to, the usual reason for cutting GHC support for older versions was that continued support would have incurred large maintenance costs. I don't see such problems yet.

vdukhovni · 2020-08-24T08:40:16Z

The problem here is that many dependencies may depend on bytestring and yet not have sufficiently tight version bounds. Now one will have to work around these. How of the ecosystem will in any case fail to build against 0.11 with GHC 7.x? That would all packages that use Data.ByteString.Internal.PS and all their dependencies. This probably includes things like conduit, ...

Given even packages that will require > 7.x to use the new bytestring it seems no loss to just admit it, and take advantage of it.
Does anyone have the tools to quickly check hackage for use of PS?

Bodigrim · 2020-08-24T17:18:20Z

If a package still suports GHC < 8.0 and depends on ByteString internals and do not use upper bounds, its maintainers can add a conditional bound as a revision. Does not look disruptive to me.

  if impl(ghc < 8.0)
    build-depends: bytestring < 0.11
  else
    build-depends: bytestring

People can either add an upper bound, or drop support of older GHCs, or drop support of older bytestring, or even write separate code to support both 0.10 and 0.11 checking #if MIN_VERSION_bytestring(0,11,0). This decision is up to downstream package maintainers.

Moreover, there is a lot of packages, which do not rely on internals at all. I guesstimate that the vast majority of bytestring users don't. It seems wrong to prohibit them building on old GHCs just for no reason.

BTW conduit has dropped GHC < 8.0 years ago.

I mean, I do not mind to drop support of old GHCs in principle. But it seems that PRs, which we intend to merge in 0.11.0.0, have already bitten the bullet and ironed out all nitty-gritty CPP details to work fine with GHCs back to 7.0.

ethercrow · 2020-11-17T21:04:43Z

Dropping GHC 7.* would enable full migration of CI to GitHub Actions. This might be desirable because of https://www.reddit.com/r/haskell/comments/jvbs49/psa_maintainers_should_think_about_migrating_off/

vdukhovni · 2020-11-17T21:24:45Z

Dropping GHC 7.* would enable full migration of CI to GitHub Actions. This might be desirable because of https://www.reddit.com/r/haskell/comments/jvbs49/psa_maintainers_should_think_about_migrating_off/

Perhaps needless to say, since I proposed this PR, I agree with this reasoning, and support dropping 7.x in any case, since I see the burden of ongoing support for no-longer supported GHC versions as outweighing the benefit. Supporting 7.x also holds us back from taking full advantage of some key improvements in 8.x and beyond.

sjakobi · 2020-11-17T22:06:06Z

Dropping GHC 7.* would enable full migration of CI to GitHub Actions. This might be desirable because of https://www.reddit.com/r/haskell/comments/jvbs49/psa_maintainers_should_think_about_migrating_off/

What's the problem with GHC 7.* on GitHub CI?

Personally, I still don't feel a large maintenance burden for supporting these old GHC versions in bytestring. That's no surprise though, since I currently don't work on the code myself.

That said, I haven't heard of any demand for continued support for GHC 7 for a while, so I'm not opposed to removing it at this stage.

ethercrow · 2020-11-17T22:43:24Z

What's the problem with GHC 7.* on GitHub CI?

The oldest version available there is 7.10. We could probably install older versions by jumping through enough hoops, but I'd rather not.

Bodigrim · 2021-02-17T18:23:58Z

Since bytestring-0.11.1.0 is out, let's finally do it. @vdukhovni would you be interested to rebase? Alternatively I can raise a new PR myself.

vdukhovni · 2021-02-21T06:29:32Z

Since bytestring-0.11.1.0 is out, let's finally do it. @vdukhovni would you be interested to rebase? Alternatively I can raise a new PR myself.

Done. Will attend to CI issues now.

Also shed much CPP baggage and a no longer needed FFI helper.

vdukhovni · 2021-02-21T07:33:27Z

@Bodigrim Looks like CI is going to pass (just FreeBSD left to do), but I'm entirely unfamiliar with GitHub CI, just cargo-culting the prior config. Please suggest changes. In particular is any of the "ghcprep" stuff needed? Can more of the dependency builds be cached? ... Bottom line, feel free to push a fixup commit for the CI bits...

vdukhovni · 2021-02-21T17:50:27Z

Thanks for the CI cleanup!

vdukhovni · 2021-02-21T18:04:21Z

Once this is merged, only 7 of the source files in Data/ByteString will have CPP directives, down from 19 (all of them) previously!

vdukhovni · 2021-02-22T03:03:37Z

@sjakobi, in case this saves you some review time, the remaining #if conditions on either __GLASGOW_HASKELL__ or MIN_VERSION_base are:

$ git grep -E '__GLASGOW_HASKELL__|MIN_VERSION_base'
Data/ByteString/Builder/ASCII.hs:#if __GLASGOW_HASKELL__ >= 811
Data/ByteString/Builder/Internal.hs:#if !(MIN_VERSION_base(4,11,0))
Data/ByteString/Internal.hs:#if MIN_VERSION_base(4,13,0)
Data/ByteString/Internal.hs:#if __GLASGOW_HASKELL__ < 900
Data/ByteString/Internal.hs:#if MIN_VERSION_base(4,10,0)
Data/ByteString/Internal.hs:#if __GLASGOW_HASKELL__ >= 811
Data/ByteString/Internal.hs:#if (__GLASGOW_HASKELL__ < 802) || (__GLASGOW_HASKELL__ >= 811)
Data/ByteString/Internal.hs:#if MIN_VERSION_base(4,15,0)
Data/ByteString/Internal.hs:#if !MIN_VERSION_base(4,15,0)
Data/ByteString/Internal.hs:#if !MIN_VERSION_base(4,10,0)
Data/ByteString/Internal.hs:#if __GLASGOW_HASKELL__ >= 802
Data/ByteString/Internal.hs:#if __GLASGOW_HASKELL__ >= 811
Data/ByteString/Internal.hs:#if __GLASGOW_HASKELL__ >= 811
Data/ByteString/Internal.hs:#if __GLASGOW_HASKELL__ >= 811
Data/ByteString/Lazy/Internal.hs:#if MIN_VERSION_base(4,13,0)
Data/ByteString/Short/Internal.hs:#if MIN_VERSION_base(4,10,0)
Data/ByteString/Short/Internal.hs:#if MIN_VERSION_base(4,10,0)

Since our minima are __GLASGOW_HASKELL == 800 and MIN_VERSION_base(4,9,0), the above are all still potentially falsifiable.

vdukhovni · 2021-02-22T03:11:33Z

The odd looking < 802 || >= 811 condition is because the Int constructor I# is used either if !MIN_VERSION_base(4,10,0) or GHC >= 811. Perhaps the conditional import:

#if (__GLASGOW_HASKELL__ < 802) || (__GLASGOW_HASKELL__ >= 811)
import GHC.Types                (Int (..))
#endif

should use exactly the same conditions as the associated code? In which case (not due to my PR, but could be improved now), the < 802 condition should really be !MIN_VERSION_base(4,10,0) matching the code fragment:

#if !MIN_VERSION_base(4,10,0)
-- |Advances the given address by the given offset in bytes.
--
-- The new 'ForeignPtr' shares the finalizer of the original,
-- equivalent from a finalization standpoint to just creating another
-- reference to the original. That is, the finalizer will not be
-- called before the new 'ForeignPtr' is unreachable, nor will it be
-- called an additional time due to this call, and the finalizer will
-- be called with the same address that it would have had this call
-- not happened, *not* the new address.
plusForeignPtr :: ForeignPtr a -> Int -> ForeignPtr b
plusForeignPtr (ForeignPtr addr guts) (I# offset) = ForeignPtr (plusAddr# addr offset) guts
{-# INLINE [0] plusForeignPtr #-}
{-# RULES
"ByteString plusForeignPtr/0" forall fp .
   plusForeignPtr fp 0 = fp
 #-}
#endif

sjakobi

Over 400 lines removed! Awesome! 🎉

sjakobi · 2021-02-22T09:57:08Z

.github/workflows/ci.yml

@@ -89,7 +55,6 @@ jobs:
      run: cabal haddock

  build-freebsd:
-    needs: build


Why can we remove the build step here?

It was just a guard to avoid launching the FreeBSD build when other builds fail, it is optional. Not sure why @Bodigrim removed it, but it is optional. Removing it likely makes CI complete faster when all goes well, but this may mean that the FreeBSD build runs even on broken PRs.

hs-viktor · 2021-02-22T18:45:57Z

The odd looking < 802 || >= 811 condition is because the Int constructor I# is used either if !MIN_VERSION_base(4,10,0) or GHC >= 811. Perhaps the conditional import:

#if (__GLASGOW_HASKELL__ < 802) || (__GLASGOW_HASKELL__ >= 811)
import GHC.Types                (Int (..))
#endif

should use exactly the same conditions as the associated code? In which case (not due to my PR, but could be improved now), the < 802 condition should really be !MIN_VERSION_base(4,10,0) matching the code fragment:

#if !MIN_VERSION_base(4,10,0)
-- |Advances the given address by the given offset in bytes.
--
-- The new 'ForeignPtr' shares the finalizer of the original,
-- equivalent from a finalization standpoint to just creating another
-- reference to the original. That is, the finalizer will not be
-- called before the new 'ForeignPtr' is unreachable, nor will it be
-- called an additional time due to this call, and the finalizer will
-- be called with the same address that it would have had this call
-- not happened, *not* the new address.
plusForeignPtr :: ForeignPtr a -> Int -> ForeignPtr b
plusForeignPtr (ForeignPtr addr guts) (I# offset) = ForeignPtr (plusAddr# addr offset) guts
{-# INLINE [0] plusForeignPtr #-}
{-# RULES
"ByteString plusForeignPtr/0" forall fp .
   plusForeignPtr fp 0 = fp
 #-}
#endif

Should I open a PR to change the CPP guard for the import of GHC.Types?

hs-viktor · 2021-02-22T18:46:26Z

Or one of you can do that at your leisure...

Bodigrim · 2021-02-22T18:48:48Z

It would be nice, if you have time, but low priority.

hs-viktor · 2021-02-22T18:50:30Z

Understood, not sure when I'll get to it, so feel free to bundle the one-liner into any other PR you happen to already be working on, or beat me to opening one just for this change...

* Drop support for GHC versions before 8.0, base 4.9 Also shed much CPP baggage and a no longer needed FFI helper. * Drop redundant CI jobs * Restore cabal file Co-authored-by: Viktor Dukhovni <[email protected]> Co-authored-by: Bodigrim <[email protected]>

vdukhovni mentioned this pull request Aug 23, 2020

Release planning #241

Closed

vdukhovni force-pushed the drop-7x branch from bffc23d to 65eacfb Compare August 23, 2020 08:17

Bodigrim mentioned this pull request Aug 25, 2020

Use qsort to sort short ByteString #267

Merged

vdukhovni force-pushed the drop-7x branch from 65eacfb to 0b283a4 Compare February 21, 2021 06:28

vdukhovni force-pushed the drop-7x branch 2 times, most recently from ed23272 to 9799a96 Compare February 21, 2021 06:54

Drop support for GHC versions before 8.0, base 4.9

20dae1c

Also shed much CPP baggage and a no longer needed FFI helper.

vdukhovni force-pushed the drop-7x branch from 9799a96 to 20dae1c Compare February 21, 2021 07:12

Bodigrim added 2 commits February 21, 2021 17:25

Drop redundant CI jobs

c879262

Restore cabal file

d425b2f

Bodigrim approved these changes Feb 21, 2021

View reviewed changes

Bodigrim requested a review from sjakobi February 21, 2021 17:52

Bodigrim mentioned this pull request Feb 22, 2021

Implement floating point conversion with ryu #365

Merged

sjakobi approved these changes Feb 22, 2021

View reviewed changes

sjakobi mentioned this pull request Feb 22, 2021

GHC backward compatibility policy / support for GHC 7.6 haskell/containers#699

Closed

Bodigrim merged commit 784cdd7 into haskell:master Feb 22, 2021

sjakobi mentioned this pull request Jul 24, 2021

Compatibility with transformers 0.6 pcapriotti/optparse-applicative#427

Closed

Bodigrim added this to the 0.11.2.0 milestone May 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop 7x #265

Drop 7x #265

vdukhovni commented Aug 23, 2020

sjakobi commented Aug 23, 2020

vdukhovni commented Aug 24, 2020

sjakobi commented Aug 24, 2020

vdukhovni commented Aug 24, 2020

Bodigrim commented Aug 24, 2020

ethercrow commented Nov 17, 2020

vdukhovni commented Nov 17, 2020

sjakobi commented Nov 17, 2020

ethercrow commented Nov 17, 2020

Bodigrim commented Feb 17, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 22, 2021

vdukhovni commented Feb 22, 2021 •

edited

Loading

sjakobi left a comment

sjakobi Feb 22, 2021

vdukhovni Feb 22, 2021 •

edited

Loading

hs-viktor commented Feb 22, 2021

hs-viktor commented Feb 22, 2021

Bodigrim commented Feb 22, 2021

hs-viktor commented Feb 22, 2021

Drop 7x #265

Drop 7x #265

Conversation

vdukhovni commented Aug 23, 2020

sjakobi commented Aug 23, 2020

vdukhovni commented Aug 24, 2020

sjakobi commented Aug 24, 2020

vdukhovni commented Aug 24, 2020

Bodigrim commented Aug 24, 2020

ethercrow commented Nov 17, 2020

vdukhovni commented Nov 17, 2020

sjakobi commented Nov 17, 2020

ethercrow commented Nov 17, 2020

Bodigrim commented Feb 17, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 21, 2021

vdukhovni commented Feb 22, 2021

vdukhovni commented Feb 22, 2021 • edited Loading

sjakobi left a comment

Choose a reason for hiding this comment

sjakobi Feb 22, 2021

Choose a reason for hiding this comment

vdukhovni Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

hs-viktor commented Feb 22, 2021

hs-viktor commented Feb 22, 2021

Bodigrim commented Feb 22, 2021

hs-viktor commented Feb 22, 2021

vdukhovni commented Feb 22, 2021 •

edited

Loading

vdukhovni Feb 22, 2021 •

edited

Loading