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

Support GHC 9.0.1 (at least in some packages) #2154

Merged
merged 4 commits into from May 13, 2021
Merged

Conversation

sjakobi
Copy link
Collaborator

@sjakobi sjakobi commented Feb 26, 2021

Blocked on:

The incantation for circumventing the bounds issues is:

cabal build -O0 all --enable-tests --enable-benchmarks --allow-newer=haskell-lsp:base,haskell-lsp-types:base,hnix:hashable,hnix:template-haskell,hnix:semialign-indexed,hnix:semialign,cryptohash-sha512:base,cryptohash-sha256:base,cryptohash-sha1:base,cryptohash-md5:base,hslogger:base

@Anton-Latukha
Copy link

Anton-Latukha commented Feb 27, 2021

haskell-hvr/cryptohash-sha is passed from hnix-store?

hnix would have a big release, there is big migration ongoing. Estimate release in ~2 months, maybe a ~1 month maybe faster. Seems like trying to chip at 0.9 worth this release.

I was doing hnix-store migration to cryptonite, but so far I postponed work in favor of HNix refactors in the main computational path.

BTW, hnix-store-core is under removal from Stackage Nightly, and from new LTSes.

With current resources I give estimate of ~3 month until theHNix migration to GHC 9.0

BTW, because of big changes in HNix, besides of giving a good ChangeLog, I probably would double in helping migrating dhall-nix to the new HNix releases once they come out, especially definitely the next one. Because dhall-nix is the main alive well-used project in HNix deps - that strategy gives HNix team the freedom to do huge code improvements in it.

@sjakobi
Copy link
Collaborator Author

sjakobi commented Feb 27, 2021

haskell-hvr/cryptohash-sha is passed from hnix-store?

Indeed. We depend on the cryptohash-* packages only via hnix-store-core.

I was doing hnix-store migration to cryptonite

Yeah, cryptonite doesn't seem to be very actively maintained either, but it doesn't look as dire as HVR's cryptohash-* packages. dhall uses cryptonite directly too.

BTW, because of big changes in HNix, besides of giving a good ChangeLog, I probably would double in helping migrating dhall-nix to the new HNix releases once they come out, especially definitely the next one. Because dhall-nix is the main alive well-used project in HNix deps - that strategy gives HNix team the freedom to do huge code improvements in it.

Thanks! If you could give us a hand with the adjustments in dhall-nix (and possibly in dhall-nixpkgs too), I'd really appreciate it! :)

@Gabriella439
Copy link
Collaborator

@antislava: Yeah, don't worry about make breaking changes to the hnix API. We can easily accommodate those changes

@sjakobi sjakobi force-pushed the sjakobi/ghc-9.0.1 branch 2 times, most recently from c70aeff to d540af3 Compare February 28, 2021 16:48
@sjakobi
Copy link
Collaborator Author

sjakobi commented Mar 12, 2021

At this stage, haskell-foundation/foundation#548 appears to be the only true blocker for GHC 9.0.1 compatibility in the core dhall package. Since the basement maintainer has been so unresponsive, I've been wondering how we could remove that dependency.

The direct dependencies that use basement are: cryptonite, http-client-tls, memory, where cryptonite and memory AFAIK are only used to produce and parse SHA256 digests.

My main concern about this switch is that SHA might be slower than crytonite.

In any case, I don't plan on executing this switch until GHC 9.0.2 has been released – GHC 9.0.1 has a serious correctness bug.

@Gabriella439
Copy link
Collaborator

@sjakobi: I think it would be fine to switch to those packages, including SHA. From a quick scan of the SHA source code it seems like they pay careful attention to performance in the implementation, so I'm not too concerned about a performance regression.

@sjakobi
Copy link
Collaborator Author

sjakobi commented May 9, 2021

Now that Vincent Hanquez has uploaded compatible versions of basement, memory and cryptonite, dhall, dhall-bash, dhall-json, dhall-yaml and dhall-openapi build with GHC 9.0.1 and their tasty test suites pass.

dhall-docs builds, but its golden test suite produces different output due to a change in the Hashable Text instance in hashable-1.3.1.0. I've opened chrisdone/lucid#107 for this and added a bound hashable < 1.3.1 to this test suite.

dhall-nix and dhall-nixpkgs will need a compatible hnix release.

dhall-lsp-server will need to migrate from the old haskell-lsp package to the new lsp package (see http://hackage.haskell.org/package/lsp-1.0.0.1/changelog).

I think we should merge and release the progress that we have so far.

@sjakobi sjakobi marked this pull request as ready for review May 9, 2021 01:14
@sjakobi sjakobi changed the title WIP: Support GHC 9.0.1 Support GHC 9.0.1 (at least in some packages) May 9, 2021
@sjakobi
Copy link
Collaborator Author

sjakobi commented May 10, 2021

Hydra failed due to

Dhall Tests
  QuickCheck
    everything well-typed should normalize:                                                                                    FAIL (31.38s)
      *** Failed! (after 82619 tests and 1 shrink):
      Exception:
        Error: Compiler bug

        An ill-typed expression was encountered during normalization.
        Explanation: This error message means that there is a bug in the Dhall compiler.
        You didn't do anything wrong, but if you would like to see this problem fixed
        then you should report the bug at:

        https://github.com/dhall-lang/dhall-haskell/issues

        CallStack (from HasCallStack):
          error, called at src/Dhall/Eval.hs:1201:13 in dhall-1.38.1-BNLcpta7e8d3IQfIPD3r7g:Dhall.Eval
      App TextReplace (TextLit (Chunks [] ""))
      Use --quickcheck-replay=126743 to reproduce.

Looks like a bug, but probably not due to this PR.

@Gabriella439 Gabriella439 merged commit 7be4ee5 into master May 13, 2021
@Gabriella439 Gabriella439 deleted the sjakobi/ghc-9.0.1 branch May 13, 2021 19:49
@Anton-Latukha
Copy link

GHC9.0: (hnix-store-{core,remote} release were made ~ month ago & migrated away from cryptohash-* to cryptonite) & hnix release just was made supporting all that, that means the majority of the list is covered.

& guys went over and made a set of revisions for HVR packages, hslogger also got new revision allowing GHC 9.0.

@sjakobi
Copy link
Collaborator Author

sjakobi commented Nov 17, 2021

The direct dependencies that use basement are: cryptonite, http-client-tls, memory, where cryptonite and memory AFAIK are only used to produce and parse SHA256 digests.

The direct dependencies on cryptonite and memory were removed in #2335. The transitive dependency on basement via http-client-tls remains TODO.

@Anton-Latukha
Copy link

Anton-Latukha commented Nov 17, 2021

Well, "just great", hnix-store migrated away from cryptohash-sha512 to cryptonite, dhall-haskell migrated to cryptohash-sha256.

Is there some upstream consensus? When phadej needs to also maintain cryptohash-sha512 & cryptonite stays single-handedly vincent - I do not feel safe with neither of those "solutions". I see no people were given maintainers privileges, seems like those projects learned nothing.


Btw, I have grown tired on : continuously work on HNix & pursuing Nix upstream currently - (when no one seems interested) & went to do more meaningful work, so ping me - or, - some people do have access.

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.

None yet

3 participants