Repository navigation
Implementing stable package lock for CI builds - #1148
sabollim-silabs wants to merge 5 commits into
Conversation
|
I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: example@example.com or I have read the CLA Document and I hereby sign the CLA behalf of my company, e-mail: example@example.com Signature is valid for 6 months. This bot will be retriggered when the Contributor License Agreement comment has been provided. Posted by the CLA Assistant Lite bot. |
|
bugbot review |
PR SummaryMedium Risk Overview A new composite action Package builder workflows and Reviewed by Cursor Bugbot for commit 59ff923. Configure here. |
77610cb to
ef237fe
Compare
87a34fb to
2e565aa
Compare
2e565aa to
6c6e097
Compare
aeb1831 to
59ff923
Compare
|
I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: [sabollim@silabs.com] |
|
bugbot review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 59ff923. Configure here.
|
|
||
| ### Important: two lock formats | ||
|
|
||
| | | `pkg.lock` | `conan.lock` | |
There was a problem hiding this comment.
I understand the role of these two files, but can you clarify the need for conanfile.txt?
There was a problem hiding this comment.
this provides the recipe for conan install
There was a problem hiding this comment.
is that not what conan.lock does?
There was a problem hiding this comment.
No recipe → Conan doesn’t know what install you are doing.
Recipe + no lock → Conan resolves versions (can float / change over time).
Recipe + lock → Conan installs that same graph, pinned exactly
| If nightly fails, the previous committed locks are left unchanged, so PRs keep | ||
| using the last green pin set. | ||
|
|
||
| ## Manual update (local) |
There was a problem hiding this comment.
is there a use case for manual updates at all? I would assume that if we want to update package versions, we should do so by running full CI and allowing automation to update only if CI passes with new packages to ensure dependency bumps don't break anything.
There was a problem hiding this comment.
this is drafted for any corner case if needed. even if we do the changes manually, they can be only merged after running the CI. only the update part instead done by nightly can be done by maintaner.
like example Nightly is down due to some infra issue and we know the particular build is stable etc. just for a rare corner case
| if [[ "${{ inputs.use-lockfile }}" == "true" ]]; then | ||
| CHECK_UPDATES="false" | ||
| fi | ||
| slt install "matter_app/${{ inputs.package-version }}@silabs" -e conan \ |
There was a problem hiding this comment.
for my understanding why do we conan install all other packages, and slt install only matter_app package? can we perhaps consolidate on slt install <package> -e conan for everything for consistency?
There was a problem hiding this comment.
sure let me give it a try
| if [[ "$ref" == "release_2.10-1.6.1" && "$event" == "push" ]]; then | ||
| echo "use-lockfile=false" >> "$GITHUB_OUTPUT" | ||
| elif [[ "$ref" == "release_2.10-1.6.1" && "$event" == "workflow_dispatch" && "${{ inputs.use_lockfile }}" == "false" ]]; then | ||
| echo "use-lockfile=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
should we update packages for automation builds only? (ie schedule event). each push seems like overkill with nightly automation.
| - name: Checkout repository | ||
| uses: actions/checkout@v7 | ||
| with: | ||
| ref: ${{ matrix.branch }} | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Check for recent changes | ||
| id: check_changes | ||
| env: | ||
| BRANCH: ${{ matrix.branch }} | ||
| run: | | ||
| commit_count=$(git rev-list --count --since="24 hours ago" HEAD 2>/dev/null || echo "0") | ||
|
|
||
| echo "Commits in last 24 hours: $commit_count" | ||
| echo "has_changes=$([ "$commit_count" -gt 0 ] && echo "true" || echo "false")" >> $GITHUB_OUTPUT | ||
|
|
||
| if [ "$commit_count" -gt 0 ]; then | ||
| echo "Found $commit_count commit(s) in the last 24 hours. Will trigger build." | ||
| echo "Recent commits:" | ||
| git log --since="24 hours ago" --oneline | head -10 | ||
| else | ||
| echo "No commits found in the last 24 hours. Skipping build trigger." | ||
| fi | ||
|
|
There was a problem hiding this comment.
why are we removing this? we want to check for recent commits so we don't have unnecessary runs.
There was a problem hiding this comment.
as in package world , changes are not only form our repo any package change can cause issue. so checking no new commit on our repo is no longer a valid criteria, as our dependencies an change and cuase us issues
|
could use stacked PRs feature in github for this that way flow can be merge #1048, then this will rebase and can merge into |
Co-authored-by: Michael Duggan <miduggan@silabs.com>
Issue Link:
Description of Problem/Feature:
Package CI currently resolves Conan/SLT dependencies from floating ranges. Newly published prereleases can change the resolve without any Matter code change, so PR validation is not reproducible and can fail or pass for reasons unrelated to the PR.
Description of Fix/Solution:
Introduce CI-only dependency locking under .github/pkg-manifest/:
nightly mode (use_lockfile=false).
Testing Done:
PR package validation with use_lockfile=true (locked install + app builds)
Nightly / unlocked path with use_lockfile=false completes and opens/updates the lock refresh PR when appropriate
Confirm .github/pkg-manifest/{pkg.lock,conan.lock,conanfile.txt} stay in sync after conversion
Series 3 bootloader / solution builds via packages/build_app.sh