-
Notifications
You must be signed in to change notification settings - Fork 18
Regain control over the SDK libraries #1036
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
base: master
Are you sure you want to change the base?
Conversation
[SDK_LIBS_UPDATE] Updating static SDK libraries
runs-on: ubuntu-latest | ||
container: | ||
image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-lite:latest | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
target: | ||
- core: cortex-m3 | ||
se: st33 | ||
- core: cortex-m35p+nodsp | ||
se: st33k1 | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
# TODO: remove this next line before merging! | ||
ref: feat/regain_control_over_libs | ||
sparse-checkout: | | ||
tools/build_clangrt_builtins.sh | ||
sparse-checkout-cone-mode: false | ||
|
||
- run: ./tools/build_clangrt_builtins.sh -t ${{ matrix.target.core }} -o artifact/arch/${{ matrix.target.se }}/lib | ||
|
||
- uses: actions/upload-artifact@v4 | ||
with: | ||
name: arch-${{ matrix.target.se }} | ||
path: artifact/ | ||
|
||
merge: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
To fix the issue, we need to add a permissions
block to the workflow. This block should specify the least privileges required for each job. For example:
- The
build
job only needscontents: read
to check out the repository. - The
merge
job may needcontents: read
andactions: write
for artifact handling. - The
pr_create
job requirescontents: read
andpull-requests: write
to create pull requests.
The permissions
block can be added at the root level of the workflow to apply to all jobs or individually for each job. In this case, we will add permissions for each job to ensure granular control.
-
Copy modified lines R26-R27 -
Copy modified lines R58-R60 -
Copy modified lines R71-R73
@@ -25,2 +25,4 @@ | ||
runs-on: ubuntu-latest | ||
permissions: | ||
contents: read | ||
container: | ||
@@ -55,2 +57,5 @@ | ||
runs-on: ubuntu-latest | ||
permissions: | ||
contents: read | ||
actions: write | ||
steps: | ||
@@ -65,2 +70,5 @@ | ||
runs-on: ubuntu-latest | ||
permissions: | ||
contents: read | ||
pull-requests: write | ||
if: ${{ success() && inputs.create_pr }} |
needs: build | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/upload-artifact/merge@v4 | ||
with: | ||
name: arch | ||
pattern: arch-* | ||
delete-merged: true | ||
|
||
pr_create: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
To fix the issue, we will add a permissions
block at the workflow level to define the minimal permissions required for the workflow. This block will apply to all jobs unless overridden at the job level. Based on the workflow's operations, the following permissions are required:
contents: read
for accessing the repository's contents.pull-requests: write
for creating pull requests in thepr_create
job.
We will add the permissions
block at the top of the workflow file, just below the name
field.
-
Copy modified lines R2-R4
@@ -1,2 +1,5 @@ | ||
name: Build clang runtime builtins | ||
permissions: | ||
contents: read | ||
pull-requests: write | ||
|
needs: merge | ||
runs-on: ubuntu-latest | ||
if: ${{ success() && inputs.create_pr }} | ||
continue-on-error: true | ||
steps: | ||
- name: Clone repository | ||
uses: actions/checkout@v4 | ||
with: | ||
# by default the action uses fetch-depth = 1, which creates | ||
# shallow repositories from which we can't push | ||
fetch-depth: 0 | ||
ref: ${{ inputs.target_sdk_branch }} | ||
|
||
- name: Download Binaries artifact | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: arch | ||
|
||
- name: PR creation | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
run: | | ||
git config --global user.email ${{ env.GIT_USER_EMAIL }} | ||
git config --global user.name ${{ env.GIT_USER_NAME }} | ||
git switch --create ${{ env.UPDATE_BRANCH }} | ||
git add -A . | ||
git commit -m 'Updating static SDK libraries' | ||
git push -u origin ${{ env.UPDATE_BRANCH }} | ||
gh pr create -B ${{ inputs.target_sdk_branch }} --title '[SDK_LIBS_UPDATE] Updating static SDK libraries' --body 'Created by Github workflow "${{ github.workflow }}", job "${{ github.job }}", run "${{ github.run_id }}".' |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
To fix the issue, we will add a permissions
block to the pr_create
job to explicitly define the minimal permissions required for its tasks. Specifically, the pr_create
job needs contents: write
to push changes to a branch and pull-requests: write
to create a pull request. This change ensures that the GITHUB_TOKEN
has only the necessary permissions, reducing the risk of unintended actions.
The permissions
block will be added under the pr_create
job definition in the .github/workflows/build_clangrt_builtins.yml
file.
-
Copy modified lines R67-R69
@@ -66,2 +66,5 @@ | ||
if: ${{ success() && inputs.create_pr }} | ||
permissions: | ||
contents: write | ||
pull-requests: write | ||
continue-on-error: true |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1036 +/- ##
=======================================
Coverage 62.08% 62.08%
=======================================
Files 13 13
Lines 1817 1817
=======================================
Hits 1128 1128
Misses 689 689
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cb002e9
to
bea58f1
Compare
bea58f1
to
0dd9032
Compare
Based on @apaillier-ledger's work here: #924.
Corresponds to https://ledgerhq.atlassian.net/browse/B2CA-1995.
See https://github.com/LedgerHQ/ledger-secure-sdk/blob/feat/regain_control_over_libs/arch/libraries.md as documentation.
Also internal documentation:
libclang_rt.builtins
replaceslibgcc
for Nano X, Stax and Flex targets.github/workflows/build_clangrt_builtins.yml
workflow and pushed through [SDK_LIBS_UPDATE] Updating static SDK libraries #1035libc
andlibm
removed from the SDK and from now on the ones frompicolibc
Debian package are usedledger-app-tester
SYSROOT
change, most probably because of incorrect header inclusion)app-plugin-rarible
app-tron
(fix example: Correct inclusion of used system header file app-tron#88)app-mimblewimblecoin
app-plugin-okx
app-acre
app-zcash-new
Description
Please provide a detailed description of what was done in this PR.
(And mention any links to an issue docs)
Changes include
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it.
Additional comments
Please post additional comments in this section if you have them, otherwise delete it.