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 all 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
55 changes: 55 additions & 0 deletions .github/workflows/haskell.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: github-action

on: [push, pull_request]

jobs:
build:
strategy:
fail-fast: false
matrix:
ghc: ['8.6', '8.8', '8.10', '9.0', '9.2', '9.4', '9.6', '9.8', '9.10']
os: ['ubuntu-latest', 'macos-13'] # https://github.com/haskell-actions/setup/issues/77
runs-on: ${{ matrix.os }}

name: GHC ${{ matrix.ghc }} on ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: haskell-actions/setup@v2
with:
ghc-version: ${{ matrix.ghc }}
- name: Cache
uses: actions/cache@v3
env:
cache-name: cache-cabal
with:
path: ~/.cabal
key: ${{ runner.os }}-${{ matrix.ghc }}-build-${{ env.cache-name }}-${{ hashFiles('**/*.cabal') }}-${{ hashFiles('**/cabal.project') }}
restore-keys: |
${{ runner.os }}-${{ matrix.ghc }}-build-${{ env.cache-name }}-
${{ runner.os }}-${{ matrix.ghc }}-build-
${{ runner.os }}-${{ matrix.ghc }}-
${{ runner.os }}

- name: Install system packages (linux)
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt-get update
sudo apt install libsodium-dev

- name: Install system packages (macos)
if: matrix.os == 'macos-13'
run: |
brew update
brew install libsodium

- name: Set up cabal project file for CI
run: cd lib && cp cabal.project.ci cabal.project

- name: Install dependencies
run: |
cabal update
cd lib && cabal build --only-dependencies --enable-tests --enable-benchmarks all
- name: Build obelisk libraries
run: cd lib && cabal build --enable-tests --enable-benchmarks all
- name: Run tests
run: cd lib && cabal test all
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: hnix:*, hnix-store-core:algebraic-graphs, lens-family-th:template-haskell

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"
18 changes: 18 additions & 0 deletions lib/cabal.project
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
packages:
./executable-config/inject
./executable-config/lookup
./frontend
./route
./tabulation

if !arch(javascript)
packages:
./asset/manifest
./asset/serve-snap
./backend
./command
./run
./selftest
./snap-extras

import: cabal.dependencies.project
18 changes: 18 additions & 0 deletions lib/cabal.project.ci
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
packages:
./executable-config/inject
./executable-config/lookup
./frontend
./route
./tabulation

if !arch(javascript)
packages:
./asset/manifest
./asset/serve-snap
./backend
-- ./command
./run
-- ./selftest
./snap-extras

import: cabal.dependencies.project
26 changes: 0 additions & 26 deletions lib/cabal.project.dev

This file was deleted.

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
Loading