Skip to content

Conversation

@iartemov-ledger
Copy link
Contributor

@iartemov-ledger iartemov-ledger commented Aug 5, 2025

Description

Replacement for #1036
Corresponds to https://ledgerhq.atlassian.net/browse/B2CA-1995.

DONE

  • libclang_rt.builtins replaces libgcc for Nano X, Stax and Flex targets, built using build_clangrt_builtins.sh script
  • libc and libm have been updated from the ones built using build_newlib.sh script (newlib 3.3.0 from Debian 12 currently used for app-builder)
  • the both are built using .github/workflows/build_clangrt_builtins.yml workflow (https://github.com/LedgerHQ/ledger-secure-sdk/actions/runs/17647456726 job and pushed through [SDK_LIBS_UPDATE] Updating static SDK libraries #1186 - it matches to the libs built from feat/regain_control_over_libs_2)
  • Rust part is not be impacted - tested locally with app-boilerplate-rust and app-starknet apps. This is expected as Rust build does not seem to use libgcc.a and libm.a - only libc.a. So some improvements to be done later in the Rust SDK as follow Regain control over the SDK libraries for Rust ledger-device-rust-sdk#269
  • remove libgcc from the SDK (still "used" by NanoS)? - let's do it in an independent PR cleaning up all occurrences!

Tests

Few other remarks

  • the libs in question are not used for he Ledge rOS build
  • once we switch to Debian 13 in app-builder we'll need to rebuild the libs and benefit among other from the newest newlib version 4.5.0

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Tests
  • Documentation
  • Other (for changes that might not fit in any category)

Breaking changes

Please complete this section if any breaking changes have been made, otherwise delete it.

Additional comments

[x] TARGET_API_LEVEL: API_LEVEL_24
[x] TARGET_API_LEVEL: API_LEVEL_25

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.58%. Comparing base (fddf1d7) to head (3954b8f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1076   +/-   ##
=======================================
  Coverage   62.58%   62.58%           
=======================================
  Files          14       14           
  Lines        1860     1860           
=======================================
  Hits         1164     1164           
  Misses        696      696           
Flag Coverage Δ
unittests 62.58% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@iartemov-ledger iartemov-ledger changed the title Regain control over the SDK libraries Regain control over the SDK libraries [new variant using rebuilt newlib 4.5.0) Aug 5, 2025
Copy link
Contributor

@0pendev 0pendev left a comment

Choose a reason for hiding this comment

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

I'll leave @bboilot-ledger to look at the workflow 🙏

Copy link
Contributor

@bboilot-ledger bboilot-ledger left a comment

Choose a reason for hiding this comment

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

Workflows look good to me! We could however enforce the GITHUB_TOKEN permissions. The fixes proposed by copilot should work well.

Note that it's recommended to use intermediate env variables in shell scripts as variable exponentiation (${{*}}) happens before the script execution which could result in code execution (https://docs.github.com/en/actions/reference/security/secure-use#use-an-intermediate-environment-variable). Also those variable should be escaped using double quotes to bind them to only one argument in case the variable contains spaces.
-> Since this workflow can only be triggered by a person with wirte access to the repository there's not much risk.

@iartemov-ledger iartemov-ledger force-pushed the feat/regain_control_over_libs_2_master branch 3 times, most recently from 0c4e247 to 6d5d3ba Compare August 12, 2025 14:00
@iartemov-ledger iartemov-ledger force-pushed the feat/regain_control_over_libs_2_master branch from 6d5d3ba to 0b5182e Compare August 29, 2025 08:31
@LedgerHQ LedgerHQ deleted a comment from bboilot-ledger Aug 29, 2025
@iartemov-ledger iartemov-ledger force-pushed the feat/regain_control_over_libs_2_master branch 3 times, most recently from e4ce829 to 2bbb8d6 Compare September 11, 2025 14:56
@iartemov-ledger iartemov-ledger changed the title Regain control over the SDK libraries [new variant using rebuilt newlib 4.5.0) Regain control over the SDK libraries [new variant using rebuilt newlib from Debian sources) Sep 11, 2025
@iartemov-ledger iartemov-ledger force-pushed the feat/regain_control_over_libs_2_master branch from 2bbb8d6 to b6abd3f Compare September 11, 2025 15:02
@iartemov-ledger iartemov-ledger marked this pull request as ready for review September 12, 2025 07:28
@iartemov-ledger iartemov-ledger force-pushed the feat/regain_control_over_libs_2_master branch from b37acaa to b29db46 Compare September 19, 2025 15:07
@iartemov-ledger iartemov-ledger force-pushed the feat/regain_control_over_libs_2_master branch from b29db46 to 3954b8f Compare September 19, 2025 15:10
@iartemov-ledger iartemov-ledger merged commit 6fa8509 into master Sep 19, 2025
226 checks passed
@iartemov-ledger iartemov-ledger deleted the feat/regain_control_over_libs_2_master branch September 19, 2025 15:34
@apaillier-ledger apaillier-ledger mentioned this pull request Oct 3, 2025
6 tasks
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.

6 participants