Skip to content

CI test#30

Closed
Sjors wants to merge 4 commits into2023/01/ci-forkfrom
2024/01/ci-test
Closed

CI test#30
Sjors wants to merge 4 commits into2023/01/ci-forkfrom
2024/01/ci-test

Conversation

@Sjors
Copy link
Owner

@Sjors Sjors commented Jan 18, 2024 •
edited
Loading

This tests CI for a pull request on a fork against a branch that's not master.

Demo for bitcoin#29274

@Sjors Sjors force-pushed the 2024/01/ci-test branch 19 times, most recently from 15240d1 to e92bdbf Compare January 18, 2024 14:36
@Sjors Sjors mentioned this pull request Jan 18, 2024
Merged
@Sjors Sjors force-pushed the 2024/01/ci-test branch 9 times, most recently from 4979aae to 5d88b3d Compare January 18, 2024 17:23
@maflcko
Copy link

maflcko commented May 28, 2024

Running as root may be required?

@maflcko
Copy link

maflcko commented May 28, 2024

See also bitcoin#29804 (comment)

@Sjors
Copy link
Owner Author

Sjors commented May 28, 2024

Sounds like it, but it seems better if this test is skipped if it's missing a permission. The CI machine is still doing enough other useful stuff.

@maflcko
Copy link

maflcko commented May 28, 2024

Not sure, as that could mean cirrus is accidentally not testing tracepoints. Recall that the tracepoints are only explicitly enabled on cirrus (

bitcoin/.cirrus.yml

Line 187 in 5825cd8

- sed -i "s|\${CIRRUS_CI}|true|g" ./ci/test/00_setup_env_native_asan.sh
) and not otherwise run in a CI.

I'd say to just move the task to GHA and leave it as-is for now.

@Sjors Sjors force-pushed the 2023/01/ci-fork branch from 6398efb to 65abc0b Compare May 28, 2024 13:39
@Sjors Sjors force-pushed the 2024/01/ci-test branch from 96a15eb to 6d7fbd0 Compare May 28, 2024 13:41
@Sjors
Copy link
Owner Author

Sjors commented May 28, 2024

move the task to GHA

That could work too.

@maflcko
Copy link

maflcko commented May 28, 2024

move the task to GHA

That could work too.

@m3dwards is working on this

@m3dwards
Copy link

m3dwards commented May 29, 2024

@m3dwards is working on this

Still a WIP but here is draft PR: bitcoin#30193

@0xB10C
Copy link

0xB10C commented May 30, 2024 •
edited
Loading

Not sure how to get the USDT runner working, but at least that seems doable in theory.

@maflcko the error Failed to compile BPF module is a bit cryptic.
https://cirrus-ci.com/task/5275704273666048?logs=ci#L3276

I have bpfcc-tools linux-headers-$(uname --kernel-release) installed on the machine, but I guess it's missing something else. The check that skips this test doesn't catch it.

It should check for permissions beforehand and otherwise skip the USDT tests, no?

https://github.com/bitcoin/bitcoin/blob/f61ede574c12f1f8ae67f38f15387586719c856f/test/functional/interface_usdt_mempool.py#L135

edit: Oh nvm, you mention that they don't catch it. I misread.

The current failure logged to stderr seems to be:

could not open bpf map: coin_selection_events, error: Operation not permitted

@Sjors Sjors force-pushed the 2023/01/ci-fork branch 4 times, most recently from 8d228bb to 3c998b0 Compare June 18, 2024 09:19
@Sjors Sjors force-pushed the 2024/01/ci-test branch from 6d7fbd0 to 6a7fd20 Compare June 18, 2024 09:43
@Sjors
Copy link
Owner Author

Sjors commented Jun 18, 2024 •
edited
Loading

bitcoin#30193 landed.

Currently trying if qemu-aarch64 is quick enough, see bitcoin#29274 (comment)

Update: giving up on qemu for now.

@Sjors Sjors force-pushed the 2023/01/ci-fork branch from 3c998b0 to e1c25f9 Compare June 18, 2024 10:35
@Sjors Sjors force-pushed the 2024/01/ci-test branch from 935f272 to cda4419 Compare June 18, 2024 10:43
@Sjors Sjors force-pushed the 2023/01/ci-fork branch from e1c25f9 to ff7f6c1 Compare June 18, 2024 11:02
@Sjors Sjors force-pushed the 2024/01/ci-test branch 2 times, most recently from ea8303c to d8229d5 Compare June 20, 2024 08:42
@Sjors Sjors force-pushed the 2023/01/ci-fork branch from ff7f6c1 to 015710f Compare June 20, 2024 12:15
@Sjors Sjors force-pushed the 2024/01/ci-test branch 3 times, most recently from e66f4e9 to 2981904 Compare June 20, 2024 16:42
@Sjors Sjors force-pushed the 2023/01/ci-fork branch from 015710f to 5004bd9 Compare June 20, 2024 16:58
Sjors and others added 4 commits June 25, 2024 17:10
@Sjors
ci: forks can opt-out of CI branch push (Cirrus only)
46be986
@Sjors @ryanofsky
ci: test-each-commit merge base optional
0882bab 
The ci "test-each-commit" job fetches the PR branch being tested with a depth of (# of commits in PR + 2), and then tries to run tests on commits after the most recent merge commit.

When a PR is opened against a bitcoin core branch, a merge commit is always guaranteed to exist within the fetch depth, because bitcoin core branches always point at merge commits.

However, in fork repositories, pull requests can be opened based on other branches that don't contain recent merge commits, and this will currently cause git rev-list to fail with fatal: bad revision '^^@'.

Work around this problem by not requiring a recent merge commit, and just testing on all fetched commits if a merge commit can't be found.

Co-authored-by: Ryan Ofsky <[email protected]>
@Sjors
Add dummy file to demo CI
30e08fb
@Sjors
Second commit
8d52c09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Sjors @maflcko @m3dwards @0xB10C