-
Notifications
You must be signed in to change notification settings - Fork 225
Fix CI #795
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
Fix CI #795
Conversation
It was moved by bitcoin/bitcoin#31161
It seems that CI doesn't re-run if I modify a commit and force push. |
What's happening it seems is that when a force push occurs, not all previous runners are cancelled. Eventually they finish or fail, and then the jobs start for the new push. Only when that happens do the checks show up in the PR itself. In other words, just be patient... You can see what it's actually working on here: https://github.com/bitcoin-core/HWI/actions/workflows/ci.yml |
18a3a5b
to
10fcca8
Compare
Another problem is that we're building the bitcoind binaries with ubuntu-latest which is 24.04. This produces that binaries that require at least glibc 2.38. We then download those on debian bookworm (the default for https://hub.docker.com/_/python) which comes with 2.36. So the binary fails to run: https://github.com/bitcoin-core/HWI/actions/runs/16440824447/job/46463441345 Possible solutions:
I'll try (2) though (3) is probably a more robust solution. (1) sounds like a massive pain. |
Closing this and continuing in #796 because https://github.com/bitcoin-core/HWI/actions/runs/16440824447 has been stuck for hours. |
Never mind, opening a fresh PR doesn't help. It looks like the available workers are just tied up on that old job. Also, although the timeout of each individual test is set to 45 minutes, only a handful run in parallel, so it really does take forever if all of them are stuck for that time. And some (?) jobs just ignore that timeout setting and stall for hours, e.g. https://github.com/bitcoin-core/HWI/actions/runs/16440824447/job/46463441359. |
I was able to "fix" both the Ledger Speculos and Keepkey builders by downgrading to Ubuntu 22.04. Since I don't want to needlessly downgrade the other builders, it now makes sense to pull #797 in here. |
I'm not 100% sure if the Coldcard multisig patch update in e782954 is necessary, because the previous CI failure may have been due to me messing up whitespace. Still it seems a good idea to refresh it given how much the underlying file changed in recent years. Longer term though we should modify our test rather than sabotage the firmware. |
Ok, the Coldcard patches apply again, but the build fails. Trying the downgrade approach... Update: that worked... 😕 Though the test jobs themselves fail:
e.g. https://github.com/bitcoin-core/HWI/actions/runs/16466809973/job/46548656297?pr=795 So I'll try downgrading those too. Although some jobs did pass. The ColdCard test also kept lingering for 45 minutes until it hit the timeout: https://github.com/bitcoin-core/HWI/actions/runs/16466809973/job/46548656344 But thanks to 4a375d4 and 99ddf39 once that timeout was hit, it shut down it's sibling jobs. Although not all, so we should probably check if the simulator is running. I'll do that in the next push for all simulators. Trezor T jobs are fine, but all Trezor 1 jobs fail with:
https://github.com/bitcoin-core/HWI/actions/runs/16466809973/job/46548656415 So I'll downgrade those too. The ledger runners seem stuck, pushing again with the above change that aborts the test in that scenario. Hopefully that reveals an actual error message, because I already tried downgrading the sim builder. |
Ledger was missing |
abac277
to
69495bf
Compare
Oh Speculos now also needs ledgered, whatever that is... LedgerHQ/speculos#593 |
Seems to be no longer needed: Coldcard/firmware#537 Related CI failure: https://github.com/bitcoin-core/HWI/actions/runs/16464204169/job/46537622260?pr=797
Manually re-applied the patch after the original code seems to have moved around a bit.
``` ERROR: coldcard: test_signtx (test_device.TestSignTx.test_signtx) (addrtypes=['legacy'], multisig_types=['legacy'], external=True, op_return=False) ---------------------------------------------------------------------- Traceback (most recent call last): File "/__w/HWI/HWI/test/test_device.py", line 588, in test_signtx self._test_signtx(addrtypes, multisig_types, external, op_return) File "/__w/HWI/HWI/test/test_device.py", line 576, in _test_signtx self._generate_and_finalize(True, psbt) File "/__w/HWI/HWI/test/test_device.py", line 403, in _generate_and_finalize self.assertTrue(first_sign_res["signed"]) ~~~~~~~~~~~~~~^^^^^^^^^^ KeyError: 'signed' ```
This reverts commit edab2af. The always() option is too powerfull. The next commit implements an alternative solution to the original issue.
This ensures the failure to build a simulator for one device doesn't abort running jobs for the others. They're still grouped by manufacturer. Alternative to bitcoin-core#743.
Build failure on ubuntu-latest: ``` ../py/stackctrl.c: In function ‘mp_stack_ctrl_init’: ../py/stackctrl.c:32:32: error: storing the address of local variable ‘stack_dummy’ in ‘mp_state_ctx.thread.stack_top’ [-Werror=dangling-pointer=] 32 | MP_STATE_THREAD(stack_top) = (char *)&stack_dummy; ../py/stackctrl.c:31:18: note: ‘stack_dummy’ declared here 31 | volatile int stack_dummy; | ^~~~~~~~~~~ In file included from ../py/runtime.h:29, from ../py/stackctrl.c:27: ../py/mpstate.h:282:23: note: ‘mp_state_ctx’ declared here 282 | extern mp_state_ctx_t mp_state_ctx; | ^~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [../py/mkrules.mk:77: build/py/stackctrl.o] Error 1 ``` Test failure (after downgrading build sim): ``` File "/github/home/.cache/pypoetry/virtualenvs/hwi-crEDFiR--py3.9/lib/python3.9/site-packages/sdl2/dll.py", line 362, in <module> dll = DLL("SDL2", ["SDL2", "SDL2-2.0", "SDL2-2.0.0"], os.getenv("PYSDL2_DLL_PATH")) File "/github/home/.cache/pypoetry/virtualenvs/hwi-crEDFiR--py3.9/lib/python3.9/site-packages/sdl2/dll.py", line 253, in __init__ raise RuntimeError("could not find any library for %s (%s)" % RuntimeError: could not find any library for SDL2 (PYSDL2_DLL_PATH: unset) ``` https://github.com/bitcoin-core/HWI/actions/runs/16466809973/job/46548656293?pr=795
The build on ubuntu-latest succeeds, but the resulting binary uses a too recent version of glibc for the test runners to handle. This only seems to impact Trezor 1, but just downgrade for Trezor T as well.
NanoS support has been dropped: LedgerHQ/app-bitcoin-new#262 NanoX also makes it possible to test MuSig2 in the future. Keep NanoS for legacy.
This adds support for BitBox02 Nova. It has the same API has BitBox02.
@Sjors bb02 master is now fixed and its simulator does not depend on libcmocka0 anymore. |
Also fixes indentation.
Co-authored-by: Claude <[email protected]>
Co-authored-by: Claude <[email protected]> Co-authored-by: GPT-5 (Preview) <[email protected]>
7f06228
to
70e14aa
Compare
ACK 70e14aa Seems fine I guess |
Modernize
glibc compatility
Robustness / CI performance
bitcoind
src/bitcoind
was moved tobin/bitcoind
by cmake: Set top-level target output locations bitcoin/bitcoin#31161bitcoind
with Ubuntu 22.04 for glibc compatibilityColdcard
pysdl2-dll
--headless
Jade
Ledger
flask-cors
ledgerd
Trezor
The simulator build jobs as well as the simulator test jobs are split in one group per device brand. This means that the failure of one simulator to build will only cancels jobs for that simulator and not the other ones. Previously this was done in #743 using
always()
, but that's a bit overkill.Issues left for a followup: