Changes for GHC 9.0

[bgamari]

This includes changes rewriting uses of withForeignPtr to unsafeWithForeignPtr where appropriate for GHC 9.0.

[Bodigrim]

@bgamari please resolve merge conflicts.

[bgamari]

I'm happy to rebase but do note that this will ultimately also need to be backported to the 0.10 branch.

[bgamari]

I have pushed the old branch (fixing 0.10) to https://github.com/bgamari/bytestring/tree/wip/no-fptr-0.10.

[Bodigrim]

I'm happy to rebase but do note that this will ultimately also need to be backported to the 0.10 branch.

@bgamari I'm very reluctant to resurrect bytestring-0.10 series. What's exactly holding GHC back from switching to bytestring-0.11 branch?

[bgamari]

I'm happy to rebase but do note that this will ultimately also need to be backported to the 0.10 branch.

@bgamari I'm very reluctant to resurrect bytestring-0.10 series. What's exactly holding GHC back from switching to bytestring-0.11 branch?

It's simply too late in the release cycle for a change like this; we already released the 9.0 alpha (which is supposed to be at least mostly interface-stable) with 0.10 two months ago. If we were discussing a minor bump then this would be an option, but doing a major bump at this point is far too much work (since we would need to update a non-trivial portion of Hackage up-to-update to test via head.hackage) and would pose yet another migration challenge.

[Bodigrim]

The difference between interfaces of bytestring-0.11 and 0.10 is far smaller than GHC 9.0 vs 9.0 alpha (for instance, there were non-negligible changes in ghc-bignum). I'm happy to help with head.hackage revisions, but actually for the purpose of testing against GHC 9.0 simple --allow-newer should do the trick almost always. Breaking changes in bytestring-0.11 are pretty much limited to GHC < 8.0.

[bgamari]

The difference between interfaces of bytestring-0.11 and 0.10 is far smaller than GHC 9.0 vs 9.0 alpha (for instance, there were non-negligible changes in ghc-bignum).

The changes in bytestring-0.11 are quite significant and will affect a significant number of packages. Even the removal of Data.ByteString.Lazy.Builder alone will require a non-trivial amount of effort for downstream users to account for (for better or worse, users tend to ignore deprecation warnings). This isn't to say that the changes are bad -- 0.11 looks like a great release! --- but we simply cannot retarget the entire ecosystem onto it overnight. Even simply bumping all of the core libraries' bounds to allow 0.11 in the install plan would realistically take a several weeks.

GHC is a large project with a large number of dependencies and a very large impact on the ecosystem; a task like bumping bytestring takes time. Adding another month to a release which is already two months late is simply not that we can afford.

I'm happy to bump bytestring for 9.2, but the window is long-closed for 9.0.

[Bodigrim]

Even the removal of Data.ByteString.Lazy.Builder alone will require a non-trivial amount of effort for downstream users to account for (for better or worse, users tend to ignore deprecation warnings).

Our estimate was that only around 1% of dependencies (6 out of top 500) are affected.

Even simply bumping all of the core libraries' bounds to allow 0.11 in the install plan would realistically take a several weeks.

All core libraries do support bytestring-0.11 already.

According to this list, there are only two boot libraries, which do not support bytestring-0.11 yet. These are haskeline (just pending a revision) and Cabal (blocked by a trivial patch to cryptohash-sha256).

That said, I do not agree that "retarget the entire ecosystem onto it overnight" is a correct description. There was a lot of work done during three month since bytestring-0.11 release.

(I'm sorry, this comment by itself probably looks more combative than I would like it to be. I have to go, will get back to your other points later)

[bgamari]

According to this list, there are only two boot libraries, which do not support bytestring-0.11 yet. These are haskeline (just pending a revision) and Cabal (blocked by a trivial patch to cryptohash-sha256).

Unfortunately even trivial patches can take weeks to merge. Furthermore, it's not true that these are the only libraries in need of patching: at least unix and parsec are both in need of updating as well; I'm not sure whether there are others. In general, GHC tries to finalize its submodules near the beginning of the release cycle for good reason: getting this right takes time. Currently, time is not something that we have.

Perhaps let me elaborate on how I'm approaching this process in hopes that it will explain my reluctance to bump bytestring. Release engineering is a process of gradual convergence. We released 9.0.1-alpha1 months ago and it has already seen a fair amount of testing; thankfully it looks to be in good shape. The only things that we have backported since then are:

• fixes for regressions and newly introduced features
• well-audited fixes for high-severity, pre-existing bugs
• the memory ordering work (already an extremely large backport for this late in the cycle, but it turns out it is required for Apple ARM support)
• extremely low-risk features (adding the -pgmlm flag)
• keepAlive# (to which this patch adapts bytestring; this is also an alarmingly large change to backport but #17760 is a severe soundness issue which has been felt with increasing regularity by users, consequently we decided to hold the release on fixing it)

None of this backported work had any user-breaking changes; they were either non-interface-relevant, rarely, changed only internal modules. The reason we have been so conservative is simple: the release is already terribly late and my goal is to move quickly to a final. In order to do this we need to tread carefully, making minimal changes to fix the issues that remain outstanding. Unfortunately, this precludes including 2kLoC of deltas in a unnecessary submodule bump, especially one that does not fix any immediate bug.

Believe me, I do understand the desire to see the new bytestring release in users' hands. However, if GHC 9.0.1 is going to be out this year we need to start converging and very soon. As I said, I would be more than happy to bump bytestring for 9.2. However, I am trying hard to minimize the delta between the pre-release we have and the final. If the 9.0 release goes on much longer it carries the very real risk that it starts running into the 9.2 release cycle, which is in no one's best interest.

(I'm sorry, this comment by itself probably looks more combative than I would like it to be. I have to go, will get back to your other points later)

Quite alright; I similarly apologize if the above reads as harsh. Electronic communications is always hard.

[Bodigrim]

at least unix and parsec are both in need of updating as well

According to Hackage, both are fine.

---

Ben, I am in no position to dictate downstream dependencies which version of bytestring to use. I was not terribly happy with your decision (which you did not communicate beforehand despite being asked and being involved in 0.11 release) to go ahead with bytestring-0.10.12.0 for GHC 9.0 alpha, but it is not up to me to decide. I made an effort to clarify your further intentions, so that I could plan ahead my release schedule, but you ignored my emails. Now, at 11th hour you bring a pretty substantial patch and ask for making a new release in 0.10 series urgently. I'm sorry, I'm an unpaid volunteer, it is less than two weeks to Christmas and I'm already on vacation.

If you insist on following your plan and schedule, please feel free to assume all responsibility and prepare 0.10.* release yourself. You have all necessary permissions.

Ben, I have nothing but utter respect to you and your work. Nevertheless we need to improve the way we communicate about changes in GHC and releases. Current pattern is detrimental for both parties.

[bgamari]

at least unix and parsec are both in need of updating as well

According to Hackage, both are fine.

Ahh, I had missed that a revision had been pushed; unfortunately, a Hackage revision alone is not sufficient. GHC requires that the change be committed upstream, not merely committed as a Hackage revision since we need a Git commit to reference.

so that I could plan ahead my release schedule, but you ignored my emails.

My apologies for missing your follow-up email; believe me that I did not ignore it intentionally. However, as I mentioned in my first reply, this discussion was already too late in the release cycle to have been actionable. The alpha had already been released and we were hoping to have a release candidate out in a few weeks. Unfortunately, the approach to #17760 that I was pursuing ended up not be viable; this is why the release has been delayed as long as it has.

Now, at 11th hour you bring a pretty substantial patch and ask for making a new release in 0.10 series urgently.

I would hesitate to call this branch "substantial". The only patch in this branch needed for 9.0 is the second (b82da95), which is only changes a few withForeignPtrs to unsafeWithForeignPtr for performance reasons; we could technically release without it but at the cost of a performance regression. It, of course, brings me no joy to make this request so late, especially during a holiday and apologize for this cutting into your vacation.

Ben, I have nothing but utter respect to you and your work.

Likewise, I am very happy that bytestring now has active maintenance and by all accounts it seems you have done a great job bringing 0.11 together. Thank you!

Nevertheless we need to improve the way we communicate about changes in GHC and releases. Current pattern is detrimental for both parties.

Agreed; the signalling surrounding this release has admittedly been terrible, in part due to the uncertainty of fixing #17760, and in part due to my own shortcomings: There should have been more active status updates from me.

That being said, when you do want to get in touch with me email is not the preferred way; I receive far too much mail and it's very likely for things to go unseen. If a change is necessary in GHC's bytestring submodule, please do open a GHC ticket and assign me before the feature freeze. Unless someone asks before the freeze submodule bumps happen in GHC on an as-needed basis; unless a submodule is egregiously behind upstream we may or may not bump, simply because it takes so long to reach a state where all submodule bounds are compatible.

[Bodigrim]

I would hesitate to call this branch "substantial". The only patch in this branch needed for 9.0 is the second (b82da95), which is only changes a few withForeignPtrs to unsafeWithForeignPtr for performance reasons

It is substantial, because I cannot verify the changes without building GHC from sources: GHC 9.0 alpha1 does not have unsafeWithForeignPtr. And even if I succeed in building GHC, I cannot guarantee that unsafeWithForeignPtr will eventually make way into GHC 9.0.1; we have had a history of U-turns around #17760.

I mean, I do not particularly mind merging this patch into master, but I cannot release it without a newer public GHC 9.0 alpha (even if I had time to do it properly before Christmas).

Just to be clear: my suggestion above for you to release these changes in 0.10 series yourself was done in good faith.

---

In a wider context:

• Could GHC developers possibly stop submitting patches to their own fork of bytestring and commit to upstream instead? I understand historical reasons, but it is so cumbersome to reconcile.

• Is there an automated way to follow nightly builds of GHC HEAD and GHC 9.0? I would like to add a CI job, building bytestring with the latest GHC binaries available.

[bgamari]

Hi @Bodigrim,

It is substantial, because I cannot verify the changes without building GHC from sources: GHC 9.0 alpha1 does not have unsafeWithForeignPtr. And even if I succeed in building GHC, I cannot guarantee that unsafeWithForeignPtr will eventually make way into GHC 9.0.1; we have had a history of U-turns around #17760.

Yes; this is precisely why I have held off until now in submitting the patch: I did not want to push work to submodules until it was clear what the final design looked like. The fact that this MR is open is your assurance that the interface is final.

Just to be clear: my suggestion above for you to release these changes in 0.10 series yourself was done in good faith.

Alright, thanks! I will go ahead and push out a new 0.10 release.

Could GHC developers possibly stop submitting patches to their own fork of bytestring and commit to upstream instead? I understand historical reasons, but it is so cumbersome to reconcile.

I'm a bit unclear on what you mean here. Is there a case you are thinking of?

To be clear, we typically do avoid merging patches to ghc until all submodule patches have gone in. Unfortunately, this policy was recently inadvertently broken with the recent sized primop refactor (which is addressed by the first commit in this branch). However, this is something that we generally try hard to avoid and procedures have been improved to avoid this sort of thing in the future.

Is there an automated way to follow nightly builds of GHC HEAD and GHC 9.0? I would like to add a CI job, building bytestring with the latest GHC binaries available.

Indeed, nightly binaries distributions are available from CI. Moreover, ghc-up can be used to install these. Does this help?

[Bodigrim]

It does not compile with GHC 9.0.1 RC1, because unsafeWithForeignPtr is not imported.

Data/ByteString/Internal.hs:439:36: error:
    • Variable not in scope:
        unsafeWithForeignPtr
          :: ForeignPtr Word8 -> (Ptr a3 -> IO [Word8]) -> IO [Word8]
    • Perhaps you want to add ‘unsafeWithForeignPtr’ to the import list
      in the import of ‘GHC.ForeignPtr’
      (Data/ByteString/Internal.hs:(162,1)-(166,61)).
    |
439 |     accursedUnutterablePerformIO $ unsafeWithForeignPtr fp $ \base ->
    |

[phadej]

I suppose there should be a branch point at 0.10.12.0 tag, and this PR retargeted there (and then forward ported to master)?

The GHC-9.0.1-rc1 is built with just Replace withForeignPtr with unsafeWithF...Ptr where appropriate commit on top of 0.10.12.0 tag.

@bgamari, @angerman is [CmmSized] adjust commit also required, and if it is, why it's missing from ghc-9.0.1-rc1

[Bodigrim]

I suppose there should be a branch point at 0.10.12.0 tag...

Yes, there is https://github.com/bgamari/bytestring/tree/wip/no-fptr-0.10

@bgamari I can take care of 0.10.X.X release next week, if you wish. Should both commits be included?

[Bodigrim]

@bgamari I've prepared 0.10.12.1 release in #340, please take a look.

[Bodigrim]

I pushed fixes, similar to merged in #340.