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

feat: implement GitHub Actions based starter CI #6171

Merged
merged 5 commits into from
Aug 11, 2024

Conversation

PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Aug 4, 2024

Issue being fixed or feature implemented

We currently rely on GitLab for running CI, and while it has worked quite well, I am worried about having all of our eggs in one basket! As such, I've long wanted to explore implemeenting Github Actions based CI, but was too lazy! Well I finally spent some 60 commits trying to figure everything out, and this PR is the result of it.

As a result, we will now have two semi-redunant CI systems, the primary one on GitLab, and this one on Github Actions. Currently the GA based CI will only do one host, and does linting. Be aware this GA based CI does not actually run the tests, it does build depends, src and run linters on 1 host.
In the future, we should expand it from simply arm32 builds to having feature parity to GitLab.

While it appears the GA default runners are a bit slower than what we have on GitLab, there's a big difference, the GA runners are free :D If we decide to make the GA based CI primary, we'll probably want to setup some custom runners to have improved build speeds. Even still, a heavily cached build doing all linters took around 5 minutes if I recall correctly. Without caches I think it took maybe an hour, so defenitely not bad.

What was done?

See the individual commits, they're pretty self explanatory

How Has This Been Tested?

Lots of CI runs on my prior branch :) CI should run on this PR, and we should see how long it'll take w/o cache :D

Breaking Changes

N/A - CI only

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • [ ] I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@PastaPastaPasta PastaPastaPasta force-pushed the pr-github-ci branch 2 times, most recently from bd56cd4 to 4657a00 Compare August 6, 2024 09:07
.github/workflows/build.yml Show resolved Hide resolved
ci/dash/build_src.sh Outdated Show resolved Hide resolved
depends/Makefile Show resolved Hide resolved
ci/dash/build_src.sh Outdated Show resolved Hide resolved
knst
knst previously approved these changes Aug 8, 2024
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK 81d64e1

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK 419d51f

@UdjinM6 UdjinM6 added this to the 21.2 milestone Aug 8, 2024
@UdjinM6
Copy link

UdjinM6 commented Aug 8, 2024

Looks good but needs rebase to fix Check Merge Fast-Forward Only

PastaPastaPasta and others added 5 commits August 11, 2024 15:02
…few packages

Reasonably enough I guess, it appears that a number of our dependencies block github runner IP addresses? the blobs stored at the s3 link were downloadable from the normal location when accessed locally, but returned a 404 from github runners. These blobs were also not present in the bitcoincore depends-sources anyhow.
BACKPORT NOTICE:
we keep and maintain cppcheck linter as lint-cppcheck-dash.sh

efae252 test: Remove extended lint (cppcheck) (laanwj)

Pull request description:

  These are unreferenced in the CI and documentation, and have been since 2019 (see bitcoin#17549).

  I'm not sure the cppcheck is worthwhile. It takes a long time to run (I think this is why it isn't in the normal lints), and right
  now it only appears to find implicit constructors. The list of exceptions is out of date. But if anyone wants to bring it back at any
  time in the future they can do so from git history (and port it to Python).

ACKs for top commit:
  fanquake:
    ACK efae252

Tree-SHA512: 1a770b5d20ff1199d0d6bc471ae3d2c3438f0f0b169ce8d2fe73480daf8d3a7146c066b799afc90aa7898982c5fee79c1daca10e16e2bff0a7b38850aedd55b2
@PastaPastaPasta PastaPastaPasta merged commit de4e7e1 into dashpay:develop Aug 11, 2024
5 of 6 checks passed
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