-
-
Notifications
You must be signed in to change notification settings - Fork 375
ci(release): Switch from action-prepare-release to Craft #7150
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
Conversation
This PR migrates from the deprecated action-prepare-release to the new Craft GitHub Actions (reusable workflow or composite action). Changes: - Migrate .github/workflows/release.yml to Craft reusable workflow
9b5a994 to
aaca1f4
Compare
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Build / dependencies / internal 🔧Deps
Other
Other
🤖 This preview updates automatically when you update the PR. |
.github/workflows/release.yml
Outdated
| release: | ||
| uses: getsentry/craft/.github/workflows/release.yml@v2 | ||
| with: | ||
| name: ${{matrix.variant.name}} | ||
| suffix: ${{matrix.variant.suffix}} | ||
| macho-type: ${{matrix.variant.macho-type}} | ||
| configuration-suffix: ${{matrix.variant.configuration-suffix}} | ||
| variant-id: ${{matrix.variant.id}} | ||
| release-version: ${{ github.event.inputs.version }} | ||
| sdk-list: ${{ needs.setup-matrix.outputs.sdk-list-array }} | ||
| strategy: | ||
| matrix: | ||
| variant: ${{ fromJson(needs.setup-matrix.outputs.slices) }} | ||
|
|
||
| assemble-xcframework-variant: | ||
| needs: [files-changed, build-xcframework-variant-slices, setup-matrix] | ||
| # Run the job only for PRs with related changes or non-PR events. | ||
| if: github.event_name != 'pull_request' || needs.files-changed.outputs.run_release_for_prs == 'true' | ||
| name: Assemble XCFramework Variant | ||
| uses: ./.github/workflows/assemble-xcframework-variant.yml | ||
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} | ||
| merge_target: ${{ inputs.merge_target }} | ||
| path: '|' | ||
| secrets: inherit |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
.github/workflows/release.yml
Outdated
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} | ||
| merge_target: ${{ inputs.merge_target }} | ||
| path: '|' |
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.
Suspicious pipe character value for path parameter
High Severity
The path parameter is set to '|' which is a single pipe character. This is not a valid file path and appears to be a typo or misconfiguration. In YAML, | is used for literal block scalars for multiline content, but when quoted as '|', it's just a string containing a pipe. This could cause the Craft release workflow to fail or behave unexpectedly since the value doesn't represent a valid path.
| description: Force a release even when there are release-blockers | ||
| required: false | ||
| merge_target: | ||
| description: Target branch to merge into. Uses the default branch as a fallback (optional) |
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.
Missing XCFramework builds breaks preReleaseCommand execution
High Severity
The removal of the XCFramework build jobs breaks the release process. The .craft.yml defines a preReleaseCommand that runs bump.sh, which calls update-package-sha.sh. That script expects XCFramework zip files to exist in XCFrameworkBuildPath/ to calculate checksums for Package.swift. The old workflow built and assembled these frameworks before releasing, but the new workflow just calls the Craft reusable workflow without building anything, causing the preReleaseCommand to fail with missing file errors.
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.
This seems like valid feedback, we use that command with the prebuilt binary to get the file hashes
| description: Force a release even when there are release-blockers | ||
| required: false | ||
| merge_target: | ||
| description: Target branch to merge into. Uses the default branch as a fallback (optional) |
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.
Dependent workflow expects removed xcframeworks.zip artifact
High Severity
The release-upload-xcframework.yml workflow (triggered on pushes to release/** branches) expects to download an artifact named xcframeworks.zip from the release workflow run ID stored in .github/last-release-runid. The old release.yml created and uploaded this artifact, but the new workflow doesn't produce it. When release-upload-xcframework.yml runs, the artifact download will fail because xcframeworks.zip no longer exists.
itaybre
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.
This seems to be a big change to the workflows.
I understand action-prepare-release is deprectated and shouldn't be used anymore.
But in this case, shouldn't only that step be changed and leave the other steps like before.
Also, have you confirmed this works for releasing sentry-cocoa with a beta release?
| push: | ||
| branches: | ||
| - main | ||
| - v8.x | ||
|
|
||
| pull_request: | ||
| types: [opened, synchronize, reopened, labeled] | ||
|
|
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.
This cannot be removed, we use this workflow to verify the framework builds on PRs and main
| description: Force a release even when there are release-blockers | ||
| required: false | ||
| merge_target: | ||
| description: Target branch to merge into. Uses the default branch as a fallback (optional) |
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.
This seems like valid feedback, we use that command with the prebuilt binary to get the file hashes
The previous migration incorrectly removed the GitHub App token authentication step. This commit restores it by switching to the composite action pattern which preserves the auth flow.
The previous migration incorrectly removed the GitHub App token authentication step. This commit restores it by switching to the composite action pattern which preserves the auth flow.
| steps: | ||
| - name: Get auth token | ||
| id: token | ||
| uses: actions/create-github-app-token@v1 | ||
| with: | ||
| app-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }} | ||
| private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }} | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| token: ${{ steps.token.outputs.token }} | ||
| fetch-depth: 0 |
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 release workflow will fail because it attempts to calculate checksums on XCFramework artifacts that are no longer being built or downloaded.
Severity: CRITICAL
🔍 Detailed Analysis
The updated release.yml workflow migrates to getsentry/craft@v2 but removes the preceding steps that built and downloaded XCFramework artifacts. The craft action executes a preReleaseCommand, bash ./scripts/bump.sh, which in turn calls ./scripts/update-package-sha.sh. This script attempts to calculate checksums for several .xcframework.zip files within the XCFrameworkBuildPath/ directory. Since these files are no longer built or downloaded, the shasum command will fail with a "No such file or directory" error. The script's set -euo pipefail option ensures this failure will halt the entire release workflow, preventing any new releases from completing.
💡 Suggested Fix
Reintroduce the necessary build jobs and artifact download steps into the release.yml workflow before the getsentry/craft@v2 action is called. The workflow must ensure that the required XCFramework ZIP files are present in the XCFrameworkBuildPath/ directory before the update-package-sha.sh script is executed.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/release.yml#L18-L28
Potential issue: The updated `release.yml` workflow migrates to `getsentry/craft@v2` but
removes the preceding steps that built and downloaded XCFramework artifacts. The `craft`
action executes a `preReleaseCommand`, `bash ./scripts/bump.sh`, which in turn calls
`./scripts/update-package-sha.sh`. This script attempts to calculate checksums for
several `.xcframework.zip` files within the `XCFrameworkBuildPath/` directory. Since
these files are no longer built or downloaded, the `shasum` command will fail with a "No
such file or directory" error. The script's `set -euo pipefail` option ensures this
failure will halt the entire release workflow, preventing any new releases from
completing.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8423777
| required: false | ||
| merge_target: | ||
| description: Target branch to merge into. Uses the default branch as a fallback (optional) | ||
| description: Target branch to merge into |
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.
Removed PR and push triggers break CI validation
High Severity
The release.yml workflow previously triggered on push to main/v8.x branches and on pull_request events, running the full XCFramework build and validation pipeline. The reviewer explicitly noted: "This cannot be removed, we use this workflow to verify the framework builds on PRs and main." With only workflow_dispatch remaining, the XCFramework build, SPM validation, duplication tests, and app metrics jobs no longer run automatically, breaking CI verification for PRs and the required release-required-check status check.
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} | ||
| merge_target: ${{ inputs.merge_target }} | ||
| path: '|' |
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.
Missing concurrency control enables race conditions in releases
Medium Severity
The old release.yml workflow included a concurrency configuration with explicit comments explaining it prevents "multiple release builds from running simultaneously, which could lead to race conditions in artifact generation and storage" and that "partial releases could corrupt our distribution pipeline." Both new workflows lack any concurrency control. If triggered multiple times (accidentally or otherwise), concurrent runs could create conflicting git tags, corrupt artifacts, or produce inconsistent release state.
Additional Locations (1)
| GITHUB_TOKEN: ${{ steps.token.outputs.token }} | ||
| with: | ||
| version: ${{ inputs.version }} | ||
| path: '|' |
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.
XCFramework artifact handling removed from upload workflow
High Severity
The workflow previously triggered on push to release/** branches and handled XCFramework artifact re-uploading to associate them with the release branch commit (needed for Craft to find artifacts by commit hash). The old comments explained this solves a chicken-and-egg problem with Swift Package Manager checksums. The new workflow removes both the push trigger and all artifact handling, meaning Craft will not find the XCFramework artifacts when preparing the release.
|
Closing this PR in favor of #7153 which takes a minimal approach. Reason: This PR was too aggressive and removed essential build infrastructure:
The reviewers correctly identified that:
The new PR #7153 only changes the action reference from |
Summary
This PR migrates from the deprecated
action-prepare-releaseto the new Craft GitHub Actions.Changes
.github/workflows/release-upload-xcframework.ymlto Craft reusable workflowDocumentation
See https://getsentry.github.io/craft/github-actions/ for more information.
Closes #7152