Use Cabal PackageInfo_warp instead of Paths_warp#1042
Use Cabal PackageInfo_warp instead of Paths_warp#1042lrworth wants to merge 2 commits intoyesodweb:masterfrom
Conversation
Use of Paths_warp causes a huge runtime dependency closure because it references GHC itself.
Vlix
left a comment
There was a problem hiding this comment.
Looks fine, though does the cabal-version bump mean it won't build with GHC 9.10 anymore? That might be unacceptable.
We'd need to add conditionals etc. to add the Path/PackageInfo modules.
| !mRange = getHeaderValue tokenRange reqvt | ||
| !mReferer = getHeaderValue tokenReferer reqvt | ||
| !mUserAgent = getHeaderValue tokenUserAgent reqvt | ||
| !mPath = getFieldValue tokenPath reqvt -- SHOULD |
There was a problem hiding this comment.
Requires an increase to the lower version bound of http2 (>= 5.2)
| import qualified Data.ByteString as S | ||
| import Data.ByteString.Internal (create) | ||
| import qualified Data.CaseInsensitive as CI | ||
| import Data.List (foldl') |
There was a problem hiding this comment.
I expect this to fail on any GHC < 9.10, so this might need CPP around it.
| -- | The version of Warp. | ||
| warpVersion :: String | ||
| warpVersion = showVersion Paths_warp.version | ||
| warpVersion = showVersion PackageInfo_warp.version |
There was a problem hiding this comment.
How did this end up in this module?
Can you move this to Network.Wai.Handler.Warp.Internal and re-export from here with a comment that it's for backwards compatibility?
| , settingsNoParsePath = False | ||
| , settingsInstallShutdownHandler = const $ return () | ||
| , settingsServerName = C8.pack $ "Warp/" ++ showVersion Paths_warp.version | ||
| , settingsServerName = C8.pack $ "Warp/" ++ showVersion PackageInfo_warp.version |
There was a problem hiding this comment.
After moving warpVersion to Network.Wai.Handler.Warp.Internal, please import from there and use here.
|
Also please merge the current |
|
The lts-21 / GHC 9.4 configuration listed in CI doesn't work: If in principle this work might be merged at a later date, we are happy to maintain a fork in a mergeable state. What do you say, @Vlix? |
|
I think we could work around this and keep the Adding a flag to flag package-info
description: Use 'PackageInfo_warp' instead of 'Paths_warp' (needs Cabal >= 3.10)
default: False
manual: Trueif flag(package-info)
other-modules: PackageInfo_warp
else
other-modules: Paths_warp |
|
haskell/cabal#9481 guards against using |
|
#1044 is a much more conservative approach — optionally removing dependency on |
Use of
Paths_warpcauses a large dependency closure when building with nix. See NixOS/nixpkgs#264434 for example.This reduces the closure size of our application from 756Mb to 53Mb by virtue of not including GHC and GCC.
It makes a significant change to
cabal-versionand I'm not sure what compatibility we're trying to maintain in this project; also I made some incidental changes so that it would build with GHC 9.12.This is not merge-ready — the PR is meant to open a discussion about using
PackageInfo_.