Skip to content

ci: Add Ubuntu 24.04 ARM support for build-test job #200

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vkprogrammer-001
Copy link

Description

This PR adds Ubuntu 24.04 ARM as a target platform for the build-test job in our CI workflow. This change will help ensure our codebase works correctly on ARM architecture.

Changes

  • Added ubuntu-24.04-arm to the matrix of operating systems in the build-test job
  • Modified runs-on to use the matrix variable for OS selection
  • Maintained existing test configurations (Rust versions and features)

Testing

The CI workflow will now run in parallel on both:

  • Ubuntu latest (x86_64)
  • Ubuntu 24.04 ARM

Each platform will test all combinations of:

  • Rust versions (latest and MSRV 1.63.0)
  • Feature combinations (no-std and all-features)

Benefits

  • Ensures compatibility with ARM-based systems
  • Expands test coverage to include ARM architecture
  • Helps catch potential architecture-specific issues early

Issue: #23

@vkprogrammer-001
Copy link
Author

Hey, @notmandatory can you please review this PR? Thank you!

@notmandatory notmandatory added the chore Non-coding related work label Apr 21, 2025
@notmandatory notmandatory moved this to In Progress in BDK Wallet Apr 21, 2025
@notmandatory
Copy link
Member

Did you see the PR and discussion here: bitcoindevkit/bdk#1905

You may run into the same issue and help review that PR if you have any suggestions.

@coveralls
Copy link

coveralls commented Apr 22, 2025

Pull Request Test Coverage Report for Build 14604822386

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.525%

Totals Coverage Status
Change from base Build 14270666930: 0.0%
Covered Lines: 7269
Relevant Lines: 8401

💛 - Coveralls

@notmandatory
Copy link
Member

Looks like this one also needs to be rebased on the master branch.

@vkprogrammer-001
Copy link
Author

Did you see the PR and discussion here: bitcoindevkit/bdk#1905

You may run into the same issue and help review that PR if you have any suggestions.

I reviewed PR bitcoindevkit/bdk#1905 and found an issue with electrsd. In my PR #1922, I resolved this by excluding electrs-dependent tests on ARM runners instead of building ARM versions of electrs and bitcoind.

I specifically updated the workflow to:

  • Exclude bdk_electrum, bdk_bitcoind_rpc, bdk_testenv, and bdk_esplora tests on ARM
  • Modify example builds to remove electrum-dependent examples on ARM

This approach allowed the ARM CI to pass without altering the electrsd crate itself. It's a temporary solution to maintain progress while we work on a comprehensive fix for ARM-compatible electrsd.

Since we have separated bdk_wallet from bdk, no additional adjustments are needed in the bdk_wallet.

@vkprogrammer-001
Copy link
Author

vkprogrammer-001 commented Apr 22, 2025

Looks like this one also needs to be rebased on the master branch.

Hello @notmandatory,

I've verified that this PR is already rebased on the latest master branch. Both branches point to the same commit (3e61d2a).

Is there something specific in the CI that needs attention? Happy to address any other issues.

@github-project-automation github-project-automation bot moved this from In Progress to Done in BDK Wallet Apr 22, 2025
@ValuedMammal ValuedMammal moved this from Done to Needs Review in BDK Wallet Apr 22, 2025
@ValuedMammal ValuedMammal added this to the 2.0.0 milestone Apr 22, 2025
@notmandatory
Copy link
Member

notmandatory commented Apr 26, 2025

Ah ok I see the issue, I'll need to rename the required checks to use "ubuntu-latest".

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 63e7f4c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Non-coding related work
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

4 participants