rippled icon indicating copy to clipboard operation
rippled copied to clipboard

Build and test rippled in Github Actions, plus some related fixes and improvements

Open ximinez opened this issue 4 years ago • 4 comments

High Level Overview of Change

The primary focus of this PR is building and testing rippled using Github Actions. Along the way, some other issues came up that either needed to be fixed or were convenient improvements. The changes are divided into several different commits, which can (and probably should) be reviewed independently - they probably could have been independent PRs, but I didn't want to get hung up on the dependencies.

These commits should NOT be squashed down to a single commit for merging, unless there is a consensus otherwise.

Note that I restructured the directories for external dependencies (.nih_c). Until this is merged, if building in an existing repo, you will almost certainly need to move or delete that folder first, then move or delete again when switching to another branch.

Context of Change

Travis CI can be slow and unreliable. Github Actions offers some improvement in those two areas. It's still not as fast as I'd like, but that can potentially be addressed in the future with hosted runners.

The new platform also offers different ways to organize the jobs, and manage caches and artifacts. For example,

  • The tasks are organized across 7 different mostly-independent workflow files. If a failure occurs in one job, that workflow can be re-run without worrying about dependencies on others.
  • Every job is organized into steps, which can be expanded and collapsed on github.com, reducing the need to scroll or search through multi-thousand line log files.
  • Caches are accessed using keys, which are computed based on the CMake config and rippled source code, and are immutable once written. This should mitigate or prevent "corrupt" cache issues, and reduce or eliminate the need to manually delete caches for PRs and branches.
  • Logs and executables are provided as artifacts. This allows developers to get more information more easily, and debug the executable built by the job in the event of CI-only problems. It also speeds up test-only steps by reusing executables instead of rebuilding them.

Type of Change

  • [x ] Refactor (non-breaking change that only restructures code)
  • [x ] Tests (You added tests for code that already exists, or your new feature included in this PR)
  • [x ] Release

Test Plan

There isn't much to test other than verifying that the CI jobs succeed. Unit tests were added, and some changes were made to non-test code, but they shouldn't affect any run-time functionality.

Future Tasks

As mentioned above, I would eventually like to get these jobs running on better hardware, either some kind of paid-tier resource via Github, or self-hosted runners.

ximinez avatar May 19 '21 19:05 ximinez

Some data for comparison:

The Github Actions all ran simultaneously. This list indicates how long each of them took.

  • Linux, gcc-8 release unity: 55m 49s
  • Linux, clang-8 debug unity: 55m 9s
  • Linux, clang-8 release unity: 1h 24m 1s
  • Linux, gcc-8 debug unity: 1h 46m 12s
  • MacOS: 1h 51m 45s
  • Linux: 2h 1m 24s
  • Windows: 3h 0m 54s

All of them succeeded. I expect they'll run faster in the future as caches are seeded.

Travis took a little longer to get started, and when it did, it still took longer to run all the jobs.

  • Ran for 6 hrs 38 min 59 sec
  • Total time 22 hrs 9 min 15 sec

And to top it off, two of the tasks failed:

  • ipv6 (macos)
  • windows, visual studio, debug

I've restarted those to make sure there is not a real problem, but I doubt it.

I can't make any promises that the Github jobs will never have a spurious failure, or start this quickly when more of us start using it, but these numbers look pretty good.

ximinez avatar May 24 '21 17:05 ximinez

NIce PR @ximinez !

carlhua avatar May 24 '21 18:05 carlhua

@thejohnfreeman @manojsdoshi @legleux @xzhaous

I just pushed a bunch of commits that fix more issues, and address most, if not all, of the feedback so far. To make reviewing a tiny bit easier, I didn't rewrite any of the commit history you may have already seen, and I tried to organize the commits logically. If this PR gets approved, I'll squash them down into the single "Build and test rippled in Github Actions" commit, other than the unit test fixes.

Highlights:

  • The build-action has no parameters
  • I think I've resolved occasional build failures due to OOM. For now.
  • I think I've resolved occasional test failures by restricting the number of unittest processes in jobs that tended to fail.
  • Job names are much cleaner and clearer.
  • Caches are better organized, especially in Mac and Windows builds that have relatively static dependencies.
  • Documentation

To answer one lingering question: I left the Travis configuration in for now, so we can run them side-by-side for a while until we're confident that the real-world performance and reliability of Github Actions is at least as good.

If you've been holding off reviewing this because I told you I was working on stuff, I am done, and this is ready to review again. For now. 🤞

ximinez avatar Jul 15 '21 22:07 ximinez

Rebased on to 1.8.0-b4. No other changes. I see a couple of jobs have failed. I'll look into that.

ximinez avatar Jul 29 '21 20:07 ximinez

(notes: not ready to merge; need @thejohnfreeman to make the Conan changes before we can do this)

intelliot avatar Nov 28 '22 19:11 intelliot

@ximinez given conan, what is the best path forward (or not) for this PR?

intelliot avatar Jan 18 '23 23:01 intelliot

I've changed this PR to be a draft, indicating that it's not ready for prime time. I would like to integrate it with Conan to build the dependencies, but use the much larger configuration matrix in these workflow files to get more test coverage, but it's not a high priority. (I don't necessarily have to do it if someone else wants to pick up the baton.)

ximinez avatar Jan 18 '23 23:01 ximinez

  • goals and matrix here may be useful
  • there have been so many changes that it may be better to start over and just take what's useful from here

We'll close this PR but the branch will remain, and @thejohnfreeman will look at it to copy over the useful parts.

See https://github.com/XRPLF/rippled/issues/4371#issuecomment-1360693790

intelliot avatar Apr 19 '23 18:04 intelliot