ci: Avoid CI failures from temp env file reuse#29441
Merged
fanquake merged 2 commits intobitcoin:masterfrom Feb 20, 2024
Merged
ci: Avoid CI failures from temp env file reuse#29441fanquake merged 2 commits intobitcoin:masterfrom
fanquake merged 2 commits intobitcoin:masterfrom
Conversation
This also avoids the sudo requirement for self-hosted CI runners.
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
Sjors
approved these changes
Feb 16, 2024
BrandonOdiwuor
approved these changes
Feb 16, 2024
Contributor
BrandonOdiwuor
left a comment
There was a problem hiding this comment.
ACK fa91bf2
Looks good to me
hebasto
reviewed
Feb 19, 2024
| @@ -55,7 +55,7 @@ base_template: &BASE_TEMPLATE | |||
| << : *FILTER_TEMPLATE | |||
| merge_base_script: | |||
| # Unconditionally install git (used in fingerprint_script). | |||
Member
Author
There was a problem hiding this comment.
git is still installed unconditionally, otherwise, it wouldn't be present, no?
Might replace "install" with "require" if I re-touch, or in a follow-up?
hebasto
approved these changes
Feb 19, 2024
Member
Author
|
rfm? :) |
Contributor
|
Post merge ACK fa91bf2. I ran multiple CI jobs at the same time and observed the nicer naming in the /tmp directory. |
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Mar 13, 2024
Github-Pull: bitcoin#29441 Rebased-From: c65fde4
luke-jr
pushed a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Mar 13, 2024
This also avoids the sudo requirement for self-hosted CI runners. Github-Pull: bitcoin#29441 Rebased-From: fa91bf2
ryanofsky
added a commit
that referenced
this pull request
Jul 10, 2024
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 #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 #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 [#20328](#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 [#29274 (comment)](#29274 (comment)). Initially this PR tried to prevent that with b9fdd0d, but this had some potentially negative side effects, see [#29274 (comment)](#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 #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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, running separate CI tasks at the same time may intermittently fail, because they race to read/write
/tmp/env. Fix this by adding$CONTAINER_NAMEto the file name.Also, add
$USER, while touching the line, to allow different users to run the same CI task at the same time.Also, skip the git install if there is no need.
Ref: #29274