Skip to content

Set --haddock flag based on BUILD_TYPE per joneshf#3409

Merged
kritzcreek merged 2 commits into
purescript:masterfrom
justinwoo:test-build-script-changes
Aug 11, 2018
Merged

Set --haddock flag based on BUILD_TYPE per joneshf#3409
kritzcreek merged 2 commits into
purescript:masterfrom
justinwoo:test-build-script-changes

Conversation

@justinwoo

@justinwoo justinwoo commented Aug 8, 2018

Copy link
Copy Markdown
Contributor

Based on #3389 (comment)

Minor change, but if this gets things going, can we unblock other PRs?

@justinwoo

Copy link
Copy Markdown
Contributor Author

I guess this still overruns though...

@kritzcreek

kritzcreek commented Aug 9, 2018

Copy link
Copy Markdown
Member

@justinwoo Try rebasing on master now. I had successful CI runs against GHC 8.4.3

@justinwoo justinwoo force-pushed the test-build-script-changes branch from eaf437a to 34a8651 Compare August 9, 2018 09:28
@hdgarrood

Copy link
Copy Markdown
Contributor

This may not have been clear, but the script is actually working as intended. We expect to see failures like in https://travis-ci.org/purescript/purescript/builds/413820789 occasionally.

  • A Travis CI job may end with status succeeded, errored, or failed.
  • If a job takes more than 50 minutes, it is immediately canceled and marked as errored.
  • Travis only stores the build cache on a succeeded or failed job; if a job is marked as errored, the build cache is not stored.

We have enough dependencies that installing them all is likely to take almost all of this time window up, leaving insufficient time to actually run the tests. We therefore rely on the build cache containing the (compiled) dependencies so that we can run the tests during CI and not have to worry about it timing out.

As we have seen, installing all the dependencies and then running the tests can take more than 50 minutes on Travis. Therefore, when this does happen, we need to store the progress which has been made installing dependencies so that we can ask Travis to start a new job (by pushing a new commit), and have the new job pick up where the previous one left off.

However, since build caches are not stored on errored builds, we need to intervene to ensure that builds which are taking too long get marked as failed instead. This is why we have this part in the build script:

# Setup & install dependencies or abort
ret=0
$TIMEOUT 45m $STACK --install-ghc build \
--only-dependencies --test --haddock \
|| ret=$?
case "$ret" in
0) # continue
;;
124)
echo "Timed out while installing dependencies."
echo "Try pushing a new commit to build again."
exit 1
;;
*)
echo "Failed to install dependencies."
exit 1
;;
esac

The timeout command aborts the command you ask it to run if it takes longer than the time you specify. In this case, it exits with the code 124. In this case we catch that and the build script exits with a code of 1; this aims to ensure that we don't trigger Travis' own job timeout mechanism, so that the build is marked as failed rather than errored.

In the case that this happens, all you need to do is push an empty commit to ensure that Travis picks up where it left off. Also, you need to push a new commit instead of amending a previous one: Travis looks at the commit parent to decide which cache to use, so if you instead amend the previous commit and push it again, the cache will be in the same state as in was at the beginning of the previous attempt, which means that the progress made in the previous attempt is lost.

Does that make sense?

@hdgarrood

Copy link
Copy Markdown
Contributor

I previously set a timeout of 45 minutes (see line 8 in the above code snippet), thinking that the remaining 5 minutes would be sufficient to complete the rest of the CI job. The fact that these jobs (eg https://travis-ci.org/purescript/purescript/jobs/413970257) are erroring shows that this is no longer the case, so we need to decrease this timeout. Pursuit's is currently set to 40 minutes; that's probably appropriate here too.

@justinwoo

Copy link
Copy Markdown
Contributor Author

Okay, let's try that

@justinwoo justinwoo force-pushed the test-build-script-changes branch from 873cdda to d0b3bf2 Compare August 9, 2018 16:18
@justinwoo

Copy link
Copy Markdown
Contributor Author

Okay, it built on a shorter timer this time

@kritzcreek kritzcreek merged commit f6cf990 into purescript:master Aug 11, 2018
justinwoo added a commit to justinwoo/purescript that referenced this pull request Aug 12, 2018
* Set --haddock flag based on BUILD_TYPE per joneshf

* set lower timeout
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.

3 participants