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

Bump pandoc to allow up to 2.9 #68

Closed
wants to merge 7 commits into from

Conversation

cdepillabout
Copy link

@cdepillabout cdepillabout commented May 13, 2020

This PR bumps the pandoc dep to allow up to pandoc-2.9.

I want to use yesod-markdown in a project based on lts-15, which uses pandoc-2.9.

I've also done a few other things in this PR:

  • Update the default stack.yaml to use lts-15, which should test for ghc-8.8.
  • Add a stack yaml file for testing with lts-14, which should test for ghc-8.6. I also added this to CI.
  • I bumped the version number of yesod-markdown, as well as adding a changelog entry. This should hopefully make it easy to make a new release.

@cdepillabout cdepillabout marked this pull request as ready for review May 13, 2020 02:20
@cdepillabout
Copy link
Author

cdepillabout commented May 13, 2020

As far as I can tell, CI is failing on the linting phase for build_8.6.5: https://circleci.com/gh/pbrisbin/yesod-markdown/849?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

This appears to be failing during the weeder check.

However, running this locally appears to work:

$ cat stack-lts-14.27.yaml
resolver: lts-14.27

# Fix weeder with stack-2.0
ghc-options: { "$locals": -ddump-to-file -ddump-hi }
$ stack --stack-yaml stack-lts-14.27.yaml test
... (tests succeed) ...
$ stack --stack-yaml stack-lts-14.27.yaml exec -- weeder .
= Package yesod-markdown =
No warnings

I'm not sure how to fix CI here.


Also, recent advice for stack-based projects is to check the .cabal file into the repo, instead of just relying on package.yaml:

commercialhaskell/stack#5210

I didn't make this change in this PR, but maybe it could be done in a future PR.

@pbrisbin
Copy link
Owner

Oof, yeah, so this is going to be annoying...

  • build is failing due to out of memory

    This is a constant battle that sort of just ends up with me doing retries until we get lucky.

  • build_8.0.2 is failing in lint for Unknown extension: NoStaticPointers

    I'm fine if you want to set LINT=0 for that build to skip this. But I'm also fine just removing the 8.0.2 build entirely. My (loose) intention is to support current - 2 GHCs, which would really be 8.8, 8.6 and 8.4. There's no need for the 8.2 or 8.0 jobs, IMO.

  • build_8.6.5 is failing because I think it's in an incorrectly cached state

    I think you cached a build without the "fix for weeder" stanza in place. After adding that stanza, it re-used cache and didn't rebuild, so it didn't generate the hi files and so weeder still failed.

    The fix for this is to bust cache by incrementing the key prefix. But that could land you in the memory issues of build, so be prepared for that :)

Did you want to try and address these, or should I add plan to pick this up when I'm able?

@cdepillabout
Copy link
Author

@pbrisbin Thanks for the quick response.

I removed the GHC-8.0 build. I didn't touch the GHC-8.2 build, since it appears to be building fine, but if you'd like, I can remove it as well.

build_8.6.5 is failing because I think it's in an incorrectly cached state

I think you cached a build without the "fix for weeder" stanza in place. After adding that stanza, it re-used cache and didn't rebuild, so it didn't generate the hi files and so weeder still failed.

The fix for this is to bust cache by incrementing the key prefix. But that could land you in the memory issues of build, so be prepared for that :)

Great guess! This is exactly what happened.

incrementing the key prefix

Do you mean changing the build_8.6.5 prefix to something like build_8.6.5_?

I'm not too familiar with CircleCI, but on TravisCI, I believe there is a way to drop a cache for a specific build configuration from the web. Instead of changing the key prefix, would it be possible for you to just drop the cache for this build configuration, and force a rebuild?

build is failing due to out of memory

I don't have any good idea how to fix this. I guess for now we could remove this from CI, but that's somewhat unfortunate.

Alternatively, yesod-markdown could move to using GitHub Actions as a CI, since GitHub Actions runs on machines with 7G of memory:

https://help.github.com/en/actions/reference/virtual-environments-for-github-hosted-runners

As far as I could tell, Circle CI machines only have 4G of memory:

https://circleci.com/pricing/#

@pbrisbin
Copy link
Owner

Do you mean changing the build_8.6.5 prefix to something like build_8.6.5_?

By "key prefix", I mean the v3- prefixes on the caching steps. Incrementing that, say from 3 to 4, would bust all the caches and run clean builds. Unfortunately you can't do this for just the failing job.

Separately (not this PR, IMO), we could (should) change the cache key to digest the stack yaml's contents at that position in the key(s). I think it's a silly mistake that it currently spits the file name through md5 into rdigest. If it were using contents the weeder fix would've naturally busted the cache, and for only that job.

I don't have any good idea how to fix this. I guess for now we could remove this from CI, but that's somewhat unfortunate.

Theoretically, any build can fail for this reason, so removal isn't really an answer.

The status quo has been to find "big" dependencies and build them ahead of time with -j 1. It's possible we need to add more packages to this generally (there's some arguments to the Makefile to allow nightly to add specific packages here, but we could just add the same new ones always.)

Another possible option would be to update the save_cache steps to have when: always, then we'd store a cache even on these failures, presumably saving some work next time and potentially succeeding.

could move to using GitHub Actions as a CI

That's quite the can of worms. I'm not at all against it on principle (e.g. if you wanted to own that), I'm just unfamiliar with that whole ecosystem at this point, so I'm not sure I'd be able to execute that change myself for some time.

Something in the back of my mind has been to use Circle's Orbs feature to share almost all of this CI config between all my Haskell OSS as an Orb. I have some WIP in another branch that's doing that. Moving to Actions would work against that, but I suppose we could extract to something central in Actions to?

@cdepillabout
Copy link
Author

cdepillabout commented May 15, 2020

@pbrisbin I just uploaded commit 2c62576 which bumps the cache prefix from v3 to v4. Hopefully this gets things passing.

To be honest, I'm not particularly invested in yesod-markdown, I was really just hoping to get a version to Hackage with a looser dependency on pandoc, since i use yesod-markdown for one of my projects.

I added the additional CI targets (and stack.yaml file) because I thought it would be a help to you, but I'm not particularly interested in figuring out these Circle CI problems. Please feel free to take this PR over (push to my branch, etc).


As for GitHub Actions, I completely understand not wanting to switch over.

I switched to GitHub Actions for one of my Haskell projects because of the 50-minute time limit for Travis CI jobs. It was pretty easy.

However, it appears you're pretty familiar with CircleCI, so I can see how switching to GitHub Actions could be a lot of unwanted work, especially if there is some other good way to fix the out-of-memory issues.

@pbrisbin
Copy link
Owner

To be honest, I'm not particularly invested in yesod-markdown, I was really just hoping to get a version to Hackage with a looser dependency on pandoc

No worries, I'll take over this change for you and deal with CI.

@pbrisbin
Copy link
Owner

Merged in #70. I moved to the machine executor, which has more RAM. Sorry for the trouble! I'll release this in a new version this week.

@pbrisbin pbrisbin closed this May 18, 2020
@cdepillabout
Copy link
Author

Thanks a lot for taking a look at this!

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