fix(ci): cache Windows Craft dependencies - #10698
Conversation
Cache the dependency-only Craft environment using the current CraftMaster and blueprint revisions. Remove the unused OpenCppCoverage setup and use a shallow checkout. Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1643570224
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $craftMasterRevision = remoteRevision "https://invent.kde.org/packaging/craftmaster.git" "HEAD" | ||
| $kdeBlueprintRevision = remoteRevision "https://github.com/nextcloud/craft-blueprints-kde.git" "refs/heads/stable-34.0" | ||
| $nextcloudBlueprintRevision = remoteRevision "https://github.com/nextcloud/craft-blueprints-nextcloud.git" "refs/heads/stable-34.0" |
There was a problem hiding this comment.
can the refs be read from craftmaster.ini somehow?
There was a problem hiding this comment.
@nilsding
good one. question for the craft revision. craftmaster.ini is using a different one.
on purpose? or do we need to use the same?
https://github.com/nextcloud/desktop/blob/master/craftmaster.ini#L5-L6
There was a problem hiding this comment.
to match what we use for our releases this should be the same
There was a problem hiding this comment.
@nilsding can you check the last commit one more time? not really sure if we are doing the correct thing or if the complexity is not worth it
|
Artifact containing the AppImage: nextcloud-appimage-pr-10698.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
Read the pinned Craft revision and both blueprint refs from craftmaster.ini. Reuse the blueprint refs for repository setup and resolve their current commits for the cache key. Signed-off-by: Rello <github@scherello.de> Assisted-by: Codex:GPT-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 182428e471
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ${{ env.CRAFT_MASTER_CONFIG }} | ||
| ${{ github.workspace }}\craft-clone | ||
| ${{ github.workspace }}\${{ env.CRAFT_TARGET }} | ||
| key: ${{ runner.os }}-craft-v1-${{ env.CRAFT_TARGET }}-python-${{ env.PYTHON_VERSION }}-${{ steps.craft-revisions.outputs.key }} |
There was a problem hiding this comment.
In this Windows pull-request workflow, the key covers only CraftRevision and the two blueprint branch tips, while the cache also restores the complete craftmaster.ini and a CraftMaster checkout cloned from its independently moving HEAD. If a later push to the same PR changes an unkeyed setting such as libs/qt6.version, libs/openssl.version, or another build option—or CraftMaster alone advances—the existing immutable cache is hit, the old configuration is restored over the checkout, and dependency installation is skipped, so CI builds against stale inputs. Include the full configuration hash and the resolved CraftMaster revision in the key, and avoid restoring the checked-in configuration itself.
Useful? React with 👍 / 👎.
|
|
/backport to stable-34.0 |



Cache the dependency-only Craft environment using the current CraftMaster and blueprint revisions. Remove the unused OpenCppCoverage setup and use a shallow checkout.
Assisted-by: Codex:GPT-5