-
-
Notifications
You must be signed in to change notification settings - Fork 32
ci(release): Merge artifacts for Craft release discovery #1249
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -406,3 +406,22 @@ jobs: | |
| packages/spotlight/dist-electron/*.zip | ||
| packages/spotlight/dist-electron/*.blockmap | ||
| packages/spotlight/dist-electron/*.yml | ||
|
|
||
| merge-artifacts: | ||
| name: Merge Artifacts | ||
| needs: [build, electron-mac] | ||
| if: always() && needs.build.result == 'success' && needs.electron-mac.result == 'success' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Download release artifacts | ||
| uses: actions/download-artifact@v5 | ||
| with: | ||
| pattern: '{built-packages,spotlight-binaries,electron-binaries}' | ||
| path: artifacts/ | ||
| merge-multiple: true | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Artifact paths not flattened despite PR description claimMedium Severity The PR description claims that Additional Locations (1) |
||
|
|
||
| - name: Upload merged artifact | ||
| uses: actions/upload-artifact@v5 | ||
| with: | ||
| name: ${{ github.sha }} | ||
| path: artifacts/ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling for empty artifact uploadLow Severity The merged artifact upload step is missing |
||
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.
Bug: The
patternparameter foractions/download-artifactuses brace expansion ({a,b,c}), which is likely unsupported. Standard glob syntax does not include brace expansion, which is a shell feature.Severity: CRITICAL
🔍 Detailed Analysis
The
actions/download-artifact@v5action is configured with apatternusing brace expansion:'{built-packages,spotlight-binaries,electron-binaries}'. Brace expansion is a shell feature and is not typically supported by standard glob libraries, including the one likely used by GitHub Actions (@actions/glob). The action will probably search for a single artifact with the literal name'{built-packages,spotlight-binaries,electron-binaries}', which does not exist. This will cause the artifact download to fail, breaking the subsequentmerge-artifactsjob and preventing the release from being created.💡 Suggested Fix
Replace the brace expansion pattern with a method that is explicitly supported. Either call the
download-artifactaction multiple times, once for each artifact name, or use a wildcard pattern like*or*-binariesif appropriate, combined withmerge-multiple: trueto download all required artifacts into a single directory.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID:
8505748