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.

Sign up for GitHub

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

more flexible da flavour #215

Merged
merged 5 commits into from
Jun 3, 2020
Merged

more flexible da flavour #215

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a8f52c6
more flexible da flavour
garyverhaegen-da Jun 2, 2020
4f3459f
attach da params to Da constructor
garyverhaegen-da Jun 3, 2020
57010e9
add note in README for da-specific build
garyverhaegen-da Jun 3, 2020
84a1599
Da record on a single line
garyverhaegen-da Jun 3, 2020
e550d56
drop da prefix for subflags; remove mention of upload
garyverhaegen-da Jun 3, 2020
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
Prev Previous commit
Next Next commit
attach da params to Da constructor
  • Loading branch information
@garyverhaegen-da
garyverhaegen-da committed Jun 3, 2020
commit 4f3459f113e352f1b3208b6670218ac4c186aa3e
91 changes: 48 additions & 43 deletions CI.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,12 @@ main = do
<> Opts.progDesc "Build (and possibly upload) ghc-lib and ghc-lib-parser tarballs."
<> Opts.header "CI - CI script for ghc-lib"
)
Options { ghcFlavor, daMergeBaseSha, daPatches, daFlavor, stackOptions } <- Opts.execParser opts
version <- buildDists ghcFlavor stackOptions daMergeBaseSha daPatches daFlavor
Options { ghcFlavor, stackOptions } <- Opts.execParser opts
version <- buildDists ghcFlavor stackOptions
putStrLn version

data Options = Options
{ ghcFlavor :: GhcFlavor
, daMergeBaseSha :: String
, daPatches :: [String]
, daFlavor :: String
, stackOptions :: StackOptions
} deriving (Show)

Expand All @@ -51,7 +48,15 @@ data StackOptions = StackOptions
, ghcOptions :: Maybe String -- If 'Just _', pass '--ghc-options="xxx"' to 'stack build' (for ghc verbose, try 'v3').
} deriving (Show)

data GhcFlavor = Ghc8101 | Ghc881 | Ghc882 | Ghc883 | Da | GhcMaster String
data GhcFlavor = Ghc8101
| Ghc881
| Ghc882
| Ghc883
| Da { mergeBaseSha :: String
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't comment on style matters but I think this will fit on a single line eh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with whatever style you prefer on this project. I've updated this one (though you need longer-than-80-chars lines for it to fit ;-) ); don't hesitate to point out others. In particular, I have no idea how the parseOptions function "should" be indented.

Copy link
Contributor

Choose a reason for hiding this comment

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

"Consistency is the hobgoblin of little minds" 🤣

, patches :: [String]
, flavor :: String
}
| GhcMaster String
deriving (Eq, Show)

-- Last tested gitlab.haskell.org/ghc/ghc.git at
Expand All @@ -70,13 +75,13 @@ stackResolverOpt = \case
Just resolver -> "--resolver " ++ resolver
Nothing -> ""

ghcFlavorOpt :: String -> GhcFlavor -> String
ghcFlavorOpt daFlavor = \case
ghcFlavorOpt :: GhcFlavor -> String
ghcFlavorOpt = \case
Ghc8101 -> "--ghc-flavor ghc-8.10.1"
Ghc881 -> "--ghc-flavor ghc-8.8.1"
Ghc882 -> "--ghc-flavor ghc-8.8.2"
Ghc883 -> "--ghc-flavor ghc-8.8.3"
Da -> "--ghc-flavor " <> daFlavor
Da { flavor } -> "--ghc-flavor " <> flavor
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good. That is the flag that gets pushed "downstream" into Ghclibgen. I like it.

GhcMaster _hash -> "--ghc-flavor ghc-master"
-- The git SHA1 hash is not passed to ghc-lib-gen at this time.

Expand Down Expand Up @@ -104,29 +109,12 @@ genVersionStr = \case

parseOptions :: Opts.Parser Options
parseOptions = Options
<$> Opts.option readFlavor
( Opts.long "ghc-flavor"
<> Opts.help "The ghc-flavor to test against"
)
<*> Opts.strOption
( Opts.long "da-merge-base-sha"
<> Opts.help "DA flavour only. Base commit to use from the GHC repo."
<> Opts.showDefault
<> Opts.value "ghc-8.8.1-release"
)
<*> (Opts.some
(Opts.strOption
( Opts.long "da-patch"
<> Opts.help "DA flavour only. Commits to merge in from the DA GHC fork, referenced as 'upstream'. Can be specified multiple times. If no patch is specified, default will be equivalent to `--da-patch upstream/da-master-8.8.1 --da-patch upstream/da-unit-ids-8.8.1`. Specifying any patch will overwrite the default (i.e. replace, not add)."
)
)
Opts.<|> pure ["upstream/da-master-8.8.1", "upstream/da-unit-ids-8.8.1"])
<*> Opts.strOption
( Opts.long "da-gen-flavor"
<> Opts.help "DA flavor only. Flavor to pass on to ghc-lib-gen."
<> Opts.showDefault
<> Opts.value "da-ghc-8.8.1"
)
<$> (parseDaOptions
Opts.<|>
(Opts.option readFlavor
( Opts.long "ghc-flavor"
<> Opts.help "The ghc-flavor to test against"
)))
<*> parseStackOptions
where
readFlavor :: Opts.ReadM GhcFlavor
Expand All @@ -135,9 +123,29 @@ parseOptions = Options
"ghc-8.8.1" -> Right Ghc881
"ghc-8.8.2" -> Right Ghc882
"ghc-8.8.3" -> Right Ghc883
"da" -> Right Da
"ghc-master" -> Right (GhcMaster current)
hash -> Right (GhcMaster hash)
parseDaOptions :: Opts.Parser GhcFlavor
parseDaOptions =
Opts.flag' Da ( Opts.long "da" <> Opts.help "Enables DA custom build." )
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe da-ghc-flavor? Something to think about. I guess this has to be exclusive with respect to ghc-flavor. Does the parser enforce that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is my understanding of the documentation, and seems to be confirmed in usage:

$ stack runhaskell --package extra --package optparse-applicative CI.hs -- --da --ghc-flavor "ghc-8.8.1"
Invalid option `--ghc-flavor'

Usage: CI.hs (--da [--da-merge-base-sha ARG] [--da-patch ARG] 
               [--da-gen-flavor ARG] |
               --ghc-flavor ARG) [--stack-yaml ARG] [--resolver ARG] 
             [--verbosity ARG] [--cabal-verbose] [--ghc-options ARG]
  Build (and possibly upload) ghc-lib and ghc-lib-parser tarballs.
$ stack runhaskell --package extra --package optparse-applicative CI.hs -- --ghc-flavor "ghc-8.8.1" --da
Invalid option `--da'

Usage: CI.hs (--da [--da-merge-base-sha ARG] [--da-patch ARG] 
               [--da-gen-flavor ARG] |
               --ghc-flavor ARG) [--stack-yaml ARG] [--resolver ARG] 
             [--verbosity ARG] [--cabal-verbose] [--ghc-options ARG]
  Build (and possibly upload) ghc-lib and ghc-lib-parser tarballs.
$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which makes me think maybe the other flags don't need the da prefix? 🤔 This may be veering close to bikeshedding territory though. Happy to go with whatever you prefer for all flag names.

Copy link
Contributor

Choose a reason for hiding this comment

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

If they don't need the da prefix, let's loose them as an elegance.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no "upload" facilities any more right? Can the help comment be updated accordingly please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped da prefix from code and README, and removed the "upload" parenthesis from the help text.

<*> Opts.strOption
( Opts.long "da-merge-base-sha"
<> Opts.help "DA flavour only. Base commit to use from the GHC repo."
<> Opts.showDefault
<> Opts.value "ghc-8.8.1-release"
)
<*> (Opts.some
(Opts.strOption
( Opts.long "da-patch"
<> Opts.help "DA flavour only. Commits to merge in from the DA GHC fork, referenced as 'upstream'. Can be specified multiple times. If no patch is specified, default will be equivalent to `--da-patch upstream/da-master-8.8.1 --da-patch upstream/da-unit-ids-8.8.1`. Specifying any patch will overwrite the default (i.e. replace, not add)."
))
Opts.<|>
pure ["upstream/da-master-8.8.1", "upstream/da-unit-ids-8.8.1"])
<*> Opts.strOption
( Opts.long "da-gen-flavor"
<> Opts.help "DA flavor only. Flavor to pass on to ghc-lib-gen."
<> Opts.showDefault
<> Opts.value "da-ghc-8.8.1")

parseStackOptions :: Opts.Parser StackOptions
parseStackOptions = StackOptions
Expand All @@ -162,13 +170,10 @@ parseStackOptions = StackOptions
<> Opts.help "If specified, pass '--ghc-options=\"xxx\"' to stack"
))

buildDists :: GhcFlavor -> StackOptions -> String -> [String] -> String -> IO String
buildDists :: GhcFlavor -> StackOptions -> IO String
buildDists
ghcFlavor
StackOptions {stackYaml, resolver, verbosity, cabalVerbose, ghcOptions}
daMergeBaseSha
daPatches
daFlavor
=
do
let stackConfig = fromMaybe "stack.yaml" stackYaml
Expand Down Expand Up @@ -197,12 +202,12 @@ buildDists
Ghc881 -> cmd "cd ghc && git fetch --tags && git checkout ghc-8.8.1-release"
Ghc882 -> cmd "cd ghc && git fetch --tags && git checkout ghc-8.8.2-release"
Ghc883 -> cmd "cd ghc && git fetch --tags && git checkout ghc-8.8.3-release"
Da -> do
cmd $ "cd ghc && git fetch --tags && git checkout " <> daMergeBaseSha
Da { mergeBaseSha, patches } -> do
Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly what I had in mind! 🎉

cmd $ "cd ghc && git fetch --tags && git checkout " <> mergeBaseSha
-- Apply Digital Asset extensions.
cmd "cd ghc && git remote add upstream https://github.com/digital-asset/ghc.git"
cmd "cd ghc && git fetch upstream"
cmd $ "cd ghc && git -c user.name='Cookie Monster' -c user.email=cookie.monster@seasame-street.com merge --no-edit " <> Data.List.intercalate " " daPatches
cmd $ "cd ghc && git -c user.name=\"Cookie Monster\" -c user.email=cookie.monster@seasame-street.com merge --no-edit " <> Data.List.intercalate " " patches
GhcMaster hash -> cmd $ "cd ghc && git checkout " ++ hash
cmd "cd ghc && git submodule update --init --recursive"

Expand Down Expand Up @@ -236,7 +241,7 @@ buildDists
_ -> pure ()

-- Feedback on the compiler used for ghc-lib-gen.
stack $ "exec -- ghc-lib-gen ghc --ghc-lib-parser " ++ ghcFlavorOpt daFlavor ghcFlavor
stack $ "exec -- ghc-lib-gen ghc --ghc-lib-parser " ++ ghcFlavorOpt ghcFlavor
patchVersion version "ghc/ghc-lib-parser.cabal"
mkTarball pkg_ghclib_parser
renameDirectory pkg_ghclib_parser "ghc-lib-parser"
Expand All @@ -247,7 +252,7 @@ buildDists
cmd "cd ghc && git checkout ."
appendFile "ghc/hadrian/stack.yaml" $ unlines ["ghc-options:"," \"$everything\": -O0 -j"]
-- Feedback on the compiler used for ghc-lib-gen.
stack $ "exec -- ghc-lib-gen ghc --ghc-lib " ++ ghcFlavorOpt daFlavor ghcFlavor
stack $ "exec -- ghc-lib-gen ghc --ghc-lib " ++ ghcFlavorOpt ghcFlavor
patchVersion version "ghc/ghc-lib.cabal"
patchConstraint version "ghc/ghc-lib.cabal"
mkTarball pkg_ghclib
Expand Down Expand Up @@ -278,7 +283,7 @@ buildDists
-- explanation)
unlines ["extra-deps: [transformers-0.5.6.2]"]
#endif
Da ->
Da {} ->
unlines ["flags: {mini-compile: {daml-unit-ids: true}}"]
_ -> ""

Expand Down
24 changes: 12 additions & 12 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ strategy:
# +---------+-----------------+------------+
linux-ghc-master-8.10.1:
image: "Ubuntu 16.04"
ghc-flavor: "ghc-master"
mode: "--ghc-flavor ghc-master"
resolver: "ghc-8.10.1"
stack-yaml: "stack-exact.yaml"

Expand All @@ -56,12 +56,12 @@ strategy:
# +---------+-----------------+------------+
linux-ghc-8.10.1-8.10.1:
image: "Ubuntu 16.04"
ghc-flavor: "ghc-8.10.1"
mode: "--ghc-flavor ghc-8.10.1"
resolver: "ghc-8.10.1"
stack-yaml: "stack-exact.yaml"
mac-ghc-8.10.1-8.10.1:
image: "macOS-10.14"
ghc-flavor: "ghc-8.10.1"
mode: "--ghc-flavor ghc-8.10.1"
resolver: "ghc-8.10.1"
stack-yaml: "stack-exact.yaml"

Expand All @@ -73,12 +73,12 @@ strategy:
# +---------+-----------------+------------+
linux-ghc-8.8.3-8.10.1:
image: "Ubuntu 16.04"
ghc-flavor: "ghc-8.8.3"
mode: "--ghc-flavor ghc-8.8.3"
resolver: "ghc-8.10.1"
stack-yaml: "stack-exact.yaml"
mac-ghc-8.8.3-8.10.1:
image: "macOS-10.14"
ghc-flavor: "ghc-8.8.3"
mode: "--ghc-flavor ghc-8.8.3"
resolver: "ghc-8.10.1"
stack-yaml: "stack-exact.yaml"

Expand All @@ -89,7 +89,7 @@ strategy:
# +---------+-----------------+------------+
linux-ghc-master-8.8.3:
image: "Ubuntu 16.04"
ghc-flavor: "ghc-master"
mode: "--ghc-flavor ghc-master"
stack-yaml: "stack.yaml"
resolver: "nightly-2020-03-14"

Expand All @@ -100,7 +100,7 @@ strategy:
# +---------+-----------------+------------+
mac-ghc-8.8.3-8.8.3:
image: "macOS-10.14"
ghc-flavor: "ghc-8.8.3"
mode: "--ghc-flavor ghc-8.8.3"
resolver: "nightly-2020-03-14"
stack-yaml: "stack.yaml"

Expand All @@ -111,7 +111,7 @@ strategy:
# +---------+-----------------+------------+
linux-ghc-8.10.1-8.8.3:
image: "Ubuntu 16.04"
ghc-flavor: "ghc-8.10.1"
mode: "--ghc-flavor ghc-8.10.1"
resolver: "nightly-2020-03-14"
stack-yaml: "stack.yaml"

Expand All @@ -124,17 +124,17 @@ strategy:
# +---------+-----------------+------------+
linux-da-ghc-8.8.1-8.8.1:
image: "Ubuntu 16.04"
ghc-flavor: "da"
mode: "--da"
resolver: "nightly-2019-09-26"
stack-yaml: "stack.yaml"
windows-da-ghc-8.8.1-8.8.1:
image: "vs2017-win2016"
ghc-flavor: "da"
mode: "--da"
resolver: "nightly-2019-09-26"
stack-yaml: "stack.yaml"
mac-da-ghc-8.8.1-8.8.1:
image: "macOS-10.14"
ghc-flavor: "da"
mode: "--da"
resolver: "nightly-2019-09-26"
stack-yaml: "stack.yaml"

Expand All @@ -155,4 +155,4 @@ steps:
curl -sSL https://raw.github.com/ndmitchell/hlint/master/misc/travis.sh | sh -s examples/mini-compile/src
curl -sSL https://get.haskellstack.org/ | sh
stack --stack-yaml $(stack-yaml) --resolver $(resolver) setup > /dev/null
stack runhaskell --stack-yaml $(stack-yaml) --resolver $(resolver) --package extra --package optparse-applicative -- CI.hs --ghc-flavor $(ghc-flavor) --stack-yaml $(stack-yaml) --resolver $(resolver)
stack runhaskell --stack-yaml $(stack-yaml) --resolver $(resolver) --package extra --package optparse-applicative -- CI.hs $(mode) --stack-yaml $(stack-yaml) --resolver $(resolver)