-
Notifications
You must be signed in to change notification settings - Fork 0
dont rebuild already built apps #127
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
dont rebuild already built apps #127
Conversation
fb01de5 to
331ca61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes the GitHub Actions build workflow by implementing cache-based detection to avoid rebuilding apps that haven't changed. The optimization checks for cached builds using each app's git SHA, only building apps when no cached version exists, significantly reducing build time through smart caching.
Key changes:
- Added cache detection logic to identify apps that have already been built
- Introduced separate jobs for building new apps vs. restoring cached apps
- Added the
password_policyapp to the build matrix
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| IONOS | Updated subproject commit reference |
| .github/workflows/build-artifact.yml | Implemented cache-based app build detection, added cache save/restore logic, split build and cache restoration into separate jobs |
| .github/workflows/build-artifact-original.yml | New file containing the original unoptimized workflow for reference |
|
@printminion-co I've opened a new pull request, #128, to work on those changes. Once the pull request is ready, I'll request review from you. |
62e4dc9 to
81e70b8
Compare
30abc08 to
9460269
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
|
@printminion-co I've opened a new pull request, #130, to work on those changes. Once the pull request is ready, I'll request review from you. |
c833229 to
283955e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
89b742e to
acd2289
Compare
|
@printminion-co I've opened a new pull request, #131, to work on those changes. Once the pull request is ready, I'll request review from you. |
047e72f to
09c736f
Compare
f784d18 to
9017812
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 16 comments.
Comments suppressed due to low confidence (1)
.github/workflows/build-artifact.yml:405
- The
restore-cached-appsjob doesn't include a checkout step before restoring the cache. While the cache action will restore files to the specified path, the subsequent upload-artifact step (line 399-405) references${{ matrix.app.path }}which is a relative path.
Without a workspace context established by a checkout, this relative path resolution might not work as expected. The restored cache files will be uploaded, but the directory structure might not match what's expected in the build-artifact job.
Consider adding a minimal checkout step (even without submodules) to establish the workspace structure, or verify that the artifact upload correctly handles the restored files without a checkout context.
- name: Restore cached build from cache
uses: actions/cache/restore@v4
with:
path: ${{ matrix.app.path }}
key: ${{ env.CACHE_VERSION }}-app-build-${{ matrix.app.name }}-${{ matrix.app.sha }}
fail-on-cache-miss: true
- name: Validate cached build
run: |
APP_PATH="${{ matrix.app.path }}"
# Check that the directory exists and is not empty
if [ ! -d "$APP_PATH" ] || [ -z "$(ls -A $APP_PATH)" ]; then
echo "❌ Cache validation failed: Directory is empty or missing"
exit 1
fi
# Check for appinfo/info.xml (required for all Nextcloud apps)
if [ ! -f "$APP_PATH/appinfo/info.xml" ]; then
echo "❌ Cache validation failed: Missing appinfo/info.xml"
exit 1
fi
echo "✅ Cache validation passed for ${{ matrix.app.name }}"
- name: Upload cached ${{ matrix.app.name }} build artifacts
uses: actions/upload-artifact@v5
with:
retention-days: 1
name: external-app-build-${{ matrix.app.name }}
path: |
${{ matrix.app.path }}
bfd419e to
6afffe5
Compare
413bc91 to
4ceb9f7
Compare
6963427 to
6afffe5
Compare
seriAlizations
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and compared new pipeline with Misha and LGTM, only problem I see is changes in IONOS config submodule not being registered as changes.
for later performance comparison Signed-off-by: Misha M.-Kupriyanov <[email protected]>
in order not to overwrite optimized artifact Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…ompatibility Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Signed-off-by: Misha M.-Kupriyanov <[email protected]>
lets use underscore Signed-off-by: Misha M.-Kupriyanov <[email protected]>
drop validation since it is now Redundant (by design) Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Optimized build workflow that uses cache-based detection - Checks cache for each app's current SHA - Only builds apps with no cached build - Significantly reduces build time through smart caching Signed-off-by: Misha M.-Kupriyanov <[email protected]>
Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…fact.yml Signed-off-by: Misha M.-Kupriyanov <[email protected]>
6afffe5 to
80e40e2
Compare
…ld-artifact.yml Signed-off-by: Misha M.-Kupriyanov <[email protected]>
🎯 Overview
This PR introduces a major optimization to the Nextcloud Workspace build workflow by implementing intelligent cache-based detection that only rebuilds apps when their code has changed. This significantly reduces CI/CD build times and resource consumption.
📊 Impact
Before
After
🏗️ Architecture
Cache Key Strategy
When to increment
CACHE_VERSION:Workflow Flow
graph TD A[prepare-matrix] --> B{Generate Matrix} B --> C[Detect Cache Status] C --> D{Check Each App} D --> E[apps_to_build] D --> F[apps_to_restore] E --> G[build-external-apps] G --> H[Build Changed Apps] H --> I[Save to Cache] I --> J[Upload Artifacts] F --> K[restore-cached-apps] K --> L[Restore from Cache] L --> M[Validate Cache] M --> N[Upload Artifacts] J --> O[build-artifact] N --> O O --> P[Assemble Final Package]Job Dependencies
📁 Files Changed
.github/workflows/build-artifact.yml.github/workflows/build-artifact-original.yml✅ Testing Instructions
Prerequisites
Test Scenario 1: Full Cold Build (No Cache)
This simulates the first build or after cache invalidation.
Steps:
CACHE_VERSIONin.github/workflows/build-artifact.yml(e.g., fromv1.0tov1.1)ionos-devbranchExpected Results:
build-external-appsjob runs with full matrixrestore-cached-appsjob is skipped (no cached apps)Verification:
Test Scenario 2: Incremental Build (Change 1 App)
This tests the optimization when only a single app is modified.
Steps:
apps-external/mail)Alternative: Test without code changes (delete cache manually)
Expected Results:
build-external-appsjob runs with 1 app onlyrestore-cached-appsjob runs with 24+ appsVerification:
Test Scenario 3: Multi-App Change
This tests multiple apps being changed simultaneously.
Steps:
Alternative: Test without code changes (delete caches manually)
Expected Results:
build-external-appsruns with 3 apps in parallelrestore-cached-appsruns with 22+ apps in parallelTest Scenario 4: No Changes (Re-run)
This tests pure cache restoration when no code has changed.
Steps:
Expected Results:
build-external-appsjob is skipped entirelyrestore-cached-appsjob runs with all appsVerification:
Test Scenario 5: Cache Validation
This tests that cached builds are validated before use.
Steps:
Expected Results:
Test Scenario 6: New App Addition
This tests adding a brand new app to the matrix.
Steps:
../nc-docs-and-tools/tools/add_app_submodule.sh # Follow prompts to add new appIONOS/MakefileExpected Results:
Test Scenario 7: Cache Invalidation
This tests the cache version bump mechanism.
Steps:
CACHE_VERSIONfromv1.0tov2.0in workflow filegit add .github/workflows/build-artifact.yml git commit -m "chore: bump cache version to v2.0" git pushExpected Results:
Test Scenario 8: Parallel Build Performance
This tests the parallel execution of builds and restores.
Steps:
Expected Results:
build-external-appsjobs run concurrentlyrestore-cached-appsjobs run concurrentlyTest Scenario 9: Artifact Assembly
This tests the final artifact creation with mixed built/restored apps.
Steps:
build-artifactjob to completeExpected Results:
Verification:
Test Scenario 10: Error Recovery with Retries
This tests the retry logic for artifact uploads.
Steps:
Expected Results:
Test Scenario 11: Artifact Comparison (Old vs New Pipeline)
This critical test validates that the optimized pipeline produces identical artifacts to the original pipeline.
Steps:
Trigger a build using the original workflow (full rebuild):
Trigger a build using the optimized workflow (with full cache invalidation):
Compare the artifacts:
Compare file checksums for critical apps:
Functional validation:
Expected Results:
Acceptable Differences:
The following differences are acceptable and expected:
Critical Files to Verify:
Troubleshooting:
If differences are found:
Success Criteria:
🔍 Validation Checklist
For reviewers, please verify:
Cache Detection Logic
Build Execution
Performance
Error Handling
Observability
Artifact Integrity
📈 Metrics to Monitor
After deployment, monitor these metrics:
Build Time Reduction
Cache Hit Rate
Resource Usage
Reliability
🚀 Rollout Plan
Phase 1: Testing (Current)
Phase 2: Canary (After approval)
ionos-devbranchPhase 3: Production
ionos-stablebranchRollback Plan
If issues are discovered:
build-artifact-original.ymlbuild-artifact.ymlChecklist