Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -334,20 +334,3 @@ task:
env:
MACOS_SDK: "Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers"
<< : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV

task:
name: 'macOS 13 native arm64 [gui, sqlite only] [no depends]'
Copy link
Member

Choose a reason for hiding this comment

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

nit: if there is any opinion on the pr/commit title not being sufficient, you could make it something like the following (and add a body):

ci: Run "macOS native" job on Github Actions, revert back to x86_64

Updates the macOS native ci job to run on Github Actions which will
allow us to stay on a free plan for the job. Additionally, temporarily
revert back to running on a x86_64 architecture until Github Actions
has a arm64 (Apple Silicon) runner; which is preferable to no macOS native
ci job.

macos_instance:
# Use latest image, but hardcode version to avoid silent upgrades (and breaks)
image: ghcr.io/cirruslabs/macos-ventura-xcode:14.3.1 # https://cirrus-ci.org/guide/macOS
<< : *BASE_TEMPLATE
check_clang_script:
- clang --version
brew_install_script:
- brew install boost libevent qt@5 miniupnpc libnatpmp ccache zeromq qrencode libtool automake gnu-getopt
<< : *MAIN_TEMPLATE
env:
<< : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
CI_USE_APT_INSTALL: "no"
PACKAGE_MANAGER_INSTALL: "echo" # Nothing to do
FILE_ENV: "./ci/test/00_setup_env_mac_native_arm64.sh"
59 changes: 59 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

name: CI
on:
# See: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request.
pull_request:
# See: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#push.
push:
branches:
- '**'
tags-ignore:
- '**'

env:
DANGER_RUN_CI_ON_HOST: 1
TEST_RUNNER_TIMEOUT_FACTOR: 40
Copy link
Member

@maflcko maflcko Aug 15, 2023

Choose a reason for hiding this comment

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

Forgot CI_FAILFAST_TEST_LEAVE_DANGLING ? Otherwise, it would be good to see a functional test failure log

Edit: Done in #28278

Copy link
Member

@maflcko maflcko Aug 15, 2023

Choose a reason for hiding this comment

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

Also, CCACHE_NOHASHDIR? Or maybe that'd be better moved to ./ci/test/00_setup_env.sh

Edit: Not yet done.


jobs:
macos-native-x86_64:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment linking to github/roadmap#528 to say that this should be used, when available?

Copy link
Member

Choose a reason for hiding this comment

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

Done in #28278

name: macOS 13 native, x86_64 [no depends, sqlite only, gui]
# Use latest image, but hardcode version to avoid silent upgrades (and breaks).
# See: https://github.com/actions/runner-images#available-images.
runs-on: macos-13

# No need to run on the read-only mirror, unless it is a PR.
if: github.repository != 'bitcoin-core/gui' || github.event_name == 'pull_request'

timeout-minutes: 120
Copy link
Member

Choose a reason for hiding this comment

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

Should also abort on a force push, to avoid wasting CPU, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

GHActions have no such a feature by default. It should be a separated workflow that processes pull_request.synchronize event and terminates other workflows. However, I'm not sure about the exact implementation for now.

Copy link
Member

Choose a reason for hiding this comment

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

So if someone (honestly) pushes to a pull request a few times in a few minutes, it will block CI progress on all other pulls, assuming a few more tasks are run on GHA and the "free" limit is reached?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should also abort on a force push, to avoid wasting CPU, no?

Done in #28282.


env:
MAKEJOBS: '-j4'
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to reduce this to 4 when it previously was 10?

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #28278

CI_USE_APT_INSTALL: 'no'
Copy link
Member

Choose a reason for hiding this comment

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

unrelated: Maybe remove this and replace it with a check on CI_OS_NAME?

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #28278

PACKAGE_MANAGER_INSTALL: 'echo' # Nothing to do
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think this can be removed?

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #28278

FILE_ENV: './ci/test/00_setup_env_mac_native.sh'

steps:
- name: Checkout
uses: actions/checkout@v3

- name: Clang version
run: clang --version

- name: Install Homebrew packages
run: brew install boost libevent qt@5 miniupnpc libnatpmp ccache zeromq qrencode libtool automake gnu-getopt

- name: Set Ccache directory
run: echo "CCACHE_DIR=${RUNNER_TEMP}/ccache_dir" >> "$GITHUB_ENV"

- name: Ccache cache
uses: actions/cache@v3
with:
path: ${{ env.CCACHE_DIR }}
key: ${{ github.job }}-ccache-cache-${{ github.run_id }}
restore-keys: ${{ github.job }}-ccache-cache
Copy link
Member

Choose a reason for hiding this comment

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

Looks like each pull request will rewrite the ccache and thus the hit rate will be ~0% overall?

Would be better to get the cache from the own pull only, or master.

Copy link
Member

Choose a reason for hiding this comment

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

Yea, looks like caching doesn't work at all here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

If it doesn't work, why was any of this added to the CI template?

Copy link
Member

Choose a reason for hiding this comment

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

I think it works when there are no pull requests. It should be possible to fix this in a follow-up.

Copy link
Member Author

@hebasto hebasto Aug 16, 2023

Choose a reason for hiding this comment

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

If it doesn't work, why was any of this added to the CI template?

Ccache regression, introduced in #19683, should be fixed. It is not related to the CI template that is correct.

Copy link
Member

Choose a reason for hiding this comment

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

How is this related. This is the "macOS native", not "macOS-cross" task.

Copy link
Member Author

Choose a reason for hiding this comment

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

How is this related. This is the "macOS native", not "macOS-cross" task.

My bad. Sorry for the noise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, looks like caching doesn't work at all here?

It works.

I think it works when there are no pull requests. It should be possible to fix this in a follow-up.

Yes, pull requests should not create their own caches. Going to address this issue shortly.


- name: CI script
run: ./ci/test_run_all.sh
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
#!/usr/bin/env bash
#
# Copyright (c) 2019-2022 The Bitcoin Core developers
# Copyright (c) 2019-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

export LC_ALL=C.UTF-8

export HOST=arm64-apple-darwin
export HOST=x86_64-apple-darwin
export PIP_PACKAGES="zmq"
export GOAL="install"
export BITCOIN_CONFIG="--with-gui --with-miniupnpc --with-natpmp --enable-reduce-exports"
export CI_OS_NAME="macos"
export NO_DEPENDS=1
export OSX_SDK=""
export CCACHE_MAXSIZE=300M
export CCACHE_MAXSIZE=400M
export RUN_FUZZ_TESTS=true
export FUZZ_TESTS_CONFIG="--exclude banman" # https://github.com/bitcoin/bitcoin/issues/27924