haskell-infra / hackage-trustees

Issue tracker for Hackage maintainance and trustee operations
https://hackage.haskell.org/packages/trustees/
42 stars 7 forks source link

cryptohash-sha1 doesn't allow building with ghc 9.0 or newer #305

Closed cartazio closed 3 years ago

sjakobi commented 3 years ago

Indeed. See https://github.com/haskell-hvr/cryptohash-sha1/issues/10.

ysangkok commented 3 years ago

same goes for cryptohash-md5, cryptohash-sha256, cryptohash-sha512. I tried bumping cryptohash-md5/sha1 manually and they built fine. So a revision would be sufficient for them to work on GHC 9.

sjakobi commented 3 years ago

same goes for cryptohash-md5, cryptohash-sha256, cryptohash-sha512.

Related issues and PRs are https://github.com/haskell-hvr/cryptohash-md5/issues/5 (https://github.com/haskell-hvr/cryptohash-md5/pull/6, cc @swamp-agr), https://github.com/haskell-hvr/cryptohash-sha256/issues/12, https://github.com/haskell-hvr/cryptohash-sha512/issues/7.

My main question is whether the GHC changes around optimizing withForeignPtr result in significantly reduced performance compared to GHC 8.10.

If the performance is significantly reduced, should the trustees relax the bounds, anyway? The policy document doesn't say what to do in this case – maybe @gbaz, @phadej, etc. can weigh in.

Personally, I probably won't perform a bounds relaxation, anyway, because I don't have much experience with cryptographic code. Other trustees may be more comfortable with this.

cartazio commented 3 years ago

That shouldn’t change the security characteristics of the code.

On Tue, Jun 8, 2021 at 5:48 AM Simon Jakobi @.***> wrote:

same goes for cryptohash-md5, cryptohash-sha256, cryptohash-sha512.

Related issues and PRs are haskell-hvr/cryptohash-md5#5 https://github.com/haskell-hvr/cryptohash-md5/issues/5 ( haskell-hvr/cryptohash-md5#6 https://github.com/haskell-hvr/cryptohash-md5/pull/6, cc @swamp-agr https://github.com/swamp-agr), haskell-hvr/cryptohash-sha256#12 https://github.com/haskell-hvr/cryptohash-sha256/issues/12, haskell-hvr/cryptohash-sha512#7 https://github.com/haskell-hvr/cryptohash-sha512/issues/7.

My main question is whether the GHC changes around optimizing withForeignPtr https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/9.0#withforeignptr result in significantly reduced performance compared to GHC 8.10.

If the performance is significantly reduced, should the trustees relax the bounds, anyway? The policy document https://github.com/haskell-infra/hackage-trustees/blob/master/policy.md#2-metadata-only-changes-relaxing-constraints doesn't say what to do in this case – maybe @gbaz https://github.com/gbaz, @phadej https://github.com/phadej, etc. can weigh in.

Personally, I probably won't perform a bounds relaxation, anyway, because I don't have much experience with cryptographic code. Other trustees may be more comfortable with this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/haskell-infra/hackage-trustees/issues/305#issuecomment-856627648, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQRN4UFIIZYEYVOTCRLTRXRNNANCNFSM442WLNFA .

gbaz commented 3 years ago

Our policies don't cover performance regressions between versions of GHC, or really address performance in general. In this case I think that it means that they approve of revisions even when there may be performance regressions on new versions of GHC. PVP semantics are supposed to cover buildability and correctness, and trustee revisions are supposed to abide by pvp semantics. That's really the only sane way to proceed, so I give bounds bumps a 👍

Bodigrim commented 3 years ago

We can bump bounds, but it does not feel right to depend on abandoned packages for cryptographic applications. Is there any alternative to them?

cartazio commented 3 years ago

They actually do a pretty awesome trick using capi ffi to have a nice c impl while not leaking linker symbols. They deserve to be maintained. The underlying code is very stable.

On Sun, Jun 13, 2021 at 3:42 PM Bodigrim @.***> wrote:

We can bump bounds, but it does not feel right to depend on abandoned packages for cryptographic applications. Is there any alternative to them?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/haskell-infra/hackage-trustees/issues/305#issuecomment-860260518, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQQBVTEAQBVAMT2U2MLTSUC3ZANCNFSM442WLNFA .

Bodigrim commented 3 years ago

Trustees can intervene in a short term, but clearly this situation needs a sustainable solution. Trustees are not well-positioned to semi-maintain a package for a long time.

To follow the policy I would expect someone a) to raise PRs for cryptohash-sha{256,512}, b) to remind maintainers about existing PRs for cryptohash-{sha1,md5}. Feel free checking these packages against base changelog and leaving reviews as well. Trustees need a clear evidence that maintaners were notified about changes and proved unresponsive for a long time, and that changes are indeed safe.

ysangkok commented 3 years ago

Existing PRs:

sjakobi commented 3 years ago

We can bump bounds, but it does not feel right to depend on abandoned packages for cryptographic applications. Is there any alternative to them?

AFAIK the hnix packages are currently switching to cryptonite although its maintainer hasn't always been as responsive as could be desired. See e.g. https://mail.haskell.org/pipermail/haskell-cafe/2021-March/133636.html.

There's also the new Z-Botan package, which however operates on data structures from the Z-Data package, so it's not as easy to use from existing Haskell packages.

If the cryptohash-* packages should live on it would be great to find some new and active maintainers for it.

cartazio commented 3 years ago

Vincent is full awol too. So that doesn’t even make sense to me …

On Tue, Jun 15, 2021 at 5:50 AM Simon Jakobi @.***> wrote:

We can bump bounds, but it does not feel right to depend on abandoned packages for cryptographic applications. Is there any alternative to them?

AFAIK the hnix packages are currently switching to cryptonite although its maintainer hasn't always been as responsive as could be desired. See e.g. https://mail.haskell.org/pipermail/haskell-cafe/2021-March/133636.html.

There's also the new Z-Botan https://hackage.haskell.org/package/Z-Botan package, which however operates on data structures from the Z-Data https://hackage.haskell.org/package/Z-Data-0.8.5.0/docs/Z-Data-Vector-Base.html#t:Bytes package, so it's not as easy to use from existing Haskell packages.

If the cryptohash-* packages should live on it would be great to find some new and active maintainers for it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/haskell-infra/hackage-trustees/issues/305#issuecomment-861357367, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQR6SKV6X2XOGPVAMK3TS4O6BANCNFSM442WLNFA .

Bodigrim commented 3 years ago

cryptonite will be massively broken by sized primitives in GHC 9.2, see https://gitlab.haskell.org/ghc/head.hackage/-/blob/master/patches/basement-0.0.12.patch AFAIR cryptohash-* packages used to compile with GHC 9.2 last time I checked.

gbaz commented 3 years ago

Performed base bumps on the four cryptohash-* packages.