Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3a0d75ccc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| kind="rust", | ||
| cwd=WORKSPACE_ROOT, | ||
| build_cmd=["cargo", "build", "--release", "-p", "codex-cli", "--features", "custom-features"], | ||
| watch_roots=(WORKSPACE_ROOT / "cli", WORKSPACE_ROOT / "core", WORKSPACE_ROOT / "exec", WORKSPACE_ROOT / "protocol", WORKSPACE_ROOT / "config", WORKSPACE_ROOT / "state", WORKSPACE_ROOT / "mcp-server", WORKSPACE_ROOT / "deep-research", WORKSPACE_ROOT / "utils"), |
There was a problem hiding this comment.
watch_roots
These watch_roots cover only a subset of each binary's Rust dependencies. I checked codex-rs/cli/Cargo.toml and codex-rs/tui/Cargo.toml: codex-cli directly depends on crates such as cloud-tasks, login, responses-api-proxy, rmcp-client, and supervisor, while codex-tui depends on backend-client, feedback, file-search, shell-command, and several utils/* crates that are not hashed here. With the new just fast-build --changed-only flow, edits in those crates leave the fingerprint unchanged, so the script skips rebuilding binaries that have actually changed.
Useful? React with 👍 / 👎.
| installable = [name for name in resolve_targets(args.targets) if TARGETS[name].install_map] | ||
| if not installable: | ||
| logger.info("Selected targets do not produce installable binaries; skipping install.") | ||
| return 0 | ||
| install_targets(installable, logger) |
There was a problem hiding this comment.
cmd_fast_build() can legitimately skip every build when --changed-only finds an up-to-date cache, but cmd_fast_build_install() immediately re-resolves the full requested set and copies every installable artifact anyway. In practice, once .codex-fast-build-cache.json exists, running just fast-build-install after cargo clean (or after deleting one artifact) raises Missing build artifact ... even though there were no source changes; if only a non-installable target changed, this path can also redeploy stale CLI/TUI binaries.
Useful? React with 👍 / 👎.
| if "--release" in patched and args.profile != "release": | ||
| patched.remove("--release") | ||
| patched[2:2] = ["--profile", args.profile] | ||
| patched.extend(["-j", str(args.jobs)]) |
There was a problem hiding this comment.
fast-build-install honor the selected Cargo profile
The new --profile flag rewrites cargo build --release to cargo build --profile <name>, but the install sources are still hard-coded to target/release in the target table above. That means scripts/fast_build.py fast-build-install --profile dev codex-cli either copies an old release binary or fails with Missing build artifact because Cargo wrote target/dev/codex instead, so the advertised profile override is unsafe for install flows.
Useful? React with 👍 / 👎.
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
1 participant
Motivation
justtasks can call a canonical implementation.Description
scripts/fast_build.pythat exposesfast-build,fast-build-install, andupstream-syncsubcommands, per-target configuration (codex-cli,codex-tui,codex-gui,codex-gui-x,extensions), and change-detection modesmd5,mtime, andcargo-metadata(default methodmd5), and honorsCODEX_FAST_BUILD_JOBS/--jobs(default 6).scripts/resolve_merge_conflicts.pyimplementing path-pattern rules and an “upstream-first + reinject unique local lines” strategy, andscripts/upstream_sync.pyas a thin wrapper that invokes the upstream merge flow using the orchestrator.codex-rs/fast_build.ps1that forwards task, method, jobs, targets and flags into the shared Python pipeline and supportspy -3when available.justentry points into both repo rootjustfileandcodex-rs/justfileasfast-build,fast-build-install, andupstream-syncto call the shared script (via platformpythonselection)..codex-fast-build-cache.jsoncache path inscripts/fast_build.pyand updated fingerprinting to optionally includecargo metadatafor Rust targets..gitignoreand updated documentation and release notes to reference the new implementation names andjusttasks (files edited:CHANGELOG.md,CLAUDE.md,codex-rs/README.md,docs/development/*,docs/plan/MERGE_STRATEGY.md).Testing
python3 scripts/fast_build.py list-targetsand confirmed target listing succeeded (PASS).python3 scripts/fast_build.py fast-build --helpandpython3 scripts/upstream_sync.py --help(PASS).python3 -m py_compile scripts/fast_build.py scripts/resolve_merge_conflicts.py scripts/upstream_sync.py(PASS).scripts/resolve_merge_conflicts.pyresolves markers using the expected strategy (PASS).git diff --checkon modified docs/justfile entries and validatedjustwiring by invoking the underlying Python entry points (note:justbinary not present in this environment, sojusttasks were validated by inspecting and directly running the Python scripts) (PASS with environment note).Codex Task