Would it be possible to port to GHC 9.10?

Mikolaj commented 11 months ago

The changes would probably be similar to what's in

https://github.com/haskell/vector/pull/462

See

https://github.com/haskell/core-libraries-committee/issues/167

for context and the breakage got revealed in

https://gitlab.haskell.org/ghc/ghc/-/issues/23848#note_527774

Thank you!

infinity0 commented 11 months ago

GHC 9.10 is still in development right? So far I've only updated this package for releases, and I don't really want to make a habit of chasing upstream so closely.

Do you want to help co-maintain this package? It's a bit special and involves a bunch of hacky shell scripts to auto-apply patches to the upstream libraries. In the long term it might be better to persuade those packages to include our strict versions and take over the maintenance themselves.

It's a shame that Haskell doesn't have (convenient) strictness polymorphism for data...

Mikolaj commented 11 months ago

I totally share your sentiment regarding our long term strict future.

Thank you for you kind offer of co-maintainership, but unfortunately, I'm overloaded with maintenance ATM (I maintain cabal among other things).

A pity, though, because I report a lot of GHC problems that pop up in my library horde-ad that has strict-containers as its dependencies. Fixing usually involves reproducing the problem with GHC HEAD and just now strict-containers stopped compiling with GHC HEAD. A solution would be to ask https://ghc.gitlab.haskell.org/head.hackage/ to include strict-containers so that it's kept compatible with HEAD continuously. Given that strict-containers contains modules that should be (IMHO) in the packages already in head.hackage, that makes sense to me. However, I can't judge how feasible this would be, e.g., considering the scripts you mention. Thoughs?

Mikolaj commented 11 months ago

@RyanGlScott, do you think it makes sense to add this package to head.hackage? When I'm looking at transitive deps at https://hackage.haskell.org/package/strict-containers, it seems the following deps are not yet included in head.hackage:

indexed-traversable strict vector-binary-instances assoc bifunctor-classes-compat these transformers-compat

That's a surprisingly long list, but maybe I'm looking it up wrong.

RyanGlScott commented 11 months ago

do you think it makes sense to add this package to head.hackage?

Sure, head.hackage welcomes patches for libraries that don't build with HEAD. You don't even have to be the maintainer of the package to add it—you can just submit a MR (following the instructions here). I don't have time to get around to this myself right now, but if you submit a head.hackage MR, I'd be happy to be a reviewer.

it seems the following deps are not yet included in head.hackage:

You don't need head.hackage patches for all of a package's dependencies, just those that do not build with recent GHCs. All of the libraries you list currently build with everything up to GHC HEAD, so there's no need to explicitly add them to head.hackage.

infinity0 commented 11 months ago

I don't mind having this package in head.hackage as long as I'm not expected to maintain that patched version.

Directly applying the patch from vector to here isn't the normal maintenance workflow for this package. The usual workflow is to run the scripts to re-apply (i.e. rebase) our own patches on top of the latest vector. So any patches for this package in head.hackage would basically be thrown away when I get around to releasing a next version of this package (by rebasing the existing patches as just described). Just wanted to let you guys know this, so it doesn't come as a surprise.

The other dependencies @Mikolaj you mentioned above are due to providing extra instances in both this package and also strict, to avoid having an explosion of extra packages just for trivial instances.

Mikolaj commented 11 months ago

@RyanGlScott: thank you.

@infinity0: is it plausible that people contributing to head.hackage could do just that (whenever it's the proper fix), that is, rebase the patches contained in strict-containers using the scripts? Is the process reasonably documented somewhere (I can't find a README)? Would you throw away such head.hackage patches? I guess you would, because verifying that the rebase is fine amounts to running the scripts and comparing the output, so it's just additional effort compared to running the scripts alone --- unless any non-trivial extra changes are exceptionally required.

I'm taking your time discussing this, @infinity0 and @RyanGlScott, not in order to convince anybody about anything, but honestly to figure out which course of action makes sense in this very special case.

RyanGlScott commented 11 months ago

I don't mind having this package in head.hackage as long as I'm not expected to maintain that patched version.

Patching a library in head.hackage doesn't imply any obligations on the part of the library maintainer to maintain the patch. It's an entirely separate effort. Library maintainers are welcome to use head.hackage if they wish, of course, but it's also fine not to do this.

I don't have a strong opinion on the coding conventions used in head.hackage, as it generally prioritizes making things compile as opposed to writing patches in the exact same style as the original authors.

infinity0 commented 11 months ago

@infinity0: is it plausible that people contributing to head.hackage could do just that (whenever it's the proper fix), that is, rebase the patches contained in strict-containers using the scripts? Is the process reasonably documented somewhere (I can't find a README)?

The "patches" not only include actual patch files but also involve hacky shell scripts to rename modules, and make ad-hoc version-specific adjustments such as copying utility files etc. Whether this works from upstream release to release depends on how much they've refactored in the interim.

The broad workflow is documented in https://github.com/haskellari/strict-containers/blob/master/strict-containers/update-patches-from-git.sh

For people that don't regularly look at this package, and just want to make it compile for GHC HEAD, applying a single upstream patch to this package (i.e. rather than the other way around) will probably be easier, though there's no hard guarantee about that.

Mikolaj commented 10 months ago

I've had a look. Applying the strict-containers scripts seems the fastest way to fix this, because the changes are trivial, but numerous (unless a good sed invocation covers all). Unfortunately, the version of the vector package containing the workaround (https://github.com/haskell/vector/pull/462) is not yet on Hackage, so I'd need to apply the scripts to a commit from github or alternatively to the patched code on head.hackage. This sadly means a PR would not be directly applicable to the original strict containers repo.

Huh, meta-programming (as in head-hackage and in strict-containers) is powerful, but really adds costs in random places.

If this can't be done maximally reusably, perhaps one could find a good sed invocation and apply it to the version of strict-containers from Hackage and add that patch to head.hackage. The patches should be analogues to what is on head.hackage for vector, etc., but the file names would be different.

Do I make sense?

infinity0 commented 10 months ago

This sadly means a PR would not be directly applicable to the original strict containers repo.

This is fine, no problem. I was only mentioning it above for informational purposes. Conceptually it's the same as git rebasing a persistent fork, rebasing a small part of it often can't be done reusably to help the overall operation anyways, don't worry too much about reuse. Just do whatever is easiest.

unless a good sed invocation covers all

Not for our changes unfortunately, the full logic is in this script https://github.com/haskellari/strict-containers/blob/master/strict-containers/regen.sh

Mikolaj commented 10 months ago

I drafted a head.hackage patch: https://gitlab.haskell.org/ghc/head.hackage/-/merge_requests/333

Mikolaj commented 10 months ago

The patch is merged, so strict-containers are now on head.hackage and so the HEAD compatibility part of this ticket is done. Once GHC 9.10 is out, it seems just regenerating the package (and dep bound bumps) should ensure GHC 9.10 compatibility and then this head.hackage patch can be removed.

Thank you again.