haskell / hackage-security

Hackage security framework based on TUF (The Update Framework)
http://hackage.haskell.org/package/hackage-security
56 stars 48 forks source link

Bump the lower bound on base? #303

Closed philderbeast closed 7 months ago

philderbeast commented 9 months ago

Could we bump the lower bound on base?

https://github.com/haskell/hackage-security/blob/b306678403d56ce72af99a40f3b8287b695408bf/hackage-security/hackage-security.cabal#L114-L115

https://github.com/haskell/hackage-security/blob/b306678403d56ce72af99a40f3b8287b695408bf/hackage-security/hackage-security.cabal#L33-L45

I came here to see if I could do something about the noisy -Wstar-is-type warnings from hackage-security when compiling cabal but saw that I couldn't do that without CPP conditionals given that -Wstar-is-type was introduced in ghc-8.6 and we have a lower bound on base of 4.8 that is ghc-7.10.1.

philderbeast commented 9 months ago

There's a fair bit of CPP in this project already.

$ grep -rw --include=\*.hs . -e '#if'
./example-client/src/Prelude.hs:#if !MIN_VERSION_base(4,8,0)
./example-client/src/Prelude.hs:#if MIN_VERSION_base(4,8,0)
./example-client/src/Prelude.hs:#if MIN_VERSION_base(4,6,0)
./hackage-repo-tool/src/Hackage/Security/RepoTool/Util/IO.hs:#if MIN_VERSION_directory(1,2,0)
./hackage-repo-tool/src/Hackage/Security/RepoTool/Util/IO.hs:#if MIN_VERSION_directory(1,2,0)
./hackage-repo-tool/src/Hackage/Security/RepoTool/Util/IO.hs:#if MIN_VERSION_base(4,8,0)
./hackage-repo-tool/src/Prelude.hs:#if !MIN_VERSION_base(4,8,0)
./hackage-repo-tool/src/Prelude.hs:#if MIN_VERSION_base(4,8,0)
./hackage-repo-tool/src/Prelude.hs:#if MIN_VERSION_base(4,6,0)
./hackage-repo-tool/src/Main.hs:#if !MIN_VERSION_base(4,8,0)
./hackage-security-http-client/src/Hackage/Security/Client/Repository/HttpLib/HttpClient.hs:#if MIN_VERSION_http_client(0,5,0)
./hackage-security/src/MyPrelude.hs:#if !MIN_VERSION_base(4,8,0)
./hackage-security/src/MyPrelude.hs:#if MIN_VERSION_base(4,8,0)
./hackage-security/src/MyPrelude.hs:#if MIN_VERSION_base(4,6,0)
./hackage-security/src/Text/JSON/Canonical.hs:#if !(MIN_VERSION_base(4,7,0))
./hackage-security/src/Text/JSON/Canonical.hs:#if MIN_VERSION_base(4,7,0)
./hackage-security/src/Text/JSON/Canonical.hs:#if MIN_VERSION_base(4,7,0)
./hackage-security/src/Hackage/Security/Key.hs:#if !MIN_VERSION_base(4,7,0)
./hackage-security/src/Hackage/Security/Key.hs:#if MIN_VERSION_ed25519(0,0,4)
./hackage-security/src/Hackage/Security/Key.hs:#if MIN_VERSION_ed25519(0,0,4)
./hackage-security/src/Hackage/Security/Key.hs:#if !MIN_VERSION_base(4,7,0)
./hackage-security/src/Hackage/Security/Trusted.hs:#if __GLASGOW_HASKELL__ >= 710
./hackage-security/src/Hackage/Security/Trusted/TCB.hs:#if __GLASGOW_HASKELL__ >= 710
./hackage-security/src/Hackage/Security/Trusted/TCB.hs:#if __GLASGOW_HASKELL__ >= 710
./hackage-security/src/Hackage/Security/Trusted/TCB.hs:#if MIN_VERSION_base(4,8,0)
./hackage-security/src/Hackage/Security/Client.hs:#if __GLASGOW_HASKELL__ >= 710
./hackage-security/src/Hackage/Security/Client.hs:#if __GLASGOW_HASKELL__ < 800
./hackage-security/src/Hackage/Security/Client.hs:#if __GLASGOW_HASKELL__ < 800
./hackage-security/src/Hackage/Security/Client.hs:#if MIN_VERSION_base(4,8,0)
./hackage-security/src/Hackage/Security/TUF/Patterns.hs:#if __GLASGOW_HASKELL__ >= 800
./hackage-security/src/Hackage/Security/TUF/Patterns.hs:#if __GLASGOW_HASKELL__ >= 800
./hackage-security/src/Hackage/Security/JSON.hs:#if MIN_VERSION_base(4,8,0)
./hackage-security/src/Hackage/Security/Client/Repository/Remote.hs:#if MIN_VERSION_base(4,8,0)
./hackage-security/src/Hackage/Security/Client/Repository.hs:#if MIN_VERSION_base(4,8,0)
./hackage-security/src/Hackage/Security/Util/IO.hs:#if MIN_VERSION_base(4,11,0)
./hackage-security/src/Hackage/Security/Util/IO.hs:#if MIN_VERSION_base(4,11,0)
./hackage-security/src/Hackage/Security/Util/Path.hs:#if MIN_VERSION_directory(1,2,0)
./hackage-security/src/Hackage/Security/Util/Path.hs:#if MIN_VERSION_directory(1,2,2)
./hackage-security/src/Hackage/Security/Util/Path.hs:#if MIN_VERSION_directory(1,2,0)
./hackage-security/src/Hackage/Security/Util/Checked.hs:#if __GLASGOW_HASKELL__ >= 800
./hackage-security/src/Hackage/Security/Util/Checked.hs:#if __GLASGOW_HASKELL__ >= 708
./hackage-security/src/Hackage/Security/Util/Checked.hs:#if __GLASGOW_HASKELL__ >= 708
./hackage-security/src/Hackage/Security/Util/Checked.hs:#if __GLASGOW_HASKELL__ >= 708
./hackage-security/src/Hackage/Security/Util/Checked.hs:#if MIN_VERSION_base(4, 7, 0)
./hackage-security/src/Hackage/Security/Util/Checked.hs:#if __GLASGOW_HASKELL__ >= 708
./hackage-security/src/Hackage/Security/Util/Some.hs:#if !MIN_VERSION_base(4,7,0)
./hackage-security/src/Hackage/Security/Util/Some.hs:#if MIN_VERSION_base(4,7,0)
./hackage-security/src/Hackage/Security/Util/Some.hs:#if MIN_VERSION_base(4,7,0)
./hackage-security/src/Hackage/Security/Util/JSON.hs:#if __GLASGOW_HASKELL__ < 710
./hackage-security/src/Hackage/Security/Util/JSON.hs:#if !MIN_VERSION_time(1,5,0)
./hackage-security/src/Hackage/Security/Util/JSON.hs:#if __GLASGOW_HASKELL__ >= 710
./hackage-security/src/Hackage/Security/Util/JSON.hs:#if __GLASGOW_HASKELL__ >= 710
./hackage-security/src/Hackage/Security/Util/JSON.hs:#if !MIN_VERSION_time(1,5,0)
./hackage-security/tests/TestSuite.hs:#if MIN_VERSION_Cabal(2,0,0)
./hackage-security/tests/TestSuite.hs:#if !MIN_VERSION_Cabal(2,0,0)
./hackage-security/tests/TestSuite/JSON.hs:#if MIN_VERSION_aeson(2,0,0)
./hackage-security/tests/TestSuite/JSON.hs:#if MIN_VERSION_aeson(2,0,0)
./hackage-root-tool/Main.hs:#if !MIN_VERSION_base(4,8,0)
./hackage-security-HTTP/src/Hackage/Security/Client/Repository/HttpLib/HTTP.hs:#if MIN_VERSION_base(4,8,0)
Mikolaj commented 9 months ago

Yes, I think it's a good idea to get base bounds to match Cabal-the-library's, given that cabal's bounds are already quite generous and anything that uses hackage-security most probably uses Cabal as well. I hope GHC versions in CI already do match (if not, we can narrow them further, I think).

andreasabel commented 9 months ago

After enforcing GHC >= 8 in

there is much less #if.

I routinely drop GHC 7 now in all my projects, that's a no-brainer.
Dropping GHC < 8.6 requires probably a bit more care, as it is not dead-clear that the Haskell community is abandoning these GHCs already. I seem like pantry, one of our users, still builds with GHC 8.2 (base-4.10).

andreasabel commented 9 months ago

I seem like pantry, one of our users, still builds with GHC 8.2 (base-4.10).

Actually, only on paper. In fact, it has no build plans for GHC < 8.8:

The only other user is Cabal, so going to GHC 8.6 seems fine here after all.

Mikolaj commented 9 months ago

The oldest GHC that cabal CI builds cabal with is 8.4 from what I see.

philderbeast commented 8 months ago

@Mikolaj for testing this against cabal, would you expect that I'd take cabal/.github/workflows/validate.yml, and manually run the steps locally?

To pick up my changes to hackage-security for the version bump for this test, I plan to add a source-repository-package but wouldn't commit this.

Mikolaj commented 8 months ago

@philderbeast: I'd imagine just building all and the running the cabal-testsuite (with the newly built binary in --with-cabal!) should be plenty, as described here: https://github.com/haskell/cabal/blob/master/cabal-testsuite/README.md. Or, to test some more, you may want to run the validate.sh script.

An alternative is to open a throw-away PR in cabal repo (then close it and keep it as documentation) and tweak it so that CI does what you want. And here's yet another tool that may be helpful: https://github.com/haskell/cabal/pull/9561

Mikolaj commented 8 months ago

BTW, would you also recommend a revision of the bound on Hackage? Or just wait for a new release (@andreasabel can predict better than myself on what timescale the new release is likely to emerge).

andreasabel commented 7 months ago

If I am not mistaken, master is releasable, so we can ship hackage-security/v0.6.2.5 at any time.

Mikolaj commented 7 months ago

Yes, please do, that will make @ffaf1's life easier.

andreasabel commented 7 months ago

Ok! I'll open a PR for the release.