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

Change to build-type: Simple #14

Merged
merged 1 commit into from
Feb 5, 2019
Merged

Conversation

phadej
Copy link
Contributor

@phadej phadej commented Feb 5, 2019

cabal new-build generates .ghc.environment files, so we
can run doctest on sources, and package db info will be picked implicitly.
This works on GHC >= 8.0. In short

cabal new-install -z doctest
cabal new-build
doctest src

If there aren't -z flag in your cabal, use a workaround to install
doctest:

(cd /tmp && cabal new-install doctest)

stack users can stack exec doctest src too, as stack exec will
provide package environment in an environment variable.

The only downside, is that doctest binary have to correspond
to the GHC in use. This is not a problem in CI, but is inconvenient
locally.

`cabal new-build` generates .ghc.environment files, so we
can run `doctest` on sources using package db there. This works on
GHC >= 8.0. In short

   cabal new-install -z doctest
   cabal new-build
   doctest src

If there aren't `-z` flag in your `cabal`, use a workaround to install
doctest:

   (cd /tmp && cabal new-install doctest)

`stack` users can `stack exec doctest src` too, as `stack exec` will
provide package environment in an environment variable.

The only downside, is that `doctest` binary have to correspond
to the GHC in use. This is not a problem in CI, but is inconvenient
locally.
@phadej phadej force-pushed the build-type-simple branch from 53cffeb to 571329f Compare February 5, 2019 12:28
@phadej
Copy link
Contributor Author

phadej commented Feb 5, 2019

Green, "resolves" #13

@@ -119,6 +116,9 @@ install:
cabal new-update head.hackage -v
fi
- grep -Ev -- '^\s*--' ${HOME}/.cabal/config | grep -Ev '^\s*$'
- (cd /tmp && echo '' | cabal new-repl -w ${HC} --build-dep fail)
- if [ $HCNUMVER -ge 80000 ]; then cabal new-install -w ${HC} -j2 --symlink-bindir=$HOME/.local/bin doctest --constraint='doctest ==0.16.*'; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

if [ $HCNUMVER -ge 80000 ]

I'm rather uncomfortable with hard-coding the GHC version to an old release like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this approach doesn't work with older GHCs, cannot lift this requirements with current tools

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I'm not suggesting that we run this on every version of GHC, but only on the latest release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's -ge so it will run on all versions >= 8.0 (currently doctests are run for all versions)

Copy link
Collaborator

@RyanGlScott RyanGlScott Feb 5, 2019

Choose a reason for hiding this comment

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

Ah, that makes this mildly more tolerable, then. I can live this slightly suboptimal state of affairs.

@@ -119,6 +116,9 @@ install:
cabal new-update head.hackage -v
fi
- grep -Ev -- '^\s*--' ${HOME}/.cabal/config | grep -Ev '^\s*$'
- (cd /tmp && echo '' | cabal new-repl -w ${HC} --build-dep fail)
- if [ $HCNUMVER -ge 80000 ]; then cabal new-install -w ${HC} -j2 --symlink-bindir=$HOME/.local/bin doctest --constraint='doctest ==0.16.*'; fi
- if [ $HCNUMVER -eq 80403 ]; then cabal new-install -w ${HC} -j2 --symlink-bindir=$HOME/.local/bin hlint --constraint='hlint ==2.1.*'; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's an issue about this in haskell-ci and I'll look into it (no promises when though).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, I'm not willing to accept this PR unless this issue is resolved.

If we do decide to go with this option, I'll add a hack to travis-maintenance for this.

@phadej
Copy link
Contributor Author

phadej commented Feb 5, 2019

FWIW, this is a demo. It's up to Neil to persuade you Ryan. No offence taken if this PR is rejected.

@phadej
Copy link
Contributor Author

phadej commented Feb 5, 2019 via email

@RyanGlScott
Copy link
Collaborator

Here are all of the hacks that I currently use in travis-maintenance:

data Hack
  = HLint [String]
  | DisableTestsGlobally
  | AlternateConfig [String]
  | CabalProjectMiscellanea [String]

Here is (in my mind) what each hack maps to in terms of haskell-ci issues:

  1. HLint: hlint is hard-coded to be installed on GHC 8.4.3 (not the latest GHC) haskell-CI/haskell-ci#176
  2. DisableTestsGlobally: I couldn't find one specifically for this issue, but I only use this to work around test-suite failures ku-fpg/blank-canvas#73, and I'm reasonably sure that there's a better way to accomplish this (that I haven't figured out yet).
  3. AlternateConfig: constraint-sets don't encompass tests, benchmarks, or Haddocks haskell-CI/haskell-ci#182
  4. CabalProjectMiscellanea: haskell-ci breaks with source-repository-packages haskell-CI/haskell-ci#184

@RyanGlScott RyanGlScott merged commit 571329f into ekmett:master Feb 5, 2019
@phadej phadej deleted the build-type-simple branch February 5, 2019 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants