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
drop da prefix for subflags; remove mention of upload
  • Loading branch information
@garyverhaegen-da
garyverhaegen-da committed Jun 3, 2020
commit e550d5603fa096498620b836ac08ddc098baa6c7
10 changes: 5 additions & 5 deletions CI.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ main = do
let opts =
Opts.info (parseOptions Opts.<**> Opts.helper)
( Opts.fullDesc
<> Opts.progDesc "Build (and possibly upload) ghc-lib and ghc-lib-parser tarballs."
<> Opts.progDesc "Build ghc-lib and ghc-lib-parser tarballs."
<> Opts.header "CI - CI script for ghc-lib"
)
Options { ghcFlavor, stackOptions } <- Opts.execParser opts
Expand Down Expand Up @@ -126,20 +126,20 @@ parseOptions = Options
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.long "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.long "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 `--patch upstream/da-master-8.8.1 --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.long "gen-flavor"
<> Opts.help "DA flavor only. Flavor to pass on to ghc-lib-gen."
<> Opts.showDefault
<> Opts.value "da-ghc-8.8.1")
Expand Down
14 changes: 7 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ general case: the `--ghc-flavor` flag is replaced with an "enabling" flag
stack runhaskell --package extra \
--package optparse-applicative \
CI.hs -- --da \
--da-merge-base-sha=ghc-8.8.1-release \
--da-patch=upstream/da-master-8.8.1 \
--da-patch=upstream/da-unit-ids-8.8.1 \
--da-gen-flavor=da-ghc-8.8.1
--merge-base-sha=ghc-8.8.1-release \
--patch=upstream/da-master-8.8.1 \
--patch=upstream/da-unit-ids-8.8.1 \
--gen-flavor=da-ghc-8.8.1
```

The DAML-specific process only differs from the normal one in that it patches
Expand All @@ -74,13 +74,13 @@ GHC with the given patches. More specifically, it will:
- Clone GHC. (This is also done by the normal workflow.)
- Add the [DA fork](https://github.com/digital-asset/ghc/) of GHC as a remote
named `upstream`.
- Checkout the commit provided as `da-merge-base-sha`.
- Checkout the commit provided as `merge-base-sha`.
- Create a new commit by merging in all of the commits specified through the
`--da-patch` flags.
`--patch` flags.
- Proceed as normal for the rest of the workflow.

At some later stage, the workflow calls out to the `ghc-lib-gen` program, and
at that point it needs to pass in a "flavor" argument; it will use the value of
the `--da-flavor` option for that.
the `--gen-flavor` option for that.
shayne-fletcher marked this conversation as resolved.
Show resolved Hide resolved

Note that deployment for the DAML version is handled from within the DAML CI.