Skip to content

Commit

Permalink
Merge bitcoin#29274: Fix issues with CI on forks
Browse files Browse the repository at this point in the history
576828e ci: test-each-commit merge base optional (Sjors Provoost)
e9bfbb5 ci: forks can opt-out of CI branch push (Cirrus only) (Sjors Provoost)

Pull request description:

  Maintainer note: `SKIP_BRANCH_PUSH=true` must be set in Cirrus for `bitcoin-core/gui` before merging this. See `https://cirrus-ci.com/github/bitcoin-core/gui` -> Settings.

  ---

  I find myself making pull requests against my fork (mostly on top of bitcoin#28983, or asking others to do so. Currently only the Github actions are run on forks, because we use self-hosted runners for the Cirrus tasks.

  While setting up my own self-hosted runners for my fork, I ran into a number of issues. Some of those were addressed by bitcoin#29441, but remaining issues are:

  1. When PRs are opened in the fork, cirrus CI jobs are run twice because PRs and branches reside in the same repository, rather than a main repository and a fork repository, as is the case with bitcoin/bitcoin PRs. Fix this by adding a `SKIP_BRANCH_PUSH` configuration option that allows skipping CI runs not directly associated with a PR. The fix is a generalization of [bitcoin#20328](bitcoin#20328), which fixed a similar problem for the bitcoin-core/gui mirror repository, and it allows removing a hardcoded reference to that repository.

      Github actions jobs will still run twice despite this change, see [bitcoin#29274 (comment)](bitcoin#29274 (comment)). Initially this PR tried to prevent that with bitcoin@b9fdd0d, but this had some potentially negative side effects, see [bitcoin#29274 (comment)](bitcoin#29274 (comment)), so that commit was dropped for now.

  2. When PRs are opened in the fork, the "test-each-commit" github action can fail due to not being able to find a recent merge commit. This problem doesn't happen in the bitcoin/bitcoin repository because branches in this repository used as the base for pull requests always point at merge commits.

  This PR replaces bitcoin#29259 using the self hosted workers via Cirrus instead of Github.

  You can see this PR in action on this pull request to my fork: Sjors#30

  To test it yourself:

  1. spin up at least two [self hosted runners](https://github.com/cirruslabs/cirrus-cli/blob/master/PERSISTENT-WORKERS.md). Either use a seperate VM for each, or give them their own user.
  3. Install Podman and other CI dependencies (see .cirrus.yml)
  4. Give Cirrus access to your fork at https://cirrus-ci.com/settings/github/YOU
  5. Get a token from Cirrus and use it to start your worker(s)
  6. Optionally set SKIP_BRANCH_PUSH=true ~and NO_ARM=true~ env variables (see .cirrus.yml)
  make a pull request to your own fork, with this PR as the base branch

  Security wise: when dealing with code from strangers on the internet, review it first before running the CI. There's a Cirrus check-box that requires approval for people without write access to trigger CI.

ACKs for top commit:
  maflcko:
    ACK 576828e
  ryanofsky:
    Code review ACK 576828e.

Tree-SHA512: fb6be2f228aa62f45a65ce5c613c979b6f387df396f9601ce4622b27aa317a66f198e7d7a194592b0bb397b32a2f50f8be47065834d74af4ea09407c5c8d306d
  • Loading branch information
ryanofsky committed Jul 10, 2024
2 parents 9adebe1 + 576828e commit 45f757c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
12 changes: 11 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ env: # Global defaults
# The above machine types are matched to each task by their label. Refer to the
# Cirrus CI docs for more details.
#
# When a contributor maintains a fork of the repo, any pull request they make
# to their own fork, or to the main repository, will trigger two CI runs:
# one for the branch push and one for the pull request.
# This can be avoided by setting SKIP_BRANCH_PUSH=true as a custom env variable
# in Cirrus repository settings, accessible from
# https://cirrus-ci.com/github/my-organization/my-repository
#
# On machines that are persisted between CI jobs, RESTART_CI_DOCKER_BEFORE_RUN=1
# ensures that previous containers and artifacts are cleared before each run.
# This requires installing Podman instead of Docker.
Expand Down Expand Up @@ -59,7 +66,10 @@ env: # Global defaults

# https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks
filter_template: &FILTER_TEMPLATE
skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == "" # No need to run on the read-only mirror, unless it is a PR. https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution
# Allow forks to specify SKIP_BRANCH_PUSH=true and skip CI runs when a branch is pushed,
# but still run CI when a PR is created.
# https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution
skip: $SKIP_BRANCH_PUSH == "true" && $CIRRUS_PR == ""
stateful: false # https://cirrus-ci.org/guide/writing-tasks/#stateful-tasks

base_template: &BASE_TEMPLATE
Expand Down
8 changes: 7 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@ jobs:
# and the ^ prefix is used to exclude these parents and all their
# ancestors from the rev-list output as described in:
# https://git-scm.com/docs/git-rev-list
echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD ^$(git rev-list -n1 --merges HEAD)^@ | head -1)" >> "$GITHUB_ENV"
MERGE_BASE=$(git rev-list -n1 --merges HEAD)
EXCLUDE_MERGE_BASE_ANCESTORS=
# MERGE_BASE can be empty due to limited fetch-depth
if test -n "$MERGE_BASE"; then
EXCLUDE_MERGE_BASE_ANCESTORS=^${MERGE_BASE}^@
fi
echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD $EXCLUDE_MERGE_BASE_ANCESTORS | head -1)" >> "$GITHUB_ENV"
- run: |
sudo apt-get update
sudo apt-get install clang ccache build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
Expand Down

0 comments on commit 45f757c

Please sign in to comment.