build: #5422 — dev/release/dist build taxonomy, rlib-only runtime/stdlib + staticlib wrappers, slim CLI#5469
Conversation
…ib + staticlib wrappers, slim CLI Implements all four parts of #5422: 1. perry-dev profile — fast local builds (lto off, cgu=16, opt-level=1, incremental, no strip). 2. Staticlib split — perry-runtime/perry-stdlib are rlib-only; the .a archives are emitted by new perry-runtime-static / perry-stdlib-static wrapper crates ([lib] name keeps the libperry_{runtime,stdlib}.a basenames). Wrappers carry strip=false + cgu=16 so ThinLTO keeps the exported C API (cf. #5140). Wrappers are NOT in default-members, so a plain `cargo build` no longer emits the .a. Auto-optimize (optimized_libs.rs), geisterhand (library_search.rs), and every workflow/script that produced the archives now build the -static crates. 3. Slim CLI — crates/perry/Cargo.toml feature taxonomy (default=full-cli -> dev-cli/publish-cli/mobile-cli/updater-cli/native-cli/audit-cli + backend-{js,swiftui,arkts,glance,wear-tiles,wasm}/all-codegen-backends). `--no-default-features --features dev-cli` builds a slim compiler CLI; gated commands drop from --help and disabled --target backends error cleanly. 4. dist profile — mirrors release exactly (incl. per-package overrides); release-packages.yml builds official artifacts with --profile dist (CARGO_PROFILE_DIST_PANIC; target/.../release -> target/.../dist for cargo output, staging layout + Homebrew build-from-source stay on --release). scripts/cargo_timing_summary.py surfaces the slowest build units. Validated locally: symbol guard (check_runtime_symbols.sh), prebuilt-link, and auto-optimize-rebuild link paths all compile+link+run a hello-world; default / dev-cli / bare --no-default-features all build; slim CLI behaves correctly. The release-packages.yml --profile dist cutover needs a tag/CI run to confirm the full release matrix.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces ChangesBuild Taxonomy Overhaul
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry/src/commands/compile/library_search.rs (1)
1353-1360:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winManual fallback command is missing
perry-stdlib-static.The error-path “build manually” command no longer matches the real invocation and can produce an incomplete geisterhand lib set.
Patch suggestion
"Cannot auto-build geisterhand libraries: Perry workspace not found.\n\ Build manually from the Perry source directory:\n \ CARGO_TARGET_DIR=target/geisterhand cargo build --release \\\n \ -p perry-runtime-static --features perry-runtime/geisterhand \\\n \ -p {} --features geisterhand \\\n \ - -p perry-ui-geisterhand", + -p perry-ui-geisterhand \\\n \ + -p perry-stdlib-static", ui_crate )🤖 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 `@crates/perry/src/commands/compile/library_search.rs` around lines 1353 - 1360, The error message string starting around line 1353 that provides manual build instructions for geisterhand libraries is missing the perry-stdlib-static package in the cargo build command. Add a new line with `-p perry-stdlib-static --features geisterhand \` to the multi-line build command string, placing it after the perry-runtime-static package line and before the ui_crate package specification to ensure all required packages are included in the manual build fallback instructions.
🧹 Nitpick comments (1)
scripts/cargo_timing_summary.py (1)
63-63: ⚡ Quick winUse a context manager to ensure the file is closed.
The file is opened but never explicitly closed. While CPython's reference counting will close it promptly in practice, using a
withstatement is the recommended Python idiom and ensures proper cleanup across all Python implementations.♻️ Proposed fix to use context manager
- units = extract_units(open(report, encoding="utf-8").read()) + with open(report, encoding="utf-8") as f: + units = extract_units(f.read())🤖 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 `@scripts/cargo_timing_summary.py` at line 63, The file opened in the extract_units function call on the line with open(report, encoding="utf-8").read() is not using a context manager, which means it relies on reference counting for cleanup. Wrap the open call in a with statement to ensure the file is properly closed. Change the code so that a with statement manages the file object and passes its contents to extract_units, following the Python idiom for resource management.
🤖 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 @.github/workflows/feature-matrix.yml:
- Line 49: The cargo build command in the feature-matrix workflow is missing
stdlib wrapper targets that are present in other migrated CI configurations.
Update the run command that builds with cargo build --release to include the
additional stdlib wrapper targets alongside the existing -p perry-runtime -p
perry-runtime-static -p perry packages. Check the other CI workflow files to
identify which stdlib wrapper targets should be added to ensure feature-matrix
runs have prebuilt stdlib archives available.
In `@CHANGELOG.md`:
- Around line 1-48: This PR modifies three files that are reserved for
maintainers according to repository policy: CHANGELOG.md, the version in
Cargo.toml's workspace.package section, and the version line in CLAUDE.md. If
you are not a maintainer, revert all changes to these three files and let the
maintainer handle the version bumps and changelog entry at merge time. If you
are a maintainer performing this PR, confirm that this is intentional and aligns
with your release process.
In `@crates/perry/src/commands/mod.rs`:
- Around line 15-16: The `#[cfg(feature = "watch-cli")]` attribute on line 15
only applies to the immediate following `pub mod dev;` declaration. Add
individual `#[cfg(feature = "watch-cli")]` attributes above each of the
following module declarations: `doctor`, `explain`, `fix_applier`, `fixer`,
`harmonyos_hap`, `i18n`, `init`, `install`, `lock`, `login`, and
`lower_diagnostic` in the file crates/perry/src/commands/mod.rs to ensure each
module is conditionally compiled under the watch-cli feature flag.
- Around line 7-14: The cfg feature attribute on line 7 only gates the appstore
module because cfg attributes in Rust apply only to the immediately following
item. The modules attest, audit, cache, check, compile, and deps are currently
unconditionally compiled. Determine if all these modules should be gated under
the mobile-cli feature, and if so, add the #[cfg(feature = "mobile-cli")]
attribute individually to each of the attest, audit, cache, check, compile, and
deps module declarations so they are only compiled when the mobile-cli feature
is enabled.
In `@docs/src/contributing/building.md`:
- Line 55: The cargo build command references an incorrect crate name
`perry-codegen-llvm` which does not exist. Update the command to use the correct
crate name `perry-codegen` by replacing `perry-codegen-llvm` with
`perry-codegen` in the build command so it reads `cargo build --release -p
perry-codegen`.
In `@scripts/cargo_timing_summary.py`:
- Around line 42-47: The extract_units function lacks error handling for
json.loads() which can raise a JSONDecodeError if the extracted UNIT_DATA string
is malformed. Wrap the json.loads(m.group(1)) call in a try/except block that
catches JSONDecodeError and re-raises it as a ValueError with a clear diagnostic
message (similar to the existing ValueError for missing UNIT_DATA), including
details about the parsing failure to help with debugging when the JSON format is
invalid.
---
Outside diff comments:
In `@crates/perry/src/commands/compile/library_search.rs`:
- Around line 1353-1360: The error message string starting around line 1353 that
provides manual build instructions for geisterhand libraries is missing the
perry-stdlib-static package in the cargo build command. Add a new line with `-p
perry-stdlib-static --features geisterhand \` to the multi-line build command
string, placing it after the perry-runtime-static package line and before the
ui_crate package specification to ensure all required packages are included in
the manual build fallback instructions.
---
Nitpick comments:
In `@scripts/cargo_timing_summary.py`:
- Line 63: The file opened in the extract_units function call on the line with
open(report, encoding="utf-8").read() is not using a context manager, which
means it relies on reference counting for cleanup. Wrap the open call in a with
statement to ensure the file is properly closed. Change the code so that a with
statement manages the file object and passes its contents to extract_units,
following the Python idiom for resource management.
🪄 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 Plus
Run ID: 4ff1d747-8d97-4507-b3e7-78af03c28f46
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
.github/workflows/container-tests.yml.github/workflows/feature-matrix.yml.github/workflows/node-core-subset.yml.github/workflows/node-suite-guard.yml.github/workflows/npm-package-sweep.yml.github/workflows/release-packages.yml.github/workflows/simctl-tests.yml.github/workflows/test.ymlCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdCargo.tomlREADME.mdcrates/perry-runtime-static/Cargo.tomlcrates/perry-runtime-static/src/lib.rscrates/perry-runtime/Cargo.tomlcrates/perry-stdlib-static/Cargo.tomlcrates/perry-stdlib-static/src/lib.rscrates/perry-stdlib/Cargo.tomlcrates/perry/Cargo.tomlcrates/perry/src/commands/compile.rscrates/perry/src/commands/compile/bootstrap.rscrates/perry/src/commands/compile/library_search.rscrates/perry/src/commands/compile/optimized_libs.rscrates/perry/src/commands/compile/targets.rscrates/perry/src/commands/mod.rscrates/perry/src/main.rsdocs/src/contributing/building.mdrun_parity_tests.shscripts/bisect_1114.shscripts/cargo_timing_summary.pyscripts/run_doc_tests.shscripts/run_memory_stability_tests.shscripts/run_tty_pty_smoke.sh
| ## v0.5.1197 — build: #5422 — dev/release/dist build taxonomy, rlib-only runtime/stdlib, slim CLI | ||
|
|
||
| Reworks Perry's build surface so local development is light while official release artifacts | ||
| stay explicit and optimized. Four parts (issue #5422): | ||
|
|
||
| **1. Fast `perry-dev` profile.** New `[profile.perry-dev]` inherits `release` but drops the | ||
| distribution-grade settings (`lto = false`, `codegen-units = 16`, `opt-level = 1`, | ||
| `incremental = true`, no strip). Local loop: `cargo check -p perry` for correctness, | ||
| `cargo build --profile perry-dev -p perry` for an optimized dev binary (`target/perry-dev/perry`). | ||
|
|
||
| **2. Staticlib split.** `perry-runtime` and `perry-stdlib` are now `crate-type = ["rlib"]` only, | ||
| so a plain `cargo build` no longer emits the heavy `libperry_runtime.a` / `libperry_stdlib.a` as | ||
| a side effect. The archives are produced by two new wrapper crates, `perry-runtime-static` and | ||
| `perry-stdlib-static` (each `[lib] name = "perry_runtime"` / `"perry_stdlib"`, `crate-type = | ||
| ["staticlib"]`), so every consumer's `.a` basename is unchanged — only the cargo *package* name | ||
| (`-p perry-runtime-static`) changes. The wrappers carry `strip = false` + `codegen-units = 16` | ||
| profile overrides (mirroring perry-ext-events #5140) so ThinLTO doesn't internalize and drop the | ||
| exported C API. The auto-optimize rebuild path (`optimized_libs.rs`), the geisterhand build | ||
| (`library_search.rs`), `stage-npm.sh`'s symbol guard, and every workflow / script that produced | ||
| the archives now build the `-static` crates. The wrappers are deliberately **not** in | ||
| `default-members`. Validated locally: sentinel symbols present (`check_runtime_symbols.sh`), | ||
| prebuilt-link, and auto-optimize-rebuild paths all compile + link + run a hello-world. | ||
|
|
||
| **3. Slim developer CLI.** `crates/perry/Cargo.toml` gains a feature taxonomy: `default = | ||
| ["full-cli"]` (unchanged shipped CLI) decomposes into `dev-cli` (compile/run/check/types/cache + | ||
| watch), `publish-cli`, `mobile-cli`, `updater-cli`, `native-cli`, `audit-cli`, and per-backend | ||
| `backend-{js,swiftui,arkts,glance,wear-tiles,wasm}` / `all-codegen-backends`. `cargo build -p | ||
| perry --no-default-features --features dev-cli` builds a slimmer compiler CLI: gated commands | ||
| drop out of `--help` and disabled `--target` backends return a clear "built without the | ||
| `<feature>` feature" error. The publish/setup/audit *modules* stay compiled (their config/audit | ||
| helpers are used by core paths); only their commands are gated. The 6 codegen-backend crates plus | ||
| `notify`, `ed25519-dalek`, `rand` became optional deps. Default / `dev-cli` / bare | ||
| `--no-default-features` all compile. | ||
|
|
||
| **4. Explicit `dist` profile.** New `[profile.dist]` mirrors `release` exactly (ThinLTO, | ||
| `codegen-units = 1`, `opt-level = 3`, strip) including all per-package overrides, and | ||
| `release-packages.yml` now builds official artifacts with `--profile dist` | ||
| (`CARGO_PROFILE_DIST_PANIC` for the panic=abort variant; cargo-output paths moved | ||
| `target/.../release/` → `target/.../dist/`, while the staging package layout and the | ||
| build-from-source Homebrew formula stay on `--release`). `scripts/cargo_timing_summary.py` | ||
| parses `cargo --timings` output to surface the slowest build units so regressions are visible. | ||
|
|
||
| Docs: `docs/src/contributing/building.md` documents the three-tier taxonomy, the `-static` | ||
| crates, and the slim CLI. | ||
|
|
||
| > **Release validation note:** the `--profile dist` cutover and the `-static` additions across | ||
| > `release-packages.yml` could not be exercised by the local build; they need a tag/CI run to | ||
| > confirm the full release matrix. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify version consistency across workspace and documentation files.
# Expected: v0.5.1197 appears in Cargo.toml [workspace.package].version and CLAUDE.md **Current Version:** line
# Check workspace version in Cargo.toml
echo "=== Cargo.toml workspace version ==="
rg -A2 '\[workspace\.package\]' Cargo.toml | grep -E 'version\s*='
# Check CLAUDE.md current version
echo "=== CLAUDE.md current version ==="
rg '\*\*Current Version:\*\*' CLAUDE.md
# Verify v0.5.1197 appears in all three
echo "=== v0.5.1197 references ==="
rg 'v0\.5\.1197' Cargo.toml CLAUDE.md CHANGELOG.mdRepository: PerryTS/perry
Length of output: 1303
This PR modifies CHANGELOG.md, Cargo.toml version, and CLAUDE.md version—files that should only be bumped by maintainers per the repository policy.
The established workflow in CLAUDE.md explicitly states: "PRs from outside contributors should not touch [workspace.package] version in Cargo.toml, the **Current Version:** line in CLAUDE.md, or CHANGELOG.md. The maintainer bumps the version and writes the changelog entry at merge time." This PR modifies all three. Confirm whether this is a maintainer PR, or if the version/changelog changes should be removed for the maintainer to handle at merge time.
🤖 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 `@CHANGELOG.md` around lines 1 - 48, This PR modifies three files that are
reserved for maintainers according to repository policy: CHANGELOG.md, the
version in Cargo.toml's workspace.package section, and the version line in
CLAUDE.md. If you are not a maintainer, revert all changes to these three files
and let the maintainer handle the version bumps and changelog entry at merge
time. If you are a maintainer performing this PR, confirm that this is
intentional and aligns with your release process.
Source: Learnings
| #[cfg(feature = "mobile-cli")] | ||
| pub mod appstore; | ||
| pub mod attest; | ||
| pub mod audit; | ||
| pub mod cache; | ||
| pub mod check; | ||
| pub mod compile; | ||
| pub mod deps; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the actual #[cfg] attribute placement in commands/mod.rs
# Show the full module declaration section with context
rg -n -B 1 '^pub mod ' crates/perry/src/commands/mod.rs | head -60Repository: PerryTS/perry
Length of output: 947
The #[cfg(feature = "mobile-cli")] attribute on line 7 only gates appstore, not the following modules.
In Rust, a #[cfg] attribute applies only to the immediately following item. The single #[cfg(feature = "mobile-cli")] on line 7 gates only pub mod appstore; on line 8, leaving attest, audit, cache, check, compile, and deps (lines 9-14) unconditionally compiled.
Verify whether this is intentional. If these modules should all be gated under mobile-cli, each requires its own attribute:
#[cfg(feature = "mobile-cli")]
pub mod appstore;
#[cfg(feature = "mobile-cli")]
pub mod attest;
#[cfg(feature = "mobile-cli")]
pub mod audit;
// ... etc🤖 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 `@crates/perry/src/commands/mod.rs` around lines 7 - 14, The cfg feature
attribute on line 7 only gates the appstore module because cfg attributes in
Rust apply only to the immediately following item. The modules attest, audit,
cache, check, compile, and deps are currently unconditionally compiled.
Determine if all these modules should be gated under the mobile-cli feature, and
if so, add the #[cfg(feature = "mobile-cli")] attribute individually to each of
the attest, audit, cache, check, compile, and deps module declarations so they
are only compiled when the mobile-cli feature is enabled.
| #[cfg(feature = "watch-cli")] | ||
| pub mod dev; |
There was a problem hiding this comment.
Verify that all watch-cli modules have individual #[cfg] attributes.
The #[cfg(feature = "watch-cli")] on line 15 only applies to pub mod dev; on line 16. According to the AI summary, multiple modules (dev, doctor, explain, fix_applier, fixer, harmonyos_hap, i18n, init, install, lock, login, lower_diagnostic) should be gated under this feature.
Each module needs its own #[cfg(feature = "watch-cli")] attribute, or they remain unconditionally compiled.
🤖 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 `@crates/perry/src/commands/mod.rs` around lines 15 - 16, The `#[cfg(feature =
"watch-cli")]` attribute on line 15 only applies to the immediate following `pub
mod dev;` declaration. Add individual `#[cfg(feature = "watch-cli")]` attributes
above each of the following module declarations: `doctor`, `explain`,
`fix_applier`, `fixer`, `harmonyos_hap`, `i18n`, `init`, `install`, `lock`,
`login`, and `lower_diagnostic` in the file crates/perry/src/commands/mod.rs to
ensure each module is conditionally compiled under the watch-cli feature flag.
Resolve CHANGELOG.md conflict by taking main's version: per the external- contributor policy (and CodeRabbit), PR branches must not add CHANGELOG entries; the maintainer writes them at merge time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ4RdAWQhxvqyS9vWeLoTB
- feature-matrix.yml: build the full runtime/stdlib + -static wrapper set (matching node-suite-guard / simctl-tests) so feature-matrix runs have the prebuilt stdlib archives. - building.md: fix codegen build command crate name (perry-codegen, not the nonexistent perry-codegen-llvm). - cargo_timing_summary.py: wrap UNIT_DATA json.loads in try/except, raising a clear ValueError on malformed JSON instead of a bare JSONDecodeError. Note: CodeRabbit's two "cfg only gates the next item" findings on commands/mod.rs are false positives — each module that needs gating already carries its own #[cfg]; the in-between modules are intentionally always compiled (verified: default, dev-cli, and no-default-features all build). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ4RdAWQhxvqyS9vWeLoTB
Reorder the feature-gated `targets` imports to satisfy `cargo fmt --all --check` (single imports precede grouped imports). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQ4RdAWQhxvqyS9vWeLoTB
Implements all four parts of #5422.
1. Fast
perry-devprofileNew
[profile.perry-dev]inheritsreleasebut drops the distribution-grade settings (lto = false,codegen-units = 16,opt-level = 1,incremental = true, no strip). Local loop:cargo check -p perry, thencargo build --profile perry-dev -p perry(output attarget/perry-dev/perry).2. Staticlib split
perry-runtimeandperry-stdlibare nowcrate-type = ["rlib"]only, so a plaincargo buildno longer emitslibperry_runtime.a/libperry_stdlib.aas a side effect. Two new wrapper cratesperry-runtime-static/perry-stdlib-static(each[lib] name = "perry_runtime"/"perry_stdlib",crate-type = ["staticlib"]) produce the archives — so every consumer's.abasename is unchanged; only the cargo package name (-p perry-runtime-static) changes. The wrappers carrystrip = false+codegen-units = 16overrides so ThinLTO doesn't internalize/drop the exported C API (the #5140 ext-events trap). They are deliberately not indefault-members.Rewired: auto-optimize rebuild (
optimized_libs.rs), geisterhand build (library_search.rs), and every workflow / script that produced the archives now build the-staticcrates.3. Slim developer CLI
crates/perry/Cargo.tomlgains a feature taxonomy:default = ["full-cli"](shipped CLI unchanged) →dev-cli/publish-cli/mobile-cli/updater-cli/native-cli/audit-cli+ per-backendbackend-{js,swiftui,arkts,glance,wear-tiles,wasm}/all-codegen-backends.cargo build -p perry --no-default-features --features dev-clibuilds a slim compiler CLI: gated commands drop out of--help, disabled--targetbackends return a clear "built without the<feature>feature" error. publish/setup/audit modules stay compiled (their config/audit helpers are used by core paths); only their commands are gated.notify,ed25519-dalek,rand, and the 6 codegen-backend crates became optional deps.4. Explicit
distprofileNew
[profile.dist]mirrorsreleaseexactly (incl. all per-package overrides).release-packages.ymlnow builds official artifacts with--profile dist(CARGO_PROFILE_DIST_PANICfor the panic=abort variant; cargo-output pathstarget/.../release/→target/.../dist/, while the staging package layout and the build-from-source Homebrew formula stay on--release).scripts/cargo_timing_summary.pyparsescargo --timingsoutput to surface the slowest build units.Docs:
docs/src/contributing/building.md,CONTRIBUTING.md,README.mddocument the dev/release/dist taxonomy + slim CLI.Validation (local)
check_runtime_symbols.sh— all sentinel C symbols survive in the wrapper.a.-p perry-runtime-static) — both compile, link, and run a hello-world.cargo checkpasses for default,--features dev-cli, and bare--no-default-features.--help, native compile works,--target wasm→ clean backend-disabled error.cargo test -p perry— 618 pass; 4 pre-existingoptimized_libsauto-opt-freshness tests fail identically on basemainin this sandbox (mtime-sensitive; pass in canonical CI) — not a regression.The
--profile distcutover and the-staticadditions acrossrelease-packages.yml/stage-npm.shcould not be exercised by a local build — they need a tag/CI run to confirm the full release matrix. Workflow-level-staticmisses (if any) fail loudly (CI red, missing-.aerror), not silently.Closes #5422.
Summary by CodeRabbit
Release Notes
perry-runtime-static,perry-stdlib-static) for producing the expected.aarchives.perry-devand adistprofile for packaging workflows.--no-default-features --features dev-cli.