Attempt to automate posix consts#6117
Conversation
Sorry, something went wrong.
WalkthroughAdds a new codegen script ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Script as scripts/libc_posix.py
participant Remote as libc-test (github.com/rust-lang/libc-test)
participant Formatter as format_groups
participant Output as Stdout
Dev->>Script: run script (main)
Script->>Remote: fetch wanted consts (OS docs) using OS_CONSTS_PAT
Script->>Remote: fetch per-OS constant lists (LIBC_VERSION, TARGET_OS)
Script->>Script: parse constants via CONSTS_PAT, apply EXCLUDE/RENAMES/EXTRAS
Script->>Script: build const -> OS-set map
Script->>Formatter: group by OS-set and format cfg-gated blocks
Formatter-->>Output: emit `#[cfg(...)]` + `use libc::{...}` + `#[pyattr]`
Output-->>Dev: printed code blocks for posix.rs consumption
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
vm/src/stdlib/posix.rs (4)
1646-1653: Won’t compile: libc::c_short::MAX does not exist; fix overflow check and cast
libc::c_shortis an alias (i16); it has noMAX. Usei16::MAXand cast safely.Apply this diff:
- if flags > libc::c_short::MAX as i32 { + if flags > i16::MAX as i32 || flags < i16::MIN as i32 { return Err(vm.new_value_error("Too many flags set for posix_spawn")); } - let ret = - unsafe { libc::posix_spawnattr_setflags(&mut attrp, flags as libc::c_short) }; + let ret = unsafe { + libc::posix_spawnattr_setflags(&mut attrp, flags as libc::c_short) + };
1517-1567: Leak: posix_spawn_file_actions_init resources are never destroyed on error pathsIf any
add*call fails, we return without callingposix_spawn_file_actions_destroy, leaking resources.Apply this diff inside the error branch:
- if ret != 0 { - let err = std::io::Error::from_raw_os_error(ret); - return Err(IOErrorBuilder::with_filename(&err, self.path, vm)); - } + if ret != 0 { + let err = std::io::Error::from_raw_os_error(ret); + unsafe { libc::posix_spawn_file_actions_destroy(&mut file_actions) }; + return Err(IOErrorBuilder::with_filename(&err, self.path, vm)); + }
1570-1656: Leak: posix_spawnattr_init/file_actions not destroyed on all exit pathsBoth
posix_spawnattr_initandposix_spawn_file_actions_initrequire correspondingdestroycalls. Add them for all paths afterattrpis initialized, including intermediate failures.Apply these diffs for the two intermediate error cases:
- if ret != 0 { - return Err(vm.new_os_error(format!("posix_spawnattr_setpgroup failed: {ret}"))); - } + if ret != 0 { + unsafe { + libc::posix_spawn_file_actions_destroy(&mut file_actions); + libc::posix_spawnattr_destroy(&mut attrp); + } + return Err(vm.new_os_error(format!("posix_spawnattr_setpgroup failed: {ret}"))); + }- if ret != 0 { - return Err( - vm.new_os_error(format!("posix_spawnattr_setsigmask failed: {ret}")) - ); - } + if ret != 0 { + unsafe { + libc::posix_spawn_file_actions_destroy(&mut file_actions); + libc::posix_spawnattr_destroy(&mut attrp); + } + return Err( + vm.new_os_error(format!("posix_spawnattr_setsigmask failed: {ret}")) + ); + }And ensure both are destroyed on the final return (success or error), see next comment.
1701-1706: Always destroy posix_spawn resources before returningEnsure no leaks on success and error returns.
Apply this diff:
- if ret == 0 { - Ok(pid) - } else { - let err = std::io::Error::from_raw_os_error(ret); - Err(IOErrorBuilder::with_filename(&err, self.path, vm)) - } + if ret == 0 { + unsafe { + libc::posix_spawn_file_actions_destroy(&mut file_actions); + libc::posix_spawnattr_destroy(&mut attrp); + } + Ok(pid) + } else { + let err = std::io::Error::from_raw_os_error(ret); + unsafe { + libc::posix_spawn_file_actions_destroy(&mut file_actions); + libc::posix_spawnattr_destroy(&mut attrp); + } + Err(IOErrorBuilder::with_filename(&err, self.path, vm)) + }
🧹 Nitpick comments (5)
scripts/libc_posix.py (4)
11-13: Tighten regex to avoid capturing functions like os.WIFEXITED(...)Use a negative lookahead to skip tokens immediately followed by '(' to reduce reliance on EXCLUDE for function-like names.
Apply this diff:
-OS_CONSTS_PAT = re.compile( - r"\bos\.(_*[A-Z]+(?:_+[A-Z]+)*_*)" -) # TODO: Exclude matches if they have `(` after (those are functions) +OS_CONSTS_PAT = re.compile( + r"\bos\.(_*[A-Z]+(?:_+[A-Z]+)*_*)(?!\s*\()" +) # Avoid function-like names: os.FOO(...)
68-73: Add timeouts and error handling to network fetches
urlopenwithout a timeout can hang CI; errors aren’t handled. Add a small timeout and surface helpful messages.Apply this diff and the import below:
-def get_consts(url: str, pattern: re.Pattern = CONSTS_PAT) -> frozenset[str]: - with urllib.request.urlopen(url) as f: - resp = f.read().decode() +def get_consts(url: str, pattern: re.Pattern = CONSTS_PAT) -> frozenset[str]: + try: + with urllib.request.urlopen(url, timeout=15) as f: + resp = f.read().decode() + except Exception as e: + raise RuntimeError(f"failed to fetch {url}: {e}") from eAlso add the import (either is fine; generic Exception above keeps it simple):
-import urllib.request +import urllib.request +import urllib.error
16-16: Avoid hard-coding libc version; derive or parameterizePinning
LIBC_VERSIONrisks drift vs. Cargo.lock. Read it from env/CLI, falling back to the constant.Apply this diff:
-LIBC_VERSION = "0.2.175" +import os +LIBC_VERSION = os.environ.get("LIBC_VERSION", "0.2.175")If helpful, I can wire a tiny argparse to allow
--libc-versiontoo.
94-97: Target CPython source rather than rendered docs for stabilityScraping docs is brittle across versions. Prefer parsing CPython’s posixmodule.c (or a pinned commit of os.rst) so the generator aligns with the project’s targeted CPython version.
Do you want me to adjust the script to accept a CPython ref (tag/commit) and pull constants from posixmodule.c?
vm/src/stdlib/posix.rs (1)
1605-1616: Avoid hard-coding POSIX_SPAWN_SETSID magic numberIf available, prefer
libc::POSIX_SPAWN_SETSIDto0x0080. If not defined, gate a local const behind#[cfg(target_os = "linux")].Apply this diff:
- #[cfg(target_os = "linux")] - { - flags |= 0x0080; // POSIX_SPAWN_SETSID value on Linux - } + #[cfg(target_os = "linux")] + { + #[allow(non_upper_case_globals)] + const POSIX_SPAWN_SETSID_LINUX: i32 = { + #[cfg(any())] { libc::POSIX_SPAWN_SETSID } // force compile-time fallback + #[cfg(not(any()))] { 0x0080 } + }; + flags |= POSIX_SPAWN_SETSID_LINUX; + }If
libc::POSIX_SPAWN_SETSIDexists on your minimum supported libc, you can simplify to that directly.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/libc_posix.py(1 hunks)vm/src/stdlib/posix.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Follow PEP 8 style for custom Python code
Use ruff for linting Python code
Files:
scripts/libc_posix.py
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
vm/src/stdlib/posix.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run tests under miri
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
🔇 Additional comments (2)
vm/src/stdlib/posix.rs (2)
50-56: Nice: cleanly grouped android-only base constantsThe android-gated set looks consistent and mirrors CPython exposure.
136-138: Verify SEEK_DATA/SEEK_HOLE availability on all targets
vm/src/stdlib/posix.rs:136–138
SEEK_DATAandSEEK_HOLEaren’t guaranteed in Android or DragonFly’s libc—run per-targetcargo check(e.g.aarch64-linux-android,x86_64-unknown-dragonfly) to confirm these imports build, and add#[cfg]guards if any fail.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
vm/src/stdlib/posix.rs (3)
1519-1523: Don’t use asserts for posix_spawn initializers; return proper OSErrorsAsserts will abort instead of surfacing OSError to Python. Handle init failures explicitly.
Apply:
let mut file_actions = unsafe { let mut fa = std::mem::MaybeUninit::uninit(); - assert!(libc::posix_spawn_file_actions_init(fa.as_mut_ptr()) == 0); + let r = libc::posix_spawn_file_actions_init(fa.as_mut_ptr()); + if r != 0 { + return Err(vm.new_os_error(format!("posix_spawn_file_actions_init failed: {r}"))); + } fa.assume_init() };let mut attrp = unsafe { let mut sa = std::mem::MaybeUninit::uninit(); - assert!(libc::posix_spawnattr_init(sa.as_mut_ptr()) == 0); + let r = libc::posix_spawnattr_init(sa.as_mut_ptr()); + if r != 0 { + // cleanup file_actions before returning + libc::posix_spawn_file_actions_destroy(&mut file_actions); + return Err(vm.new_os_error(format!("posix_spawnattr_init failed: {r}"))); + } sa.assume_init() };Also applies to: 1572-1576
1565-1569: Leak fix: always destroy posix_spawn file_actions/attr on all exit pathsposix_spawn_file_actions_init/posix_spawnattr_init allocate resources that must be destroyed. Several early returns skip destruction.
Apply:
- if ret != 0 { - let err = std::io::Error::from_raw_os_error(ret); - return Err(IOErrorBuilder::with_filename(&err, self.path, vm)); - } + if ret != 0 { + let err = std::io::Error::from_raw_os_error(ret); + unsafe { libc::posix_spawn_file_actions_destroy(&mut file_actions); } + return Err(IOErrorBuilder::with_filename(&err, self.path, vm)); + }- if ret != 0 { - return Err(vm.new_os_error(format!("posix_spawnattr_setflags failed: {ret}"))); - } + if ret != 0 { + unsafe { + libc::posix_spawn_file_actions_destroy(&mut file_actions); + libc::posix_spawnattr_destroy(&mut attrp); + } + return Err(vm.new_os_error(format!("posix_spawnattr_setflags failed: {ret}"))); + }- if ret == 0 { - Ok(pid) - } else { - let err = std::io::Error::from_raw_os_error(ret); - Err(IOErrorBuilder::with_filename(&err, self.path, vm)) - } + unsafe { + libc::posix_spawn_file_actions_destroy(&mut file_actions); + libc::posix_spawnattr_destroy(&mut attrp); + } + if ret == 0 { + Ok(pid) + } else { + let err = std::io::Error::from_raw_os_error(ret); + Err(IOErrorBuilder::with_filename(&err, self.path, vm)) + }Optional: wrap these in small RAII types with Drop to make this bulletproof and simplify control flow. I can draft that if you want.
Also applies to: 1653-1657, 1703-1709
1577-1591: Set POSIX_SPAWN_SETSIGDEF when setting the default signal setWithout the flag, the defaulted signal set may be ignored.
Apply:
assert!( unsafe { libc::posix_spawnattr_setsigdefault(&mut attrp, set.as_ref()) } == 0 ); + flags |= libc::POSIX_SPAWN_SETSIGDEF;
🧹 Nitpick comments (2)
vm/src/stdlib/posix.rs (2)
97-100: Verify NetBSD XATTR_*; likely Linux/macOS onlyNetBSD generally uses extattr(2) namespaces rather than XATTR_* macros. This import may not exist and could break builds.
Proposed tightening (if confirmed):
-#[cfg(any(target_os = "linux", target_os = "macos", target_os = "netbsd"))] +#[cfg(any(target_os = "linux", target_os = "macos"))] #[pyattr] use libc::{XATTR_CREATE, XATTR_REPLACE};
128-137: Android gating for SEEK_DATA/SEEK_HOLE may be riskySEEK_{DATA,HOLE} are not consistently available on Android bionic headers. Consider dropping Android from this block unless you’ve verified libc exposes them on your min API level.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/libc_posix.py(1 hunks)vm/src/stdlib/posix.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/libc_posix.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
vm/src/stdlib/posix.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run tests under miri
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run snippets and cpython tests (macos-latest)
🔇 Additional comments (2)
vm/src/stdlib/posix.rs (2)
101-109: Double-check GRND_ gating*GRND_RANDOM/NONBLOCK are certainly on Linux/Android; availability on DragonFly/NetBSD is not universal and may not be exposed by libc.
Would you like me to check libc’s per-target bindings and BSD manpages for these flags and propose exact cfgs?
1607-1619: Use libc::POSIX_SPAWN_SETSID instead of magic number
Confirm that yourlibccrate version definesPOSIX_SPAWN_SETSID(requires glibc ≥ 2.26 with_GNU_SOURCE); if it’s missing, bumplibcor gate out/implement a fallback. (spinics.net)
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
vm/src/stdlib/posix.rs (5)
50-53: Fix FreeBSD import: drop Linux-only MFD_HUGE_MASKMFD_HUGE_MASK isn’t defined on FreeBSD and will break that build. Keep only the SF_* flags here.
-#[cfg(target_os = "freebsd")] -#[pyattr] -use libc::{MFD_HUGE_MASK, SF_MNOWAIT, SF_NOCACHE, SF_NODISKIO, SF_SYNC}; +#[cfg(target_os = "freebsd")] +#[pyattr] +use libc::{SF_MNOWAIT, SF_NOCACHE, SF_NODISKIO, SF_SYNC};
87-93: Split memfd flags from POSIX_FADV_*; avoid FreeBSD/Android build breaksMFD_* are Linux/Android-only. POSIX_FADV_* are also available on FreeBSD. Split the groups.
-#[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux"))] -#[pyattr] -use libc::{ - MFD_ALLOW_SEALING, MFD_CLOEXEC, MFD_HUGETLB, POSIX_FADV_DONTNEED, POSIX_FADV_NOREUSE, - POSIX_FADV_NORMAL, POSIX_FADV_RANDOM, POSIX_FADV_SEQUENTIAL, POSIX_FADV_WILLNEED, -}; +// memfd_* flags: Linux/Android only +#[cfg(any(target_os = "android", target_os = "linux"))] +#[pyattr] +use libc::{MFD_ALLOW_SEALING, MFD_CLOEXEC, MFD_HUGETLB}; + +// posix_fadvise advice values (Android/FreeBSD/Linux) +#[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux"))] +#[pyattr] +use libc::{ + POSIX_FADV_DONTNEED, POSIX_FADV_NOREUSE, POSIX_FADV_NORMAL, POSIX_FADV_RANDOM, + POSIX_FADV_SEQUENTIAL, POSIX_FADV_WILLNEED, +};
94-97: Restrict TFD_ (timerfd) to Linux/Android*NetBSD doesn’t provide timerfd.
-#[cfg(any(target_os = "android", target_os = "linux", target_os = "netbsd"))] +#[cfg(any(target_os = "android", target_os = "linux"))] #[pyattr] use libc::{TFD_CLOEXEC, TFD_NONBLOCK, TFD_TIMER_ABSTIME, TFD_TIMER_CANCEL_ON_SET};
111-118: O_PATH is Linux-specific (Android may also expose); remove FreeBSD/RedoxThis will fail on FreeBSD/Redox.
-#[cfg(any( - target_os = "android", - target_os = "freebsd", - target_os = "linux", - target_os = "redox" -))] +#[cfg(any(target_os = "android", target_os = "linux"))] #[pyattr] use libc::O_PATH;
120-127: Restrict O_RSYNC to LinuxO_RSYNC isn’t generally exposed on BSDs; Android’s support is unclear.
-#[cfg(any( - target_os = "android", - target_os = "linux", - target_os = "netbsd", - target_os = "openbsd" -))] -#[pyattr] -use libc::O_RSYNC; +#[cfg(target_os = "linux")] +#[pyattr] +use libc::O_RSYNC;
🧹 Nitpick comments (3)
vm/src/stdlib/posix.rs (3)
75-82: Nit: simplify cfg to unixany(target_os = "android", unix) is redundant; Android is already unix.
-#[cfg(any(target_os = "android", unix))] +#[cfg(unix)] #[pyattr] use libc::{ F_OK, O_CLOEXEC, O_DIRECTORY, O_NOFOLLOW, O_NONBLOCK, PRIO_PGRP, PRIO_PROCESS, PRIO_USER, R_OK, RTLD_GLOBAL, RTLD_LAZY, RTLD_LOCAL, RTLD_NOW, W_OK, WCONTINUED, WNOHANG, WUNTRACED, X_OK, };
193-208: Verify CLD_ and F_LOCK family coverage; consider dropping redox*Some CLD_* and F_LOCK/TLOCK/ULOCK may not exist on Redox; broad gating risks build breaks.
-#[cfg(any( +#[cfg(any( target_os = "android", target_os = "dragonfly", target_os = "freebsd", target_os = "linux", target_os = "macos", target_os = "netbsd", - target_os = "openbsd", - target_os = "redox" + target_os = "openbsd" ))]
209-221: Verify O_ASYNC/WEXITED/WNOWAIT/WSTOPPED on RedoxThese waitid/async constants may be missing on Redox; suggest excluding if CI fails.
-#[cfg(any( +#[cfg(any( target_os = "android", target_os = "dragonfly", target_os = "freebsd", target_os = "linux", target_os = "macos", target_os = "netbsd", - target_os = "openbsd", - target_os = "redox" + target_os = "openbsd" ))]
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/libc_posix.py(1 hunks)vm/src/stdlib/posix.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/libc_posix.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
vm/src/stdlib/posix.rs
🧠 Learnings (1)
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Applied to files:
vm/src/stdlib/posix.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Run tests under miri
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (ubuntu-latest)
🔇 Additional comments (11)
vm/src/stdlib/posix.rs (11)
54-57: LGTM: PIDFD_NONBLOCK gating is correct (Linux-only).
58-64: LGTM: Darwin-only constants and aliasing to _COPYFILE_DATA look good.
83-86: LGTM: O_SYMLINK correctly gated to macOS/redox.
102-110: LGTM: GRND_ coverage looks reasonable across the listed Unixes.*
149-159: LGTM: RTLD_NOLOAD gating looks fine.
160-169: LGTM: O_DSYNC across listed Unixes is reasonable.
171-181: LGTM: SCHED_OTHER exposure across these targets is fine.
139-147: Remove macOS from O_DIRECT (not a Darwin open(2) flag)macOS uses F_NOCACHE via fcntl; O_DIRECT isn’t available there.
-#[cfg(any( - target_os = "android", - target_os = "dragonfly", - target_os = "freebsd", - target_os = "linux", - target_os = "netbsd" -))] +#[cfg(any( + target_os = "android", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" +))] #[pyattr] use libc::O_DIRECT;Likely an incorrect or invalid review comment.
182-192: Keep Redox in the cfg since O_EXLOCK and O_SHLOCK are defined in Redox’s libc.Likely an incorrect or invalid review comment.
98-101: Retain NetBSD in cfg NetBSD’s Linux-compatiblesys/xattr.hdefinesXATTR_CREATEandXATTR_REPLACE, so the original#[cfg(any(target_os = "linux", target_os = "macos", target_os = "netbsd"))]is correct.Likely an incorrect or invalid review comment.
129-137: Keep the single import; all three flags are supported on macOS and Android
macOS’s<dlfcn.h>defines RTLD_NODELETE and its headers (and Python itself) expose SEEK_DATA/SEEK_HOLE (opensource.apple.com, docs.python.org). Android’s Bionic<dlfcn.h>defines RTLD_NODELETE and its<unistd.h>pulls inbits/seek_constants.hfor SEEK_DATA/SEEK_HOLE (android.googlesource.com). The existing#[cfg(any(target_os = "android", "dragonfly", "freebsd", "linux", "macos"))] use libc::{RTLD_NODELETE, SEEK_DATA, SEEK_HOLE};is correct.Likely an incorrect or invalid review comment.
Sorry, something went wrong.
I can say it is arbitrary based on history of the module. Currently |
Sorry, something went wrong.
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
👍
Sorry, something went wrong.
youknowone
merged commit 056795e
into
RustPython:main
Sep 3, 2025
Reviewers
2 participants
Attempt to automate the definition of posix consts. I'm sure that this will require the script to be more adjustable but IMO this is a good first step in automating something that we do manually. for example: #5795
I ain't so sure how we decide what goes into
osdirectly and what intoposix, if someone can share some knowledge it would be very appreciated:)Summary by CodeRabbit