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

Travis workarounds #3694

Closed
wants to merge 3 commits into from
Closed

Travis workarounds #3694

wants to merge 3 commits into from

Conversation

ximinez
Copy link
Collaborator

@ximinez ximinez commented Dec 9, 2020

High Level Overview of Change

Several other branches and pull requests have had many of their Travis CI jobs fail with errors that look related to the Travis CI cache, despite the relevant caches (or all caches in some cases) being deleted. These commits attempt to work around these apparent Travis failures.

It is currently split into 3 commits:

  1. Reset corrupt / inconsistent dependency source folders works automatically. It looks at several nih_c folders which are expected to be git repos and deletes them, along with their cmake-related tracking folders, if they aren't. This will trigger cmake to make a fresh clone of the repos. It also dumps the contents of the cmake log files after the generation step so that they can be examined for more information when there are failures, and shows which files and dirs are modified in the cache folder before the cache is pushed, so this may possibly be optimized in the future.
  2. Simplify travis config moves a couple of things around for readability and squashes several of the windows prerequisite steps into one. By itself, this doesn't resolve any errors, but it should speed up the build because the combined prereq step only needs to spin up one machine, read and write the cache once, etc. instead of three times. It also prepares the code for the third commit.
  3. Force wipe the travis caches with "travis_clean_cache" AKA "drop back and punt" adds some logic to completely wipe (rm -rf) the cache directory before building if the top commit message on the branch or PR contains the string "travis_clean_cache". (That includes this commit.) In cases where the automated cleanups don't work, any developer can put this string in their branch's last commit to force everything to reset.

Until this is merged, developers that are having unexplained Travis build issues can make use of these changes in one of the following ways:

  • Rebase their branch on top of this branch, and either let the automatic checks take care of things or add / change their top commit to include the magic "travis_clean_cache" string. I will make my best efforts to keep this branch up to date with develop, but feel free to nudge me if not.
  • Cherry pick the relevant commits on top of their changes. If their build appears to be failing with errors that look like a corrupted git repo (e.g. https://travis-ci.com/github/ripple/rippled/jobs/454113921#L743), the Reset corrupt / inconsistent dependency source folders should be sufficient. (Sufficiency not guaranteed 😁 .) Otherwise, cherry pick the last two together - if the last commit is the head of your branch, the reset will occur automatically.
    In principal, once the issue is resolved, these commits can be removed from the other branch for further development and/or merging.

Context of Change

This appears to be a problem on the side of Travis CI, and these changes are an attempt at a workaround.

Type of Change

  • [X ] Bug fix (non-breaking change which fixes an issue)
  • [X ] New feature (non-breaking change which adds functionality)

Test Plan

  • Push a normal commit when a cache is not expected. Expected result: Travis CI succeeds.
  • Push a commit when a cache is expected, or restart the job from step one. Travis CI may succeed or fail. 🤷‍♂️
  • Push a commit containing the string "travis_clean_cache". Expected result: Regardless of the result from step two, Travis CI should succeed.

Future Tasks

I noticed that Travis updates the cache on almost every job, even when the dependencies are up to date. At some point I'd like to determine what's changing, and trying to find ways to prevent them from changing or move them out of the cache.


This change is Reviewable

* Also display some more information about the build
* Move all the vcpkg windows dependency installations into one step.
* Move the unmodified `before_install` step above the matrix to improve
  readability, because this step runs before any of the matrix steps.
Comment on lines +56 to +57
- if [ "${MATRIX_EVAL}" != "" ] ; then eval "${MATRIX_EVAL}"; fi
- if [ "${CMAKE_ADD}" != "" ] ; then export CMAKE_EXTRA_ARGS="${CMAKE_EXTRA_ARGS} ${CMAKE_ADD}"; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

These shouldn't need to be conditional, but that's up to you. I see you just moved these lines.

Copy link
Collaborator 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 leaving them as is. I only move this block because it put the block in more of a chronological order along with the rest of the instructions.

@thejohnfreeman
Copy link
Collaborator

Are we expecting to make these permanent changes, or just a temporary workaround until the root cause can be identified?

@ximinez
Copy link
Collaborator Author

ximinez commented Dec 15, 2020

If we ever identify a root cause - or move away from Travis entirely - we'll be able to safely remove these. But if the problem persists, then it makes sense to keep them indefinitely.

@nbougalis nbougalis mentioned this pull request Dec 17, 2020
@ximinez ximinez deleted the travis-workarounds branch December 21, 2020 15:15
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