| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
📝 Walkthrough
WalkthroughBumps the libc dependency, reorganizes and expands per-target libc symbol imports and cfg gating in the POSIX module, and replaces the hardcoded const-generation script with a TOML-driven, multi-target extraction pipeline that fetches upstream docs and formats generated Rust. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant Script as scripts/libc_posix.py
participant Cargo as Cargo.toml
participant Remote as Upstream Docs
participant Rustfmt as rustfmt
participant Out as Generated Rust (posix.rs)
Dev->>Script: invoke generation
Script->>Cargo: read libc version from Cargo.toml
Script->>Remote: fetch source pages for each TARGET
Remote-->>Script: return HTML/text
Script->>Script: extract constants, group by Cfg/Target, attach extras
Script->>Rustfmt: format generated Rust code
Rustfmt-->>Script: return formatted code
Script->>Out: write cfg-tagged Rust blocks into posix.rs
Script-->>Dev: exit (success or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem🚥 Pre-merge checks | ✅ 2 | ❌ 1 ❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches 🧪 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin update-libc-0-2-182 |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify each finding against the current code and only fix it if needed. Inline comments: In `@crates/vm/src/stdlib/posix.rs`: - Around line 64-66: The cfg currently gating SCHED_DEADLINE and SCHED_NORMAL is too narrow (only Android); change the attribute on the use/import of SCHED_DEADLINE and SCHED_NORMAL in posix.rs from #[cfg(target_os = "android")] to #[cfg(any(target_os = "android", target_os = "linux"))] so these constants are available on Linux as well (match the pattern used for SCHED_BATCH/SCHED_IDLE/SCHED_RESET_ON_FORK); update the cfg on the import that references SCHED_DEADLINE and SCHED_NORMAL accordingly.
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between 123b629 and 5248d83.
⛔ Files ignored due to path filters (1)
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)scripts/libc_posix.py (1)🤖 Prompt for all review comments with AI agents20-20: Use explicit UTF-8 when reading Cargo.toml.
Line 20 currently relies on platform default encoding; specifying UTF-8 avoids locale-dependent failures.
Proposed fix🤖 Prompt for AI Agents-CARGO_TOML = tomllib.loads(CARGO_TOML_FILE.read_text()) +CARGO_TOML = tomllib.loads(CARGO_TOML_FILE.read_text(encoding="utf-8"))Verify each finding against the current code and only fix it if needed. In `@scripts/libc_posix.py` at line 20, The code reads Cargo.toml using CARGO_TOML_FILE.read_text() which uses the platform default encoding; change the read to explicitly use UTF-8 so parsing is deterministic by calling read_text with encoding="utf-8" (or open the file with encoding="utf-8" and pass the content to tomllib.loads) and assign the result back to CARGO_TOML; update the CARGO_TOML assignment that references CARGO_TOML_FILE to use the UTF-8-decoded text.
Verify each finding against the current code and only fix it if needed. Inline comments: In `@scripts/libc_posix.py`: - Around line 163-165: The return type annotation of consts_from_url is incorrect (currently "str") — update its annotation to match the actual returned type from extract_consts, e.g. change the function signature to return frozenset[str] (or typing.FrozenSet[str] if using typing imports) so it aligns with CONSTS_PATTERN, EXCLUDE, and the extract_consts(...) return value used in the implementation. --- Nitpick comments: In `@scripts/libc_posix.py`: - Line 20: The code reads Cargo.toml using CARGO_TOML_FILE.read_text() which uses the platform default encoding; change the read to explicitly use UTF-8 so parsing is deterministic by calling read_text with encoding="utf-8" (or open the file with encoding="utf-8" and pass the content to tomllib.loads) and assign the result back to CARGO_TOML; update the CARGO_TOML assignment that references CARGO_TOML_FILE to use the UTF-8-decoded text.
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between 5248d83 and 2979d36.
⛔ Files ignored due to path filters (1)
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)scripts/libc_posix.py (1)🤖 Prompt for all review comments with AI agents235-235: Make import rendering deterministic before formatting.
Line 235 joins a set directly. Sorting here avoids nondeterministic intermediate output and avoids relying on formatter behavior for stable ordering.
Proposed fix🤖 Prompt for AI Agents- imports = ",".join(consts) + imports = ",".join(sorted(consts))Verify each finding against the current code and only fix it if needed. In `@scripts/libc_posix.py` at line 235, The current code constructs imports by joining consts directly which can be nondeterministic if consts is a set; make the output deterministic by sorting before joining — replace the join over consts with a join over sorted(consts) so the variable imports is produced in stable, alphabetical order (referencing the consts collection and the imports variable).
Verify each finding against the current code and only fix it if needed. Inline comments: In `@scripts/libc_posix.py`: - Around line 179-184: The Python helper libc_posix.py calls urllib.request.urlopen(url) without a timeout; since Python code should remain unchanged, enforce a request timeout in the Rust caller that invokes this script: locate the Rust code that spawns or invokes libc_posix.py (the Command/process that passes the `url` to libc_posix.py) and wrap the child process execution with a timeout (e.g., use std::process::Child + wait_timeout crate or tokio::time::timeout for async) so the Python helper is killed if it hangs, return a clear error to the caller; ensure the timeout value is configurable or documented and that the child process is terminated (kill) on timeout and proper errors are propagated to the calling code. --- Nitpick comments: In `@scripts/libc_posix.py`: - Line 235: The current code constructs imports by joining consts directly which can be nondeterministic if consts is a set; make the output deterministic by sorting before joining — replace the join over consts with a join over sorted(consts) so the variable imports is produced in stable, alphabetical order (referencing the consts collection and the imports variable).
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between 2979d36 and 7fe87cf.
📒 Files selected for processing (1)
Sorry, something went wrong.
| try: | ||
| with urllib.request.urlopen(url) as f: | ||
| contents = f.read().decode() | ||
| except urllib.error.HTTPError as err: | ||
| err.add_note(url) | ||
| raise |
There was a problem hiding this comment.
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== urlopen callsites (verify timeout argument presence) =="
rg -n -C2 '\burlopen\s*\(' scripts/libc_posix.pyRepository: RustPython/RustPython
Length of output: 290
🏁 Script executed:
#!/bin/bash
# Check the exact content at lines 179-184
echo "== File content at lines 179-184 =="
sed -n '179,184p' scripts/libc_posix.pyRepository: RustPython/RustPython
Length of output: 279
🏁 Script executed:
#!/bin/bash
# Check context around the function
echo "== Function context (lines 170-200) =="
sed -n '170,200p' scripts/libc_posix.pyRepository: RustPython/RustPython
Length of output: 1097
Add request timeout or resolve via Rust code path.
Line 180 uses urllib.request.urlopen(url) without a timeout. An unresponsive connection could hang the generator indefinitely. However, the project guidelines specify that Python code should not be edited and bug fixes should be made through Rust code modifications. Either apply the timeout through Rust code instead, or clarify if this helper script is exempt from that guideline.
🤖 Prompt for AI AgentsVerify each finding against the current code and only fix it if needed. In `@scripts/libc_posix.py` around lines 179 - 184, The Python helper libc_posix.py calls urllib.request.urlopen(url) without a timeout; since Python code should remain unchanged, enforce a request timeout in the Rust caller that invokes this script: locate the Rust code that spawns or invokes libc_posix.py (the Command/process that passes the `url` to libc_posix.py) and wrap the child process execution with a timeout (e.g., use std::process::Child + wait_timeout crate or tokio::time::timeout for async) so the Python helper is killed if it hangs, return a clear error to the caller; ensure the timeout value is configurable or documented and that the child process is terminated (kill) on timeout and proper errors are propagated to the calling code.
Sorry, something went wrong.
There was a problem hiding this comment.
@coderabbitai The project guideline of not editing Python files is only applicable under Lib directory
Sorry, something went wrong.
There was a problem hiding this comment.
@ShaharNaveh, thank you for the clarification! You're absolutely right—I misapplied that guideline. Since scripts/libc_posix.py is outside the Lib directory, it can be edited directly.
Corrected suggestion: Add a timeout parameter to the urlopen call in the consts_from_url function to prevent indefinite hangs:
def consts_from_url(
url: str, *, pattern: re.Pattern = CONSTS_PATTERN, exclude: frozenset[str] = EXCLUDE
) -> frozenset[str]:
try:
with urllib.request.urlopen(url, timeout=30) as f: # Add timeout
contents = f.read().decode()
except urllib.error.HTTPError as err:
err.add_note(url)
raiseA 30-second timeout is usually reasonable for network requests, but you can adjust it based on your needs.
Learnt from: ShaharNaveh Repo: RustPython/RustPython PR: 7247 File: scripts/libc_posix.py:179-184 Timestamp: 2026-03-03T16:06:06.756Z Learning: Python files outside the `Lib` and `pylib` directories (such as files in `scripts/`) can be edited normally. The restriction on editing Python code only applies to the `Lib` directory (CPython standard library) and `pylib` (auto-generated).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-06T05:30:53.056Z Learning: Applies to **/*.py : In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications only
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-06T05:30:53.056Z Learning: Applies to **/*.py : In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications only
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T05:30:53.056Z
Learning: Applies to Lib/test/**/*.py : When modifying test files in Lib/test, add a `# TODO: RUSTPYTHON` comment, use `unittest.skip("TODO: RustPython <reason>")`, or `unittest.expectedFailure` with a `# TODO: RUSTPYTHON <reason>` comment
Learnt from: reactive-firewall Repo: RustPython/RustPython PR: 0 File: :0-0 Timestamp: 2026-01-19T20:10:58.864Z Learning: In the RustPython project's CI/CD workflows (PR `#6176`), bash scripts are preferred over Python or Rust for test orchestration and result aggregation to maintain hermetic testing—avoiding circular dependencies where Python tools would depend on the RustPython interpreter being tested, and avoiding visibility loss into Rust runtime issues when using Rust tooling.
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-06T05:30:53.056Z Learning: Applies to Lib/** : Do not edit the Lib/ directory directly except for copying files from CPython
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-06T05:30:53.056Z Learning: Applies to pylib/** : Do not modify the pylib/ directory directly - its contents are generated automatically
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-06T05:30:53.056Z Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files in the Lib/ directory
Learnt from: CR Repo: RustPython/RustPython PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-06T05:30:53.056Z Learning: Applies to **/*test*.py : NEVER modify test assertions, test logic, or test data in test files
Sorry, something went wrong.
* Update to libc 0.2.182, refactor update script * Fix typing
| Back | FazBrowse Home | New Git URL |
Summary by CodeRabbit