Skip to content
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

Target Android 15: Add ELF alignment verification CI check #5634

Open
wants to merge 3 commits into
base: feature/mike/android-15
Choose a base branch
from

Conversation

mikescamell
Copy link
Contributor

@mikescamell mikescamell commented Feb 11, 2025

Task/Issue URL: https://app.asana.com/0/488551667048375/1209378846484863

Description

Added ELF alignment verification to the CI pipeline to ensure native libraries are properly aligned at 16KB or 64KB boundaries. This includes a new shell script that checks alignment in APK files and reports any misaligned libraries.

A passing build using playDebug: https://github.com/duckduckgo/Android/actions/runs/13291887849/job/37114510525?pr=5634

A failing build using internalDebug (because of Flipper which we know about)
https://github.com/duckduckgo/Android/actions/runs/13292470506/job/37116369658?pr=5634

Steps to test this PR

ELF Alignment Verification

  • Build a debug APK using ./gradlew androidTestsBuild
  • Run ./scripts/check_elf_alignment.sh path/to/your/app.apk
  • Verify that all native libraries are reported as ALIGNED
  • Check that the CI pipeline successfully executes the alignment verification step

UI changes

No UI changes in this PR

@mikescamell mikescamell force-pushed the feature/mike/android-15-16kb branch 2 times, most recently from d80c3b0 to 3eae73b Compare February 13, 2025 08:58
@mikescamell mikescamell changed the title 16KB CI Check Target Android 15: Add ELF alignment verification CI check Feb 13, 2025
@mikescamell mikescamell marked this pull request as ready for review February 13, 2025 09:07
@malmstein malmstein self-assigned this Feb 13, 2025
Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

works as expected. I’d like to get a summary of the task to make it clear why the workflow failed. At the moment it seems that only the CI tests failed.

run: chmod +x scripts/check_elf_alignment.sh

- name: Check native libraries alignment
run: ./scripts/check_elf_alignment.sh ${{ steps.find-apk.outputs.apk_path }}
Copy link
Contributor

Choose a reason for hiding this comment

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

how about capturing the output and post a summary in the workflow? if you open a failed action now it shows as if Android CI tests fails, and opening the details shows that there are no tests.

Screenshot 2025-02-13 at 10 54 06

provided by Google with a couple of tweaks with exit status so we can fail the workflow
we're adding the check in the android_tests as we already need an apk to run the tests, so we're not having to build multiple apks and slowing everything down

we considered after PR merge but that meant the person would only find out after that something they are using is not aligned
@mikescamell mikescamell force-pushed the feature/mike/android-15 branch from 563db61 to d836cb4 Compare February 14, 2025 18:19
@mikescamell mikescamell force-pushed the feature/mike/android-15-16kb branch from 3eae73b to 629b8ff Compare February 14, 2025 18:19
@mikescamell mikescamell mentioned this pull request Feb 14, 2025
2 tasks
Copy link
Contributor Author

mikescamell commented Feb 14, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

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.

2 participants