Skip to content
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

Final touches for 0.11.0.0 #287

Merged
merged 3 commits into from
Sep 23, 2020
Merged

Conversation

Bodigrim
Copy link
Contributor

  1. Since we do not test against GHC 6.12, it is better to stop listing it as a supported platform (cf. discussion in Restrict base bounds for existing bytestring releases? #285). Thus, I changed GHC 6.12 to 7.0 in README.md and bumped the lower bound for base from 4.2 to 4.3. However, I decided to retain if impl(ghc < 7) section still - it has a chance to appear useful for a daring soul. Dunno, maybe it is better to strip it as well.

  2. Following discussions elsewhere, I clarified statements about compatibility issues in Changelog.md.

  3. I removed cabal-install from build requirements: stack or whatever can be used as well. I also removed paragraphs about building and testing: we certainly do not want people to use cabal install (at the very least it should be cabal build), and cabal test does not run any tests, because they are in a separate package. I'd appreciate if someone invest time into writing a proper modern contributors guide, but at least we should avoid misleading instructions.

  4. I added badges to track versions of bytestring available from Hackage and Stackage (LTS and nightly). Given that currently they point to three different versions, I find it very useful to have this information at hand.

@Bodigrim Bodigrim added this to the 0.11.0.0 milestone Sep 18, 2020
Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I'm still trying to test the new release with pump, so I might have more insights regarding the breakage this evening.

@sjakobi
Copy link
Member

sjakobi commented Sep 20, 2020

I'm still trying to test the new release with pump, so I might have more insights regarding the breakage this evening.

Here are some build failures due to the changes in the new release that I've found by building bytestring's top 500 reverse dependencies. There might be more failures, but I've been struggling with analyzing the pump build report, and a lot of packages also failed to build for unrelated reasons.

In the case of the removal of the Data.ByteString.Lazy.Builder modules, I've been wondering whether we should possibly extend the deprecation period first. The DEPRECATED pragma was added only in 0.10.12.0. I don't feel very strongly about this though.

EDIT: For my own reference, this is the jq command I used for filtering the pump realise output:

$ cat report.json | jq 'map(select(.phase == "Building" and .exitCode == 1 and (.stderr | contains("Cabal") or contains("Cannot test") or contains("Could not resolve dep") or contains("foreign libr") or contains("include file") or contains("System.Random") or contains("(visible) method") | not )))'

@sjakobi
Copy link
Member

sjakobi commented Sep 20, 2020

Due to removal of Data.ByteString.Lazy.Builder:

  • modern-uri
  • uuid-types
  • bytestring-conversion
  • proto-lens
  • rebase
  • text-printer

I should add that, of these packages, only bytestring-conversion and text-printer have insufficient bounds on bytestring.

@Bodigrim
Copy link
Contributor Author

@sjakobi thanks for a thorough investigation, much appreciated! IMHO we are good to go.

I would strongly prefer to grab a chance and remove Data.ByteString.Lazy.Builder, because otherwise we would not be able to do it until the next major release. [It was probably unwise to rename it in the first place, but] having it lingering around for another decade would be an abomination. I believe that breaking 8 packages in 8 years is acceptable, and I'm happy to raise PRs with relevant fixes.

@sjakobi
Copy link
Member

sjakobi commented Sep 20, 2020

@Bodigrim I don't object. I think the deprecation process for these modules was less than ideal. Removing them now instead of dragging them on for years seems like the right trade-off to me. 👍

@phadej
Copy link
Contributor

phadej commented Sep 21, 2020

I should add that, of these packages, only bytestring-conversion and text-printer have insufficient bounds on bytestring.

👍


[![Build Status](https://secure.travis-ci.org/haskell/bytestring.png?branch=master)](http://travis-ci.org/haskell/bytestring)
[![Build Status](https://secure.travis-ci.org/haskell/bytestring.svg?branch=master)](http://travis-ci.org/haskell/bytestring) [![Hackage](http://img.shields.io/hackage/v/bytestring.svg)](https://hackage.haskell.org/package/bytestring) [![Hackage CI](https://matrix.hackage.haskell.org/api/v2/packages/bytestring/badge)](https://matrix.hackage.haskell.org/package/bytestring) [![Stackage LTS](http://stackage.org/package/bytestring/badge/lts)](http://stackage.org/lts/package/bytestring) [![Stackage Nightly](http://stackage.org/package/bytestring/badge/nightly)](http://stackage.org/nightly/package/bytestring)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please remove the Stackage badges? They serve no benefit for a GHC bundled package such as bytestring and only clutter the README with useless information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually find such information useful. Otherwise I have to keep in mind 1) which version of GHC is available from Stackage today, 2) the exact mapping between GHC (including minor releases) and bytestring versions. It is quite convenient to see at a glance how far Stackage is lagging back. For example, a user of Stack may very well consider adding bytestring-0.11 to extra-deps, if he/she learns that Stackage is one major version behind.

@@ -1,8 +1,8 @@
[0.11.0.0] — September 2020
* [Change internal representation of `ByteString`, removing offset](https://github.com/haskell/bytestring/pull/175)
* The old `PS` constructor has been turned into a pattern synonym that is available with GHC >= 8.0 for backwards compatibility.
* The old `PS` constructor has been turned into a pattern synonym that is available with GHC >= 8.0 for backwards compatibility. Consider adding `if !impl(ghc >=8.0) { build-depends: bytestring < 0.11 }` to packages, which use `PS` and still support GHC < 8.0.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The old `PS` constructor has been turned into a pattern synonym that is available with GHC >= 8.0 for backwards compatibility. Consider adding `if !impl(ghc >=8.0) { build-depends: bytestring < 0.11 }` to packages, which use `PS` and still support GHC < 8.0.
* The old `PS` constructor has been turned into a pattern synonym that is available with GHC >= 8.0 for backwards compatibility. Consider adding `if impl(ghc < 8.0) { build-depends: bytestring < 0.11 }` to packages, which use `PS` and still support GHC < 8.0.

If < is also supported (IIRC it is), I think it may be better to avoid the negation...

Copy link
Contributor

@phadej phadej Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. The !impl(ghc >= 8.0) and impl(ghc < 8.0) are different. Latter is true only when implementation is GHC, so the former is more correct:

If implementation is not GHC (e.g. Hugs), and particularly not GHC-8.0 or later.

@Bodigrim Bodigrim merged commit d79b997 into haskell:master Sep 23, 2020
@Bodigrim Bodigrim deleted the release-0.11.0.0 branch September 23, 2020 19:36
@Bodigrim Bodigrim mentioned this pull request Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants