Repository navigation
cmux-tui: a new build stamp no longer recompiles cmux-remote or acpmux - #22030
lawrencecchen wants to merge 5 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on ad45b0b to compare; do not merge until green; see lane-rules |
|
Visual preview of 8795cbe: in the maintainers' private review feed. |
|
Bead |
cmux-remote read CMUX_TUI_DISTRIBUTION_VERSION and CMUX_TUI_NPM_BOOTSTRAP_VERSION with option_env!. They move to the binary (distribution.rs, same values and fallback) and SshBootstrapConfig::defaults takes them as arguments. acpmux's build script reran on every new stamp (CMUX_GIT_SHORT_SHA, SOURCE_DATE_EPOCH) and every commit, and a rerun recompiles every target of the package. The script moves unchanged (git calls still run in the acpmux crate) to the leaf crate acpmux-build-id. acpmux reads the build id through hub::build(): the default build-id feature (standalone acpmux binary, acpmux tests) gives the stamped value; cmux-tui turns the feature off and installs the same value at start. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
001959c to
9fa2ee5
Compare
|
Found 8 test failures on Blacksmith runners: Failures
|
Correctness review (subagent, head 9fa2ee5)Blocker: none. Behavior is the same where the real binaries run: the distribution constants in Should-fix
Nits: build-id sources outside acpmux no longer add Resolution: should-fix 1 and 2 go to the rebase owner before merge; resolution follows in a later comment. |
…all the build id hub::build, install_build and version_line move to hub/build.rs (re-exported, callers unchanged), so hub/mod.rs does not grow. The cmux-tui tests that run an in-process acpmux Hub install acpmux_build_id::BUILD first, as main() does, so the hub reports the real build id, not `unstamped`. acpmux-build-id takes the workspace version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Resolution (review at 9fa2ee5), new head a144005:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Review (Codex, BLOCK at c244036): agent_message_reply_in_place.rs (added on base after #22030) builds an in-process acpmux Hub inside cmux-tui's tests, where acpmux has no build-id feature, and never installed the id, so its hub reported "unstamped". It now installs it as the other in-process hub tests do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-2f016c499153c1bead46
|
Closed by the perf lead (agent, cmux-ad order 2026-10-31): stale, with no owner. This is an agent PR (lawrencecchen account). It was opened on 2026-10-19, its last commit is from 2026-10-20, no person pushed or commented in the last 3 days (automated review and sweep notes do not count), no live session names it in the lead status of the last 24 h, and it conflicts with feat-cmux-next (DIRTY). The branch |
…x (takes over #22030) (#28776) * cmux-tui: a new build stamp no longer recompiles cmux-remote or acpmux cmux-remote read CMUX_TUI_DISTRIBUTION_VERSION and CMUX_TUI_NPM_BOOTSTRAP_VERSION with option_env!. They move to the binary (distribution.rs, same values and fallback) and SshBootstrapConfig::defaults takes them as arguments. acpmux's build script reran on every new stamp (CMUX_GIT_SHORT_SHA, SOURCE_DATE_EPOCH) and every commit, and a rerun recompiles every target of the package. The script moves unchanged (git calls still run in the acpmux crate) to the leaf crate acpmux-build-id. acpmux reads the build id through hub::build(): the default build-id feature (standalone acpmux binary, acpmux tests) gives the stamped value; cmux-tui turns the feature off and installs the same value at start. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * cmux-tui: rustfmt Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-2f016c499153c1bead46 * acpmux-build-id: drop the unneeded path Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-2f016c499153c1bead46 * acpmux: build id functions in hub/build.rs; in-process test hubs install the build id hub::build, install_build and version_line move to hub/build.rs (re-exported, callers unchanged), so hub/mod.rs does not grow. The cmux-tui tests that run an in-process acpmux Hub install acpmux_build_id::BUILD first, as main() does, so the hub reports the real build id, not `unstamped`. acpmux-build-id takes the workspace version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * acpmux: the Windows stack reservation stays in acpmux's own build script; build() everywhere base reads the build id Takes over #22030 onto current feat-cmux-next: - base added a Windows main-thread stack reservation to acpmux/build.rs (6c93f6e, `cargo:rustc-link-arg-bin=acpmux=...`). Moving that script to the acpmux-build-id leaf crate would apply it to a crate with no acpmux binary, so acpmux.exe would lose it. It stays in a small acpmux/build.rs that reads no stamp and reruns only when it changes. - (in the resolution of the first cherry-pick) every reader of hub::BUILD that base added since (host enroll and update, cloud join, peer status, the remote guard's hint, initialize's _meta, peer.rs) reads hub::build(). - cmux-tui installs the build id on every platform: acpmux is now a dependency on Windows too (the tailnet wrapper). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-2f016c499153c1bead46 * cmux-tui test: the reply-in-place hub installs acpmux's build id Review (Codex, BLOCK at c244036): agent_message_reply_in_place.rs (added on base after #22030) builds an in-process acpmux Hub inside cmux-tui's tests, where acpmux has no build-id feature, and never installed the id, so its hub reported "unstamped". It now installs it as the other in-process hub tests do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-2f016c499153c1bead46 --------- Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com> Co-authored-by: Leo Li <t@e>
Summary
This is the follow-up to #21980. #21980 is merged into
feat-cmux-next. This branch is rebased onfeat-cmux-next, so the diff has only the change of this PR (3 commits).When the Linux build pool builds a new stamp, cmux-remote and acpmux no longer recompile.
ssh_bootstrap.rsreadCMUX_TUI_DISTRIBUTION_VERSIONandCMUX_TUI_NPM_BOOTSTRAP_VERSIONwithoption_env!. The pool sets the first to0.0.0-r2.sha-<stamp>. The two constants move to the binary (cmux-tui/src/distribution.rs) with the same values and the same fallback (CARGO_PKG_VERSION).SshBootstrapConfig::defaultsnow takes them as arguments, as it already takesbuild_identity. Thecmux-remotetests pass the same values.CMUX_GIT_SHORT_SHAandSOURCE_DATE_EPOCH(the pool sets both for each stamp) and on every commit. A rerun recompiles every target of the package (17 s).acpmux-build-id. The logic does not change. The git calls still run in the acpmux crate directory, and the dirty check and thererun-if-changedpaths are still acpmux's.hub::build(). With its defaultbuild-idfeature (the standaloneacpmuxbinary and the acpmux tests), the stamped value is the default.default-features = false, depends onacpmux-build-iditself, and installs the same value inmain(), next toinstall_build_stamp.acpmux --versionuseshub::version_line(), which gives the same text.CMUX_TUI_BUILD_TIME. WhenCMUX_TUI_BUILD_COMMITis set, the script already skips the git source walk.aaaa…is not in the Testbox repository, and that repository is a partial clone (blob:none). Sogit log -1 --format=%ct <stamp>inbuild_time()started a lazy fetch from GitHub. With a real local commit, or withCMUX_TUI_BUILD_TIMEset, the script ran in 0.00 s.Proof (Blacksmith Testbox, 32 vCPU)
Command:
cargo build -v --timings -p cmux-tui --bin cmux-tui --locked. The settings are the pool Linux twin settings (CARGO_INCREMENTAL=0,CARGO_PROFILE_DEV_DEBUG=0). Every stamp variable follows the stamp, as inbuild-cmux-tui-source.sh:CMUX_TUI_BUILD_COMMIT,CMUX_TUI_GHOSTTY_COMMIT,CMUX_TUI_DISTRIBUTION_VERSION=0.0.0-r2.sha-<stamp>,CMUX_GIT_SHORT_SHA,SOURCE_DATE_EPOCH,CMUX_TUI_BUILD_TIME. The stamps are A=aaaa…and B=bbbb….Together with #21980, a new stamp went from about 52 s to about 21 s on the twin. The feat-cmux-next base before #21980 took 51.5 s and 52.5 s.
Readers, before and after, with the same stamp. All values are identical:
For B, every reader reports
bbbb…,bbbbbbbbb 2026-10-15and1792100000.Binary differences between A and B (equal sizes). The sections with differences are the same before and after this PR, so this PR adds no new kind of difference:
.rodata7407,.text3897,.rela.dyn839,.symtab116,.note.gnu.build-id20 bytes.rodata7337,.text3910,.rela.dyn845,.symtab114,.note.gnu.build-id20 bytesThese numbers are larger than the 380 bytes in #21980. Here the pool stamps change string lengths (distribution version, short SHA, date), so code and relocations that refer to
.rodatamove.Tests (Testbox)
cargo fmt --checkfor acpmux, acpmux-build-id, cmux-remote and cmux-tui: okcargo test -p cmux-remote --test ssh_cross_platform_bootstrap: 17 passed (bootstrap with the passed distribution version)cargo test -p cmux-tui --test cli remote_update_need: 5 passedcargo clippy --all-targets -D warningsfor acpmux, acpmux-build-id, cmux-remote and cmux-tui: ok. Also-p acpmux --no-default-features: ok.I ran the timing proof at a501c02. The 2 later commits are a rustfmt change and one path qualification.
No new tests.
Cargo.lockgains theacpmux-build-idpackage (path crate, no new external dependency).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
A new build stamp from the Linux pool now recompiles only
cmux-tui;cmux-remote,acpmux, andcmux-tui-coreno longer do. This cuts the per-stamp rebuild from about 52 s to about 21 s.distribution.rs) and pass intoSshBootstrapConfig::defaults; the build and Ghostty commits install intocmux-tui-coreat the top ofmainviainstall_build_stamp.acpmux's build script moves to the leaf crateacpmux-build-id;cmux-tuidepends onacpmuxwithout its defaultbuild-idfeature and installs the same value throughhub::install_build, so a new stamp recompiles only the tiny leaf crate.identifyorpingmust callinstall_build_stamp; a process that doesn't reportsbuild_commit: nullas before.--version,acpmux --version, andremote-probe --jsonoutput are unchanged.Written for commit 8795cbe. Summary will update on new commits.