Skip to content

Bump ubuntu image in ci to 22.04#725

Merged
thunderbiscuit merged 2 commits intobitcoindevkit:masterfrom
thunderbiscuit:ci/bump-ubuntu-image
Apr 9, 2025
Merged

Bump ubuntu image in ci to 22.04#725
thunderbiscuit merged 2 commits intobitcoindevkit:masterfrom
thunderbiscuit:ci/bump-ubuntu-image

Conversation

@thunderbiscuit
Copy link
Copy Markdown
Member

See #724.

Fixes #724.

@rustaceanrob
Copy link
Copy Markdown
Collaborator

Out of curiosity why not use ubuntu-latest?

@thunderbiscuit
Copy link
Copy Markdown
Member Author

thunderbiscuit commented Apr 8, 2025

The images in Ubuntu ship with the Android Native Development Kit (NDK), and historically changes in this NDK has not always been smooth with respect to uniffi. Using ubuntu-latest keeps that NDK version floating upwards and tests would just start failing because the CI would use a new Ubuntu, and hence a new NDK. I prefer the hard pin and once in a while a clean break (when they deprecate images), at which point I bump everything at once. In this case I'm actually considering bumping to 24.04 directly.

In this case the NDK goes up again, and that has me nervous I'm not gonna lie. I'm running the Android build here. We'll see if it succeeds.

@rustaceanrob
Copy link
Copy Markdown
Collaborator

rustaceanrob commented Apr 8, 2025

Gotcha. Would it save future headache to have rust-fmt, audit, and the Python jobs run on latest so there isn't any need to update those jobs in the future?

@thunderbiscuit
Copy link
Copy Markdown
Member Author

Actually small correction: I think we also had issues with the ring crate not being able to compile on a newer version of the NDK, and that had us stuck on NDK 21 for like a year which we had to download + install on each build instead of just pulling the default one on Ubuntu. But yeah basically I don't like changes in the NDK haha.

@thunderbiscuit
Copy link
Copy Markdown
Member Author

using latest in Python and Rust runs

Yeah in theory it would. I sort of like having it all build/run/test on the same image so it's cleaner to think through issues if there are any that are related to the CI images, but also they haven't caused any other issues than the one mentioned above really. So yeah I'm not feeling too strongly one way or the other.

In this case my local ndk is 25.X so I'm looking at bumping that to the 27 version that's the LTS version mentioned here. I won't merge until I've figured this thing out...

@thunderbiscuit
Copy link
Copy Markdown
Member Author

thunderbiscuit commented Apr 8, 2025

I'm saying all of this but in fact they still change what's in the images even the images don't change! So you can pin the version of Ubuntu all you want but I remember once the Python wheels failed for some obscure reason and it was that the build tools included in the 20.04 image had actually been updated... forget about reproducibility it's just a circus out there with the GitHub CI images lol. Mind you their purpose is probably not build/publish systems but still.

@thunderbiscuit
Copy link
Copy Markdown
Member Author

thunderbiscuit commented Apr 9, 2025

@rob I'm actually adding a fix to the live tests here (which have been failing for a few weeks) just to try to get it all in one PR and not have you review a bunch of PRs.

I merged it on this branch just so I can actually run all live tests on it and make sure we fixed the issues. In Python it was just the imports that were wrong, and in Kotlin it's that we now use new descriptors (for quicker sync) and the CHANGE_FORBIDDEN option creates problems because we currently send "out" the sats instead of cycling them back to the external keychain, and we only had one output on the external keychain (my initial send to the wallet), and so now all owned outputs by the wallet are on the change keychain. For now I just removed this flag on the builder.

I have also tested locally the Android build with the default NDK version that ships with the ubuntu-24.04 (27.2.12479018) and everything builds well. I'm going to leave it pinned for now on all runs, but using 24.04, effectively the ubuntu-latest.

@rustaceanrob
Copy link
Copy Markdown
Collaborator

Strange the python linter didn't catch that unknown import. Oh well.

Since the philosophy is to keep all versions the same in CI to rule out failures due to OS mismatch, I think this all makes sense.

ACK 749c62f

@thunderbiscuit
Copy link
Copy Markdown
Member Author

For some reason the Swift live tests fail (https://github.com/bitcoindevkit/bdk-ffi/actions/runs/14357615687/job/40254293039#step:4:624) but also the logs show that they all pass?

I ran all live tests locally and if everything is green I'll merge.

@thunderbiscuit thunderbiscuit merged commit 749c62f into bitcoindevkit:master Apr 9, 2025
50 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bump Ubuntu images used in CI

2 participants