fix: make clean_module report failed clean commands with diagnostics#6
fix: make clean_module report failed clean commands with diagnostics#6lb1192176991-lab wants to merge 9 commits into
Conversation
- Capture the subprocess result from run_text_process and check returncode - Return False when clean command exits non-zero, with module name and exit code - Include stdout/stderr output in failure diagnostic messages - Handle TimeoutExpired and FileNotFoundError separately with clear messages - Preserve existing successful clean behaviour (returns True) - Pass module-level env (e.g. CARGO_TERM_COLOR) to clean commands Addresses: benglezhenjun#1
![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: 93ce971bc0
ℹ️ 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".
| def ensure_commit_is_ancestor(commit): | ||
| comparison = request(f"/repos/{head_repo}/compare/{commit}...{head_sha}") | ||
| status = comparison.get("status") | ||
| if status not in {"ahead", "identical"}: |
There was a problem hiding this comment.
When a contributor runs build.py and then pushes additional code changes, this compare still returns ahead, so the old diagnostic/build-<commit>.json remains accepted even though it no longer describes the PR head; the PR files API is also checking the full diff, so the stale .json/.logd stay in changed. This lets the required diagnostic workflow pass for unbuilt code, defeating the new validation message that diagnostics must be regenerated after code changes.
Useful? React with 👍 / 👎.
1 participant
Summary
Closes #1 — Updates
clean_moduleinbuild.pyto properly report failed clean commands with useful diagnostic output rather than silently returningTrue.Changes
clean_modulenow captures theCompletedProcessreturned byrun_text_processand checksresult.returncodebefore returning.False: When the clean command exits with a non-zero status, the function returnsFalseand prints the module name, exit code, and any stdout/stderr output.TimeoutExpiredandFileNotFoundErrorare now caught separately with clear, module-specific failure messages, rather than being lumped into a genericExceptioncatch-all.CARGO_TERM_COLORfor the Rust backend) are now passed to clean commands, matching howbuild_modulealready works.True. Existing successful clean flow is unchanged for non-verbose mode.Validation
Diagnostic artifacts generated during the build are included.
Submitted for bounty #1