[WIP] Reuse package artifacts between CI jobs#2597
Draft
fredrikekelund wants to merge 69 commits intotrunkfrom
Draft
[WIP] Reuse package artifacts between CI jobs#2597fredrikekelund wants to merge 69 commits intotrunkfrom
fredrikekelund wants to merge 69 commits intotrunkfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes CI build times by reusing package artifacts across multiple jobs rather than rebuilding for each job.
Changes:
- Added packaging step that creates reusable app artifacts and caches trunk builds for performance comparisons
- Consolidated make scripts to use --arch and --platform arguments instead of separate named scripts
- Updated all CI jobs to download prebuilt artifacts instead of building from scratch
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.buildkite/commands/package-app-for-ci.sh |
New script that packages the app and creates tarball artifacts, with special caching for trunk builds |
.buildkite/commands/build-for-mac.sh |
New consolidated script for Mac builds that downloads prebuilt artifacts for dev builds |
.buildkite/commands/build-for-windows.ps1 |
Updated to download prebuilt artifacts for dev builds instead of packaging in-place |
.buildkite/commands/run-e2e-tests.sh |
Simplified to download and extract prebuilt artifacts instead of packaging |
.buildkite/commands/run-metrics-tests.sh |
Enhanced to download current branch artifacts and restore cached trunk artifacts |
.buildkite/commands/install-node-dependencies.sh |
Updated cache key format and replaced deprecated --unsafe-perm flag |
.buildkite/commands/prepare-environment.sh |
Updated comment to clarify purpose |
.buildkite/pipeline.yml |
Added package artifact steps and updated job dependencies |
scripts/package-in-isolation.ts |
Refactored to accept --arch and --platform arguments via yargs |
package.json |
Consolidated package and make scripts to use arch/platform arguments |
apps/studio/package.json |
Simplified make script to use --skip-package flag |
tools/compare-perf/performance.ts |
Added logic to detect and use prebuilt artifacts |
Comments suppressed due to low confidence (1)
.buildkite/pipeline.yml:168
- The Windows dev build uses a matrix that includes both x64 and arm64 architectures, but the depends_on only specifies package_app_artifacts_windows_x64. This means the arm64 build will fail when trying to download the prebuilt artifact because there's no package_app_artifacts_windows_arm64 step defined. Either add a package step for windows-arm64, or remove arm64 from the matrix.
- label: 🔨 Windows Dev Build - {{matrix}}
agents:
queue: windows
command: powershell -File .buildkite/commands/build-for-windows.ps1 -BuildType dev -Architecture {{matrix}}
artifact_paths:
- out\**\studio-setup.exe
- out\**\studio-update.nupkg
- out\**\RELEASES
- out\**\*.appx
plugins: [$CI_TOOLKIT_PLUGIN, $NVM_PLUGIN]
matrix:
- x64
- arm64
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
204
to
219
| - &mac_build_common | ||
| label: 🔨 Mac Release Build - x64 | ||
| agents: | ||
| queue: mac | ||
| command: | | ||
| .buildkite/commands/prepare-environment.sh | ||
|
|
||
| .buildkite/commands/install-node-dependencies.sh | ||
| node ./scripts/confirm-tag-matches-version.mjs | ||
|
|
||
| echo "--- :node: Building Binary" | ||
| npm run make:macos-{{matrix}} | ||
|
|
||
| # Local trial and error show this needs to run before the DMG generation (obviously) but after the binary has been built | ||
| echo "--- :hammer: Rebuild fs-attr if necessary before generating DMG" | ||
| case {{matrix}} in | ||
| x64) | ||
| echo "Rebuilding fs-xattr for {{matrix}} architecture" | ||
| npm rebuild fs-xattr --cpu universal | ||
| ;; | ||
| arm64) | ||
| echo "No need to rebuild fs-xattr because it works out of the box on Apple Silicon" | ||
| ;; | ||
| *) | ||
| echo "^^^ +++ Unexpected architecture {{matrix}}" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
||
| echo "--- :node: Packaging in DMG" | ||
| npm run make:dmg-{{matrix}} | ||
|
|
||
| echo "--- 📃 Notarizing Binary" | ||
| bundle exec fastlane notarize_binary | ||
| command: bash .buildkite/commands/build-for-mac.sh release x64 | ||
| plugins: [$CI_TOOLKIT_PLUGIN, $NVM_PLUGIN] | ||
| artifact_paths: | ||
| - apps/studio/out/**/*.app.zip | ||
| - apps/studio/out/*.dmg | ||
| matrix: | ||
| - x64 | ||
| - arm64 | ||
| notify: | ||
| - github_commit_status: | ||
| context: All Mac Release Builds | ||
|
|
||
| - <<: *mac_build_common | ||
| label: 🔨 Mac Release Build - arm64 | ||
| command: bash .buildkite/commands/build-for-mac.sh release arm64 | ||
| notify: | ||
| - github_commit_status: | ||
| context: All Mac Release Builds |
There was a problem hiding this comment.
The Mac release build steps are missing artifact_paths definitions. These builds create .app.zip and .dmg files that should be uploaded as artifacts for downstream steps to consume. Add artifact_paths similar to the dev build steps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
Proposed Changes
Note
This PR uses #2565 as the base branch, to save myself some headaches in having to deal with merge conflicts later.
Several CI jobs build and package the app. Previously, each job handled this individually rather than reusing the same packaged artifact. This PR fixes that by implementing the following changes:
.buildkite/commands/package-app-for-ci.shscript that packages the app and saves it to the Buildkite cache.scripts/package-in-isolation.tsto only build and package the app (i.e., not make it).make:*scripts inapps/studio/package.jsoninto a singlemakescript. The rootpackage.jsonfile now passes--archand--platformoptions as needed. Reduces repetition and clarifies that we orchestrate from the root, and just package/make inapps/studio.trunkdev builds using a fixed cache key.trunkpackage artifacts. This should be a big win and help us reduce overall compute time. For comparison, the current compare-perf job builds and packages thetrunkversion of the app with every commit to every PR branch.Testing Instructions
CI should pass. The artifacts from the
package_app_artifactsstep should be reused by the E2E tests and the compare-perf job.Pre-merge Checklist