Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Github Actions #11528

Merged
merged 75 commits into from
Mar 4, 2020
Merged

Github Actions #11528

merged 75 commits into from
Mar 4, 2020

Conversation

General-Beck
Copy link
Contributor

@General-Beck General-Beck commented Feb 27, 2020

Check, test and build code in Actions CI
Scheme:

  1. Check
  • Ubuntu
  1. Build Release Test Suite
  • Ubuntu
  • MacOS
  • Windows
  1. Run Test Suite
  • Ubuntu
  • MacOS
  • Windows
  1. Build Release Suite
  • Ubuntu
  • MacOS
  • Windows

VM and environment variables

  • Currently, Ubuntu 16.04 is used for compatibility with previous releases of Parity Ethereum. In the future, if necessary, you can switch to Ubuntu 18.04. During the transition phase, you can add ubuntu-latest to the matrix and test and build in parallel for the two versions.
  • Windows Server 2019 (The latest version of clang is used)
  • MacOS 10.15

Processes start after a push to the repository.

TODO

@General-Beck General-Beck added A0-pleasereview 🤓 Pull request needs code review. M0-build 🏗 Building and build system. M1-ci 🙉 Continuous integration. labels Feb 27, 2020
@General-Beck General-Beck added this to the 2.8 & .. milestone Feb 27, 2020
@General-Beck
Copy link
Contributor Author

Sum up. This skeleton can be used for work. In the future, we can and should expand the functionality. Add releases, nightly builds, publishing Docker images and more.
The first launch for PR will be slow (up to 3 hours), all further commits will be much faster due to caching.

Copy link
Collaborator

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Let's rename test.yml to check.yml, looks good otherwise!

Copy link

@denisgranha denisgranha left a comment

Choose a reason for hiding this comment

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

In general looks very nice, good work @General-Beck !

I have a few open questions, considering the time it takes to execute right now the CI:

  1. The build phase is the most time consuming one, instead of approaching this with sscache, couldnt we also do it with cached docker layers?
  2. Im not a rust developer so my understanding of Rust build process is a bit limited right now. I think anyway right now we lack of some data to assess potential problems/improvements. Could we add timing inside the scripts to differenciate between: a) time it takes to download cache b) time it takes to download dependencies c) time it takes to compile the code

.github/workflows/build-test.yml Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/check.yml Outdated Show resolved Hide resolved
.github/workflows/check.yml Outdated Show resolved Hide resolved
@ordian
Copy link
Collaborator

ordian commented Mar 2, 2020

The build phase is the most time consuming one, instead of approaching this with sscache, couldnt we also do it with cached docker layers?

Note that that we only care about compiling tests speed for now, as building release artifacts will be only done on releasing a new version, which doesn't happen very often. sccache is the defacto standard of caching for large rust projects, and already compile time with cache are reasonable (<13 mins on macOS https://github.com/OpenEthereum/open-ethereum/runs/476907565). But if you have ideas how it could be improved with docker layers, PRs are welcome :)

Could we add timing inside the scripts to differenciate between: a) time it takes to download cache b) time it takes to download dependencies c) time it takes to compile the code

We already have a) and c)
a) is

  • Cache cargo registry
  • Cache cargo index
  • Cache cargo build
  • Cache sccache

c) is

  • Build tests

b) is part of c) and negligible compared to compile times.

.github/workflows/check.yml Outdated Show resolved Hide resolved
@denisgranha
Copy link

@ordian my bad, You are right that caching download, etc is already measured in time, I was looking for it inside the test build stage. I was asking for that because it would not be the first time I see it takes more time to download a cache than actually get all depedencies from scratch, it happened to me on travis due to the machine hardware they use.

About the docker layer caching, I don't know if it will improve times, it might be the same or worse, but I would like to test it. Not now, as I don't have capacity at the moment but in a next iteration of this.

So far looks very good, good job both @General-Beck @ordian

@roninkaizen
Copy link

excellent work from my point of view,
thanks a lot for having a comparison to my personal settings :)

@ordian ordian merged commit 729b10e into openethereum:master Mar 4, 2020
@ordian ordian mentioned this pull request Mar 4, 2020
@General-Beck General-Beck deleted the actions branch March 5, 2020 09:13
ordian added a commit that referenced this pull request Mar 6, 2020
* master:
  ethcore: cleanup after #11531 (#11546)
  license update (#11543)
  Less cloning when importing blocks (#11531)
  Github Actions (#11528)
  Fix Alpine Dockerfile (#11538)
  Remove AuxiliaryData/AuxiliaryRequest (#11533)
  [journaldb]: cleanup (#11534)
ordian added a commit that referenced this pull request Mar 10, 2020
* master:
  Code cleanup in the sync module (#11552)
  initial cleanup (#11542)
  Warn if genesis constructor revert (#11550)
  ethcore: cleanup after #11531 (#11546)
  license update (#11543)
  Less cloning when importing blocks (#11531)
  Github Actions (#11528)
  Fix Alpine Dockerfile (#11538)
  Remove AuxiliaryData/AuxiliaryRequest (#11533)
  [journaldb]: cleanup (#11534)
  Remove references to parity-ethereum (#11525)
  Drop IPFS support (#11532)
  chain-supplier: fix warning reporting for GetNodeData request (#11530)
  Faster kill_garbage (#11514)
  [EngineSigner]: don't sign message with only zeroes (#11524)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. M0-build 🏗 Building and build system. M1-ci 🙉 Continuous integration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants