Fix native publish artifacts - #122
Conversation
|
Website preview: https://pr-122-eclipsa.xiarenda61.workers.dev Commit: cfc459c |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Updates the publishing/packaging pipeline to ensure native artifacts are correctly included in published packages and that GitHub Actions workflow conditions don’t reference matrix values before expansion.
Changes:
- Added an optimizer artifact “hydration” script that copies declared package files from downloaded/generated artifacts into per-target npm package directories and fails when files are missing.
- Updated the optimizer
artifactsscript and tests to validate workflow ordering and hydration behavior. - Adjusted the publish workflow job/step conditions to avoid job-level
matrixreferences; expanded GTK4 workspace cargo fallback behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/optimizer/scripts/hydrate-npm-artifacts.ts |
New script to populate per-target npm package dirs from artifacts/generated outputs and error on missing files. |
packages/optimizer/package.json |
Switches artifacts script to run hydration instead of invoking the NAPI runner. |
packages/optimizer/mod.test.ts |
Adds coverage for hydration behavior and validates workflow step ordering. |
packages/native/distribution.test.ts |
Adds a regression test ensuring job-level workflow conditions don’t reference matrix. |
packages/native-gtk4/vite.ts |
Expands workspace cargo fallback selection logic for GTK4 host command resolution. |
.github/workflows/publish.yml |
Removes job-level matrix usage in conditions; moves filtering to step-level guards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const fileExists = async (filePath: string) => { | ||
| try { | ||
| await readFile(filePath) | ||
| return true | ||
| } catch { | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
fileExists currently uses readFile() to check existence, which reads the entire artifact into memory (and will be invoked for native .node/.wasm binaries). This is unnecessarily expensive and differs from the existing pattern in prepare-downloaded-artifacts.ts which uses fs.promises.access(). Prefer access() (or stat()) here so existence checks don’t read large files and don’t allocate buffers.
| build_native_host_artifacts: | ||
| if: ${{ (inputs.package == 'native-swiftui' || inputs.package == 'native-android-compose' || inputs.package == 'native-gtk4') && matrix.settings.package == inputs.package }} | ||
| if: ${{ inputs.package == 'native-swiftui' || inputs.package == 'native-android-compose' || inputs.package == 'native-gtk4' }} | ||
| needs: prepare | ||
| runs-on: ${{ matrix.settings.host }} | ||
| strategy: |
There was a problem hiding this comment.
With the job-level if no longer filtering by matrix.settings.package, this job will still allocate runners for all matrix entries (including the two macOS runners) even when publishing a different package, and then skip most steps via step-level if. This can significantly increase CI cost/time. Consider restructuring to avoid spawning non-selected matrix jobs (e.g., split into separate per-package jobs, or make strategy.matrix depend on inputs.package via fromJson(...) so only the matching entries are included).
Fix native publish workflow matrix filtering and hydrate optimizer native package artifacts from downloaded build outputs before publishing. Co-authored-by: codex <codex@openai.com>
49a1a3e to
cfc459c
Compare
Summary
matrixbefore expansionFixes #117
Fixes #120
Verification
bunx vp fmtbunx vp lintbunx vp run typecheckbunx vitest run packages/native/distribution.test.ts packages/optimizer/mod.test.tsbun run --cwd packages/native-gtk4 testbun run test