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

Build with ghc(js) 9.8.2 + 9.10.1 #1093

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ cabal.project.local
**/ghcid-output.txt

# HLS specific files
lib/cabal.project
#lib/cabal.project
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So that git wouldn't ignore lib/cabal.project for cabal/haskell.nix builds.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not remove then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well somebody put it there for a reason I assume, if it causes problems in the future people would at least see a possible solution there.

lib/cabal.project.local
2 changes: 1 addition & 1 deletion all-tests.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ let
nginxRoot = "/run/nginx";
obelisk = import ./default.nix {};
# Get NixOS a pre-release 20.03 that contains the python based tests and recursive nix
pkgs = import (builtins.fetchTarball https://github.com/nixos/nixpkgs/archive/3de5266.tar.gz) {};
pkgs = import (builtins.fetchTarball https://github.com/nixos/nixpkgs/archive/3de5266b255cf3bd41438d2cfb0698420a33302e.tar.gz) {};
sshKeys = import (pkgs.path + /nixos/tests/ssh-keys.nix) pkgs;
make-test = import (pkgs.path + /nixos/tests/make-test-python.nix);
obelisk-everywhere = (import ./all-builds.nix { inherit supportedSystems; }).x86_64-linux.cache;
Expand Down
4 changes: 2 additions & 2 deletions dep/nix-thunk/github.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"repo": "nix-thunk",
"branch": "master",
"private": false,
"rev": "2b65d2cbf83e77a90b4103418d037ba5b0b32e77",
"sha256": "07pr014iifyy1xaiqamkx78xfszr67g8kzysmzgzms0a4470k1i9"
"rev": "682327c1f7859eaa34a7863bc3b8cccce3f5d038",
"sha256": "0d7yibmqbgw02xl7lb3m75pg8ih4mxwy1hk4mra2fvfx6nbhv442"
}
3 changes: 3 additions & 0 deletions lib/asset/manifest/obelisk-asset-manifest.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ library
-Wall -Werror -Wredundant-constraints -Wincomplete-uni-patterns -Wincomplete-record-updates -O2
-fno-warn-unused-do-bind -funbox-strict-fields -fprof-auto-calls

if arch(javascript)
buildable: False
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems right, but I'm confused on why we didn't need a if impl(ghcjs) before. Was it only implicitly gated by cabal.project ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure...


executable obelisk-asset-manifest-generate
default-language: Haskell2010
hs-source-dirs: src-bin
Expand Down
1 change: 1 addition & 0 deletions lib/asset/manifest/src/Obelisk/Asset/Promoted.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{-# LANGUAGE KindSignatures #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE TemplateHaskellQuotes #-}

module Obelisk.Asset.Promoted
( writeStaticProject
, declareStatic
Expand Down
3 changes: 3 additions & 0 deletions lib/asset/serve-snap/obelisk-asset-serve-snap.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,6 @@ library
ghc-options:
-Wall -Werror -Wredundant-constraints -Wincomplete-uni-patterns -Wincomplete-record-updates -O2
-fno-warn-unused-do-bind -funbox-strict-fields -fprof-auto-calls

if arch(javascript)
buildable: False
3 changes: 3 additions & 0 deletions lib/backend/obelisk-backend.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ library
universe
exposed-modules: Obelisk.Backend
ghc-options: -Wall -Werror -Wredundant-constraints -Wincomplete-uni-patterns -Wincomplete-record-updates -O

if arch(javascript)
buildable: False
9 changes: 7 additions & 2 deletions lib/backend/src/Obelisk/Backend.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# OPTIONS_GHC -fno-warn-orphans #-}

module Obelisk.Backend
( Backend (..)
, BackendConfig (..)
Expand Down Expand Up @@ -45,10 +46,14 @@ module Obelisk.Backend
import Control.Monad.Fail (MonadFail)
import Data.Monoid ((<>))
#endif
#endif

#if __GLASGOW_HASKELL__ >= 906
import Control.Monad
import Control.Monad.IO.Class (MonadIO (liftIO))
#else
import Control.Monad.Except
#endif
#endif

import Data.ByteString (ByteString)
import qualified Data.ByteString.Char8 as BSC8
import Data.Default (Default (..))
Expand Down
16 changes: 16 additions & 0 deletions lib/cabal.dependencies.project
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
index-state: 2025-02-01T00:00:00Z
allow-newer: all

constraints:
hnix-store-core < 0.7
, hnix-store-remote < 0.7

source-repository-package
type: git
location: https://github.com/ymeister/splitmix.git
tag: fe4d9e4ec01ba7caf8053d6888ec2e7f89fad874
--sha256: 19fbwcmdmb9w34cp19r2j4qywhnjmxxdv4rwci29pzbvgbnnjdia

-- https://github.com/haskell-cryptography/HsOpenSSL/issues/93#issuecomment-2311816896
package HsOpenSSL
ghc-options: -optc=-Wno-incompatible-pointer-types"
23 changes: 23 additions & 0 deletions lib/cabal.project
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
if arch(javascript)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we pull the common ones out of the if ?
Also, should we remove the old cabal.project.dev ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can we pull the common ones out of the if ?

Yes. Wasn't aware that there could be multiple packages: statements that collect all of them at the time of writing.

Also, should we remove the old cabal.project.dev ?

I assume so. Didn't want to touch that as I was not 100% of cabal.project.dev goal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. Wasn't aware that there could be multiple packages: statements that collect all of them at the time of writing.

Me neither, I didn't even know we could use cabal conditionals in .project

I assume so. Didn't want to touch that as I was not 100% of cabal.project.dev goal.

I don't see it doing anything your cabal.project doesn't do and better.
I think it's bitrotted due to the bad ergonomics I ran into when doing the GHC 9.6 PR.
The libs had -Werror which was a pain for nix builds but the cabal.project was explicitly disabling that, contributing to the problem.

packages:
./executable-config/inject
./executable-config/lookup
./frontend
./route
./tabulation
else
packages:
./asset/manifest
./asset/serve-snap
./backend
./command
./executable-config/inject
./executable-config/lookup
./frontend
./route
./run
./selftest
./snap-extras
./tabulation

import: cabal.dependencies.project
10 changes: 7 additions & 3 deletions lib/command/obelisk-command.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ library
, extra
, filepath
, fsnotify
, git
, github
, here
, hnix
, hnix >=0.5
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't see anything else in the command diff related to this.
For future reference, was this already de-facto or needed for some other reason?

, hpack
, io-streams
, lens
Expand All @@ -39,6 +38,7 @@ library
, optparse-applicative
, placeholders
, prettyprinter
, prettyprinter-compat-ansi-wl-pprint
, process
, reflex
, reflex-fsnotify
Expand All @@ -48,14 +48,15 @@ library
, text
, time
, transformers
, unix
, unix-compat
, unordered-containers
, utf8-string
, which
, yaml
, nix-thunk
, cli-extras

exposed-modules:
Obelisk.App
Obelisk.Command
Expand All @@ -68,6 +69,9 @@ library
Obelisk.Command.Preprocessor
ghc-options: -Wall

if arch(javascript)
buildable: False

executable ob
main-is: src-bin/ob.hs
build-depends: base, obelisk-command
Expand Down
15 changes: 7 additions & 8 deletions lib/command/src/Obelisk/App.hs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{-# LANGUAGE ConstraintKinds #-}
{-# LANGUAGE CPP #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
Expand All @@ -9,11 +10,11 @@
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE TupleSections #-}
{-# LANGUAGE PackageImports #-}

module Obelisk.App where

import Control.Lens
import Control.Monad.Catch (MonadCatch, MonadMask, MonadThrow)
import Control.Monad.Fail (MonadFail)
import Control.Monad.Reader (MonadIO, ReaderT (..), ask, runReaderT)
import Control.Monad.Writer (WriterT)
import Control.Monad.State (StateT)
Expand All @@ -25,16 +26,14 @@ import Control.Monad.Log (MonadLog)
import Cli.Extras.Types
import "nix-thunk" Nix.Thunk (NixThunkError)

#if !MIN_VERSION_base(4,18,0)
import Control.Monad.Fail (MonadFail)
#endif

import Cli.Extras
( CliConfig
, CliLog
, CliThrow
, CliT (..)
, ProcessFailure
Comment on lines -29 to -33
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh, were these all just unused?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess?

( ProcessFailure
, AsProcessFailure (..)
, AsUnstructuredError (..)
, HasCliConfig
, Output
, runCli
)

Expand Down
26 changes: 15 additions & 11 deletions lib/command/src/Obelisk/Command.hs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TupleSections #-}
{-# LANGUAGE PackageImports #-}

module Obelisk.Command where

import Control.Monad.IO.Class (MonadIO, liftIO)
Expand All @@ -16,17 +18,22 @@ import qualified Data.List.NonEmpty as NonEmpty
import qualified Data.Map as Map
import qualified Data.Text as T
import Data.Traversable (for)
import GHC.IO.Encoding.Types (textEncodingName)
import Network.Socket (PortNumber)
import Options.Applicative
import Options.Applicative.Help.Pretty (text, (<$$>))
import System.Directory
import System.Environment
import System.FilePath
import System.Exit
import qualified System.Info
import System.IO (hIsTerminalDevice, Handle, stdout, stderr, hGetEncoding, hSetEncoding, mkTextEncoding)
import GHC.IO.Encoding.Types (textEncodingName)
import System.Process (rawSystem)
import Network.Socket (PortNumber)

#if MIN_VERSION_optparse_applicative(0,18,0)
import Text.PrettyPrint.ANSI.Leijen (text, (<$$>))
#else
import Options.Applicative.Help.Pretty (text, (<$$>))
#endif

import Obelisk.App
import Obelisk.Command.Deploy
Expand Down Expand Up @@ -229,6 +236,9 @@ thunkConfig = ThunkConfig
thunkUpdateConfig :: Parser ThunkUpdateConfig
thunkUpdateConfig = ThunkUpdateConfig
<$> optional (strOption (long "branch" <> metavar "BRANCH" <> help "Use the given branch when looking for the latest revision"))
#if MIN_VERSION_nix_thunk(0,7,1)
<*> optional (strOption (short 'r' <> long "rev" <> metavar "REVISION" <> help "Update to this specific revision"))
#endif
Comment on lines +239 to +241
Copy link
Collaborator

Choose a reason for hiding this comment

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

I backported for now, but shouldn't we simply be importing all these parsers from nix-thunk?
@ryantrinkle

<*> thunkConfig

thunkPackConfig :: Parser ThunkPackConfig
Expand Down Expand Up @@ -288,17 +298,11 @@ interpretOpts :: Parser [(FilePath, Interpret)]
interpretOpts = many
( (, Interpret_Interpret) <$>
strOption (common <> long "interpret" <> help
"Don't pre-build packages found in DIR when constructing the package database. The default behavior is \
\'--interpret <project-root>', which will load everything which is unpacked into GHCi. \
\ Use --interpret and --no-interpret multiple times to add or remove multiple trees \
\ from the environment. Settings for right-most directories will \
\ override settings for any identical directories given earlier."
"Don't pre-build packages found in DIR when constructing the package database. The default behavior is '--interpret <project-root>', which will load everything which is unpacked into GHCi. Use --interpret and --no-interpret multiple times to add or remove multiple trees from the environment. Settings for right-most directories will override settings for any identical directories given earlier."
)
<|> (, Interpret_NoInterpret) <$>
strOption (common <> long "no-interpret" <> help
"Make packages found in DIR available in the package database (but only when they are used dependencies). \
\ This will build the packages in DIR before loading GHCi. \
\See help for --interpret for how the two options are related."
"Make packages found in DIR available in the package database (but only when they are used dependencies). This will build the packages in DIR before loading GHCi. See help for --interpret for how the two options are related."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know why these lines were split that way before?
I don't particularly mind this change, but in general let's avoid doing unrelated drive-by changes in PRs as long as we have a big PR queue of potential conflicts

Copy link
Collaborator Author

@ymeister ymeister Jan 30, 2025

Choose a reason for hiding this comment

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

I think multi-line string thing broke or changed is some way in newer ghc versions. Couldn't compile without this change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aaah it's some interaction of multiline strings with {-# LANGUAGE CPP #-}. I remember that biting me once.
I think there's actually a multiline string extension on 9.12 but we can't ditch older GHC yet so let's go with your change

)
)
where
Expand Down
28 changes: 23 additions & 5 deletions lib/command/src/Obelisk/Command/Deploy.hs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE LambdaCase #-}
Expand All @@ -7,6 +8,7 @@
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE PackageImports #-}
{-# LANGUAGE ViewPatterns #-}

{-|
Description:
Implementation of the CLI deploy commands. Deployment is done by intializing
Expand All @@ -16,7 +18,6 @@
-}
module Obelisk.Command.Deploy where

import Control.Applicative (liftA2)
import Control.Lens
import Control.Monad
import Control.Monad.Catch (Exception (displayException), MonadThrow, bracket, throwM, try)
Expand Down Expand Up @@ -47,12 +48,17 @@ import qualified Nix.Expr.Shorthands as Nix
import Prettyprinter (layoutCompact)
import Prettyprinter.Render.String (renderString)

#if !MIN_VERSION_base(4,18,0)
import Control.Applicative (liftA2)
#endif

import Obelisk.App (MonadObelisk, wrapNixThunkError)
import Obelisk.Command.Nix
import Obelisk.Command.Project
import Obelisk.Command.Utils

import "nix-thunk" Nix.Thunk
import "nix-thunk" Nix.Thunk.Internal (prettyReadThunkError)
import Cli.Extras

-- | Options passed to the `init` verb
Expand Down Expand Up @@ -88,8 +94,14 @@ deployInit deployOpts root = do
failWith [i|Deploy directory ${deployDir} should not be the same as project root.|]
thunkPtr <- wrapNixThunkError (readThunk root) >>= \case
Right (ThunkData_Packed _ ptr) -> return ptr
_ -> wrapNixThunkError (getThunkPtr CheckClean_NotIgnored root Nothing)
_ -> wrapNixThunkError $ compat $ getThunkPtr CheckClean_NotIgnored root Nothing
deployInit' thunkPtr deployOpts
where
#if MIN_VERSION_nix_thunk(0,7,1)
compat = fmap $ \(ptr, _isWorktree) -> ptr
#else
compat = id
#endif
Comment on lines 96 to +104
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we should treat a worktree dir the same as a standalone unpacked dir?
@ryantrinkle


-- | The preamble in 'deployInit' provides deployInit' with a 'ThunkPtr' that it can install in
-- the staging directory.
Expand Down Expand Up @@ -178,7 +190,7 @@ deployPush deployPath builders = do
checkGitCleanStatus srcPath True >>= \case
True -> wrapNixThunkError $ packThunk (ThunkPackConfig False (ThunkConfig Nothing)) srcPath
False -> failWith $ T.pack $ "ob deploy push: ensure " <> srcPath <> " has no pending changes and latest is pushed upstream."
Left err -> failWith $ "ob deploy push: couldn't read src thunk: " <> T.pack (show err)
Left err -> failWith $ "ob deploy push: couldn't read src thunk: " <> prettyReadThunkError err
let version = show . _thunkRev_commit $ _thunkPtr_rev thunkPtr
let moduleFile = deployPath </> "module.nix"
moduleFileExists <- liftIO $ doesFileExist moduleFile
Expand Down Expand Up @@ -270,8 +282,14 @@ fi

-- | Update the source thunk in the staging directory to the HEAD of the branch.
deployUpdate :: MonadObelisk m => FilePath -> m ()
deployUpdate deployPath = wrapNixThunkError $
updateThunkToLatest (ThunkUpdateConfig Nothing (ThunkConfig Nothing)) (deployPath </> "src")
deployUpdate deployPath = wrapNixThunkError $ updateThunkToLatest cfg (deployPath </> "src")
where
cfg = ThunkUpdateConfig
Nothing
#if MIN_VERSION_nix_thunk(0,7,1)
Nothing
#endif
(ThunkConfig Nothing)

-- | Platforms that we deploy obelisk artefacts to.
data PlatformDeployment = Android | IOS
Expand Down
7 changes: 6 additions & 1 deletion lib/command/src/Obelisk/Command/Nix.hs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{-# LANGUAGE AllowAmbiguousTypes #-}
{-# LANGUAGE ConstraintKinds #-}
{-# LANGUAGE CPP #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE MultiParamTypeClasses #-}
Expand All @@ -8,6 +9,7 @@
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE TypeApplications #-}

module Obelisk.Command.Nix
( Arg (..)
, NixBuildConfig (..)
Expand Down Expand Up @@ -46,9 +48,12 @@ import Data.Bool (bool)
import Data.Default
import Data.List (intercalate)
import Data.Maybe
import Data.Monoid ((<>))
import qualified Data.Text as T

#if !MIN_VERSION_base(4,18,0)
import Data.Monoid ((<>))
#endif

import Obelisk.App (MonadObelisk)
import Cli.Extras

Expand Down
Loading