[RFC] Support the latest versions of GHC #289

friedbrice · 2022-11-20T15:37:36Z

This is a tentative PR. I'd like to merge it, but I'm not adamant. This PR eliminates almost all CPP, closing #252. (Some CPP is kept around for gating certain tests by platform support.)

So, I'd like to tackle some of the Issues, such as #262, #258, and #272, and I would need to smooth out and simplify project maintenance before doing so.

I'd like to hear what thoughts @scotty-web/admins, @fumieval, @RyanGlScott, and other interested parties have about the following changes.

remove most CPP
use Cabal 3.0
add ghc 9.2, 9.4 to ci
drop ghc pre 8.10 from ci
move library files to src/
fourmolu and hlint
make copyright in license files agree with copyright in cabal files

* add ghc 9.2, 9.4 to ci * make copyright in license files agree with copyright in cabal files * move library files to src/ * fourmolu and hlint * use Cabal 3.0 * remove most CPP

friedbrice · 2022-11-20T15:44:06Z

Web/Scotty/Internal/Types.hs

-{-# LANGUAGE UndecidableInstances #-}
-{-# LANGUAGE RecordWildCards #-}
-
-module Web.Scotty.Internal.Types where


Moved to src/Web/Scotty/Internal/Types.hs

friedbrice · 2022-11-20T15:44:33Z

bench/Main.hs

-  , GeneralizedNewtypeDeriving
-  #-}
+{-# LANGUAGE GeneralizedNewtypeDeriving #-}
+{-# LANGUAGE OverloadedStrings #-}

 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:44:59Z

examples/basic.hs

@@ -1,4 +1,5 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:45:06Z

examples/bodyecho.hs

@@ -1,10 +1,11 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:45:16Z

examples/cookies.hs

@@ -1,23 +1,24 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 -- This examples requires you to: cabal install cookie
 -- and: cabal install blaze-html
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:45:23Z

examples/exceptions.hs

+{-# LANGUAGE GeneralizedNewtypeDeriving #-}
+{-# LANGUAGE OverloadedStrings #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:45:44Z

examples/globalstate.hs

@@ -1,4 +1,6 @@
-{-# LANGUAGE OverloadedStrings, GeneralizedNewtypeDeriving #-}


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:45:54Z

examples/gzip.hs

@@ -1,15 +1,16 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:46:03Z

examples/options.hs

@@ -1,4 +1,5 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:46:15Z

examples/reader.hs

@@ -10,30 +10,34 @@ module Main where
 import Control.Monad.Reader (MonadIO, MonadReader, ReaderT, asks, lift, runReaderT)


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:47:02Z

examples/scotty-examples.cabal

-Category:            Web
-Stability:           experimental
-Build-type:          Simple
-Cabal-version:       >= 1.10


Changed to Cabal 3.0

friedbrice · 2022-11-20T15:47:32Z

examples/scotty-examples.cabal

-Build-type:          Simple
-Cabal-version:       >= 1.10
-Description:         Example programs using @scotty@
-tested-with:         GHC == 7.6.3


added GHC 9.4, 9.2. Dropped GHC <8.10

friedbrice · 2022-11-20T15:49:51Z

examples/scotty-examples.cabal

+  location: https://github.com/scotty-web/scotty
+  subdir: examples
+
+common examples-common


I put Scotty and GHC boot libs in a common build-depends. This adds some spurious build-depends to some of the examples, but I figure it's a moot point as there's not really going to be a Haskell distribution that doesn't include the GHC boot libs already.

friedbrice · 2022-11-20T15:50:06Z

examples/upload.hs

@@ -1,4 +1,5 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:50:13Z

examples/urlshortener.hs

@@ -1,4 +1,5 @@
 {-# LANGUAGE OverloadedStrings #-}
+
 module Main (main) where


Code formatter/linter. No functional changes.

friedbrice · 2022-11-20T15:50:39Z

scotty.cabal

-Category:            Web
-Stability:           experimental
-Build-type:          Simple
-Cabal-version:       >= 1.10


Changed to Cabal 3.0

friedbrice · 2022-11-20T15:51:08Z

scotty.cabal

-  [WAI] <http://hackage.haskell.org/package/wai>
-  .
-  [Warp] <http://hackage.haskell.org/package/warp>
-tested-with:         GHC == 7.6.3


Add GHCs 9.4, 9.2. Drop GHCs <8.10

friedbrice · 2022-11-20T15:51:36Z

scotty.cabal

@@ -64,81 +63,87 @@ Extra-source-files:
    examples/static/jquery-json.js
    examples/uploads/.keep

-Library


cosmetic refactor, to reduce future diff noise

friedbrice · 2022-11-20T16:00:42Z

scotty.cabal

-                       Web.Scotty.Route
-                       Web.Scotty.Util
-  default-language:    Haskell2010
-  build-depends:       aeson                 >= 0.6.2.1  && < 2.2,


I'm kinda skeptical that these bounds really make a lot of sense. How were they calculated?

In the PR, I only bound base and leave the rest up to Cabal. To compensate for lack of bounds in the cabal file, I've included freeze files from some successful builds with a variety of GHCs. If people are adamant about it, I can use the feeze files to compute version bounds.

friedbrice · 2022-11-20T16:03:39Z

src/Web/Scotty.hs

@@ -7,46 +6,99 @@
 -- the comments on each of these functions for more information.
 module Web.Scotty


Code formatter/linter. No functional change.

friedbrice · 2022-11-20T16:05:32Z

src/Web/Scotty/Action.hs

-    (e,r) <- flip MS.runStateT def
-           $ flip runReaderT env
-           $ runExceptT
-           $ runAM
-           $ action `catchError` (defH h)
+    (e, r) <-
+        flip MS.runStateT def $
+            flip runReaderT env $
+                runExceptT $
+                    runAM $
+                        action `catchError` defH h


I don't necessarily agree with the code formatter, but I think it's useful for reducing future diff noise. I'm open to rolling back the formatting changes if people disagree.

friedbrice · 2022-11-20T16:08:07Z

src/Web/Scotty/Action.hs

-    parseParam t = if t' == T.toCaseFold "true"
-                   then Right True
-                   else if t' == T.toCaseFold "false"
-                        then Right False
-                        else Left "parseParam Bool: no parse"
-        where t' = T.toCaseFold t
+    parseParam t
+        | t' == true = Right True
+        | t' == false = Right False
+        | otherwise = Left "parseParam Bool: no parse"
+        where
+            t' = T.toCaseFold t
+            true = T.toCaseFold "true"
+            false = T.toCaseFold "false"


I cleaned up this block by changing nested if/then to guards, and I factored out true and false so that a sufficiently-advanced compiler could float them if it wanted.

friedbrice · 2022-11-20T16:12:47Z

src/Web/Scotty/Route.hs

+{-# LANGUAGE LambdaCase #-}
+{-# LANGUAGE OverloadedStrings #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+
 module Web.Scotty.Route


remove legacy CPP

Code formatter/linter.

friedbrice · 2022-11-20T16:14:50Z

src/Web/Scotty/Trans.hs

@@ -11,84 +12,145 @@
 -- the comments on each of these functions for more information.
 module Web.Scotty.Trans


Code formatter/linter. No functional change.

friedbrice · 2022-11-20T16:15:28Z

src/Web/Scotty/Util.hs

@@ -12,19 +12,18 @@ module Web.Scotty.Util
    , readRequestBody


Code formatter/linter. No functional change.

friedbrice · 2022-11-20T16:16:49Z

src/Web/Scotty/Action.hs

 {-# LANGUAGE OverloadedStrings #-}
-{-# LANGUAGE RankNTypes        #-}
+
 module Web.Scotty.Action


Remove legacy CPP.

Code formatter/linter.

friedbrice · 2022-11-20T16:19:49Z

test/Web/ScottySpec.hs

+                it "stops the execution of an action" $ do
+                    get "/scotty" `shouldRespondWith` 400
+
+-- Skip these tests on Windows, since Unix sockets are not available
 #if !defined(mingw32_HOST_OS)


I moved some things that were top-level and nested them into the test block. Reads better with the CPP that way.

friedbrice · 2022-11-20T16:21:18Z

test/Web/ScottySpec.hs

-{-# LANGUAGE OverloadedStrings, CPP #-}
+{-# LANGUAGE CPP #-}
+{-# LANGUAGE OverloadedStrings #-}
+
 module Web.ScottySpec (main, spec) where


minor functional change by moving some helpers from top-level into a let bind. This looks like a huge diff, but set github to ignore whitespace, as the changes are mostly indentation.

friedbrice · 2022-11-20T16:25:04Z

src/Web/Scotty/Internal/Types.hs

+    deriving newtype (Functor, Applicative, Monad, MonadIO)
+    deriving (Semigroup, Monoid) via (Ap (ActionT e m) a)


Deriving Semigroup, Monoid, and Monad.

doesn't this rule out old GHCs? or are we dropping them (which is also fine)

drop ghc pre 8.10 from ci

I suppose this is intentional

friedbrice · 2022-11-20T16:25:50Z

src/Web/Scotty/Internal/Types.hs

+instance (Monad m, ScottyError e) => Fail.MonadFail (ActionT e m) where
+    fail = ActionT . throwError . stringError
+
+instance (Monad m, ScottyError e) => Alternative (ActionT e m) where


Moved implementations from MonadPlus instance to Alternative instance.

friedbrice · 2022-11-20T16:26:52Z

src/Web/Scotty/Internal/Types.hs

+{-# LANGUAGE TypeFamilies #-}
+{-# LANGUAGE UndecidableInstances #-}
+
+module Web.Scotty.Internal.Types where


moved from ./Web/Scotty/Internal/Types.hs.

remove CPP

derive (functionally compatible) instances instead of writing them

fumieval · 2022-11-21T06:17:26Z

It might be too late, but it would be nice to separate code style changes at least

friedbrice · 2022-11-21T15:56:42Z

@fumieval good call. i can easily split them out :-)

ocramz · 2023-09-10T20:41:03Z

good call. i can easily split them out :-)

Hi @friedbrice , would you be able to move this forward so we do one last review and merge? Thank you!

ocramz

Took me a while to review it all but eventually LGTM.

For the future let's not have formatting PRs mixed with code changes.

ocramz · 2023-09-23T10:12:54Z

freezes/ghc8107-2022-11-19-cabal.project.freeze

@friedbrice is this file necessary?

ocramz · 2023-09-23T10:14:27Z

scotty.cabal

+license: BSD-3-Clause
+license-file: LICENSE
+author: Andrew Farmer <[email protected]>
+maintainer: Daniel Brice <[email protected]>


there are other maintainers too at present

ocramz

Apologies, I'm not able to fix the conflicts so I cannot merge these changes.

ocramz · 2023-09-23T11:05:03Z

Let's have separate PRs for linting and functional changes in the future.

* drop ghc pre 8.10 from ci

fca121c

* add ghc 9.2, 9.4 to ci * make copyright in license files agree with copyright in cabal files * move library files to src/ * fourmolu and hlint * use Cabal 3.0 * remove most CPP

friedbrice requested review from RyanGlScott and chessai November 20, 2022 15:38

add version bounds for base

3ed8586

friedbrice commented Nov 20, 2022

View reviewed changes

scotty.cabal

Category: Web

Stability: experimental

Build-type: Simple

Cabal-version: >= 1.10

Copy link

Member Author

friedbrice Nov 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to Cabal 3.0

friedbrice commented Nov 20, 2022

View reviewed changes

fumieval mentioned this pull request Jun 17, 2023

add Web.Scotty.Cookie #293

Merged

ocramz approved these changes Sep 23, 2023

View reviewed changes

ocramz requested changes Sep 23, 2023

View reviewed changes

ocramz closed this Sep 23, 2023

		@@ -1,4 +1,5 @@
		{-# LANGUAGE OverloadedStrings #-}

		module Main (main) where

		@@ -1,10 +1,11 @@
		{-# LANGUAGE OverloadedStrings #-}

		module Main (main) where

		@@ -1,4 +1,6 @@
		{-# LANGUAGE OverloadedStrings, GeneralizedNewtypeDeriving #-}

		@@ -1,15 +1,16 @@
		{-# LANGUAGE OverloadedStrings #-}

		module Main (main) where

		@@ -10,30 +10,34 @@ module Main where
		import Control.Monad.Reader (MonadIO, MonadReader, ReaderT, asks, lift, runReaderT)

		@@ -7,46 +6,99 @@
		-- the comments on each of these functions for more information.
		module Web.Scotty

		@@ -11,84 +12,145 @@
		-- the comments on each of these functions for more information.
		module Web.Scotty.Trans

		deriving newtype (Functor, Applicative, Monad, MonadIO)
		deriving (Semigroup, Monoid) via (Ap (ActionT e m) a)

[RFC] Support the latest versions of GHC #289

[RFC] Support the latest versions of GHC #289

Conversation

friedbrice commented Nov 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbrice Nov 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbrice Nov 20, 2022 • edited Loading

Choose a reason for hiding this comment

friedbrice Nov 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fumieval commented Nov 21, 2022

friedbrice commented Nov 21, 2022

ocramz commented Sep 10, 2023

ocramz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocramz left a comment

Choose a reason for hiding this comment

ocramz commented Sep 23, 2023

friedbrice Nov 20, 2022 •

edited

Loading

friedbrice Nov 20, 2022 •

edited

Loading

friedbrice Nov 20, 2022 •

edited

Loading