Fix #35: Add macOS static package build - #36
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds end-to-end macOS static packaging: a GitHub Actions macOS job, Makefile target, a bash packaging script to build an Apple Silicon static binary, extended unit tests, a small static-php patch, and documentation updates; release packaging now includes the macOS archive. ChangesmacOS Static Binary Packaging
Sequence DiagramsequenceDiagram
participant Developer
participant CI as GitHub Actions
participant Makefile
participant PackScript as build/package-macos.sh
participant Composer
participant Cargo
participant Artifact as ActionsArtifact
participant Release as release
Developer->>Makefile: run `make package-macos` (local)
CI->>Makefile: macos job runs `make package-macos`
Makefile->>PackScript: invoke --dist-dir / --arch
PackScript->>Composer: install deps / create PHAR
PackScript->>Cargo: rustup target add & cargo build highlighter
PackScript->>Artifact: create & pack yiipress-macos-arm64.tar.gz
CI->>Artifact: upload artifact
CI->>Release: release job waits for macos and copies artifact
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds native macOS static binary packaging alongside the existing PHAR, Linux, Windows, and distroless packaging paths.
Changes:
- Adds a macOS packaging script and
make package-macostarget. - Extends the static package workflow to build, smoke test, upload, and release macOS artifacts.
- Updates docs, roadmap, README, and packaging configuration tests for macOS support.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
build/package-macos.sh |
New Bash packaging flow for macOS static executable builds. |
Makefile |
Adds macOS package variables and target. |
.github/workflows/package-static.yml |
Adds macOS CI packaging job and release artifact handling. |
tests/Unit/Packaging/ConfigurationPackagingTest.php |
Adds assertions covering macOS package configuration. |
README.md |
Documents macOS static binary availability and package target. |
docs/deployment.md |
Documents macOS packaging usage and artifact publication. |
roadmap.md |
Marks macOS release packaging as included. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
build/package-macos.sh (1)
93-103: ⚡ Quick winAdd a timeout and retry to the archive download.
curl -fsSLhere has no time limit or retry, so a stalled connection to GitHub/PECL can hang the build indefinitely, and a transient failure aborts the whole job. The Windows packaging script already bounds its download (-TimeoutSec 300); mirror that here.♻️ Proposed change
- curl -fsSL "$url" -o "$archive" + curl -fsSL --max-time 300 --retry 3 --retry-delay 5 "$url" -o "$archive"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/package-macos.sh` around lines 93 - 103, The download in expand_tar_gz_archive uses curl without timeouts or retries; update the curl invocation in function expand_tar_gz_archive to include a maximum time (e.g., --max-time or --connect-timeout) and a retry policy (e.g., --retry N and optional --retry-delay or --retry-connrefused) so the download will fail fast on stalls and retry transient errors; keep the rest of the function (mktemp, tar, cleanup) intact and ensure the curl exit code still causes the script to fail if all retries/timeouts are exhausted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@build/package-macos.sh`:
- Around line 93-103: The download in expand_tar_gz_archive uses curl without
timeouts or retries; update the curl invocation in function
expand_tar_gz_archive to include a maximum time (e.g., --max-time or
--connect-timeout) and a retry policy (e.g., --retry N and optional
--retry-delay or --retry-connrefused) so the download will fail fast on stalls
and retry transient errors; keep the rest of the function (mktemp, tar, cleanup)
intact and ensure the curl exit code still causes the script to fail if all
retries/timeouts are exhausted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72ab7ab5-bd37-414a-b738-b3cca3ed2d86
📒 Files selected for processing (7)
.github/workflows/package-static.ymlMakefileREADME.mdbuild/package-macos.shdocs/deployment.mdroadmap.mdtests/Unit/Packaging/ConfigurationPackagingTest.php
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/deployment.md`:
- Line 170: Update the documentation line describing the macOS packaging
requirements for `make package-macos` to hyphenate the compound adjective:
change the phrase "Xcode command line build toolchain" to "Xcode command-line
build toolchain" in the sentence mentioning host requirements (the line
referencing `make package-macos`, `PACKAGE_MACOS_ARCH` and
`PACKAGE_MACOS_DIST`).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f8243ca-fd99-4f82-bf4e-a30f45b3ec9e
📒 Files selected for processing (4)
.github/workflows/package-static.ymlbuild/package-macos.shdocs/deployment.mdtests/Unit/Packaging/ConfigurationPackagingTest.php
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/package-static.yml
- tests/Unit/Packaging/ConfigurationPackagingTest.php
- build/package-macos.sh
Summary
Verification
Closes #35
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes
Chores