Skip to content

Commit 3126762

Browse files
leehanjeongleehanjeong
authored
Give os.chmod a valid __text_signature__ and keyword arguments (#8968)
* Order chmod's arguments the way CPython declares them posix.chmod took dir_fd between path and mode. Since #8807 renders each argument's parameter kind, that put `/` after `*`, which inspect cannot parse, and path and mode stayed positional-only, so os.chmod(path=..., mode=...) raised TypeError. A ChmodArgs struct follows CPython's order, as nt.chmod already does. path and mode are positional-or-keyword. follow_symlinks shows CPython's text through py_default, like LinkArgs. Assisted-by: Claude Code:claude-opus-5 * Reject a text signature that puts `/` after `*` or `**` The writer counts positional-only parameters up front and emits `/` when they run out, which assumes they come first in the Rust argument list. Nothing enforced that, and posix.chmod violated it. The signature is composed in a const context, so the assert fails the build and names the fix. Assisted-by: Claude Code:claude-opus-5
1 parent 52581b1 commit 3126762

3 files changed

Lines changed: 67 additions & 18 deletions

File tree

‎crates/vm/src/function/signature.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ struct St {
242242
emitted: bool,
243243
po_left: usize,
244244
var_pos_seen: bool,
245+
var_kw_seen: bool,
245246
star_emitted: bool,
246247
}
247248

@@ -255,6 +256,7 @@ const fn write_signature_prefix(buf: &mut [u8], name: &str, args: &[SigArg]) ->
255256
emitted: false,
256257
po_left: count_po_args(args),
257258
var_pos_seen: false,
259+
var_kw_seen: false,
258260
star_emitted: false,
259261
},
260262
args,
@@ -575,6 +577,12 @@ const fn write_one(buf: &mut [u8], mut st: St, param: Param, fallback: &str) ->
575577
st = emit_named(buf, st, "", name, param.default);
576578
st.po_left -= 1;
577579
if st.po_left == 0 {
580+
// Python rejects `/` after `*` or `**`. The Rust arguments put a
581+
// keyword or variadic one before a positional one; reorder them.
582+
assert!(
583+
!st.star_emitted && !st.var_pos_seen && !st.var_kw_seen,
584+
"positional-only parameter after `*` or `**`: reorder the Rust arguments"
585+
);
578586
st = emit_text(buf, st, "/");
579587
}
580588
st
@@ -591,7 +599,10 @@ const fn write_one(buf: &mut [u8], mut st: St, param: Param, fallback: &str) ->
591599
st.var_pos_seen = true;
592600
emit_named(buf, st, "*", name, param.default)
593601
}
594-
ParamKind::VarKeyword => emit_named(buf, st, "**", name, param.default),
602+
ParamKind::VarKeyword => {
603+
st.var_kw_seen = true;
604+
emit_named(buf, st, "**", name, param.default)
605+
}
595606
}
596607
}
597608

@@ -631,6 +642,7 @@ mod tests {
631642
emitted: false,
632643
po_left: 0,
633644
var_pos_seen: false,
645+
var_kw_seen: false,
634646
star_emitted: true,
635647
},
636648
Param {

‎crates/vm/src/stdlib/posix.rs‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,7 @@ pub mod module {
11861186
path: OsPath,
11871187
dir_fd: DirFd<'_, 0>,
11881188
mode: u32,
1189-
follow_symlinks: FollowSymlinks,
1189+
follow_symlinks: bool,
11901190
vm: &VirtualMachine,
11911191
) -> PyResult<()> {
11921192
let [] = dir_fd.0;
@@ -1195,7 +1195,7 @@ pub mod module {
11951195
not(target_os = "redox"),
11961196
not(any(target_os = "macos", target_os = "freebsd", target_os = "netbsd"))
11971197
))]
1198-
if !follow_symlinks.0 {
1198+
if !follow_symlinks {
11991199
let err_path = path.clone();
12001200
let c_path = path.into_cstring(vm)?;
12011201
return rustpython_host_env::posix::fchmodat(
@@ -1219,7 +1219,7 @@ pub mod module {
12191219
let err_path = path.clone();
12201220
let body = move || {
12211221
use std::os::unix::fs::PermissionsExt;
1222-
let meta = fs_metadata(&path, follow_symlinks.0)?;
1222+
let meta = fs_metadata(&path, follow_symlinks)?;
12231223
let mut permissions = meta.permissions();
12241224
permissions.set_mode(mode);
12251225
fs::set_permissions(&path, permissions)
@@ -1233,18 +1233,32 @@ pub mod module {
12331233
}
12341234

12351235
#[cfg(not(target_os = "redox"))]
1236-
#[pyfunction]
1237-
fn chmod(
1238-
path: OsPathOrFd<'_>,
1239-
dir_fd: DirFd<'_, 0>,
1236+
#[derive(FromArgs)]
1237+
struct ChmodArgs<'fd> {
1238+
#[pyarg(any)]
1239+
path: OsPathOrFd<'fd>,
1240+
#[pyarg(any)]
12401241
mode: u32,
1241-
follow_symlinks: FollowSymlinks,
1242-
vm: &VirtualMachine,
1243-
) -> PyResult<()> {
1242+
#[pyarg(flatten)]
1243+
dir_fd: DirFd<'fd, 0>,
1244+
// CPython writes the platform expression; on posix it is always true.
1245+
#[pyarg(named, default = true, py_default = "(os.name != 'nt')")]
1246+
follow_symlinks: bool,
1247+
}
1248+
1249+
#[cfg(not(target_os = "redox"))]
1250+
#[pyfunction]
1251+
fn chmod(args: ChmodArgs<'_>, vm: &VirtualMachine) -> PyResult<()> {
1252+
let ChmodArgs {
1253+
path,
1254+
mode,
1255+
dir_fd,
1256+
follow_symlinks,
1257+
} = args;
12441258
match path {
12451259
OsPathOrFd::Path(path) => {
12461260
#[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "netbsd",))]
1247-
if !follow_symlinks.0 && dir_fd == Default::default() {
1261+
if !follow_symlinks && dir_fd == Default::default() {
12481262
return lchmod(LchmodArgs { path, mode }, vm);
12491263
}
12501264
_chmod(path, dir_fd, mode, follow_symlinks, vm)
@@ -1254,14 +1268,28 @@ pub mod module {
12541268
}
12551269

12561270
#[cfg(target_os = "redox")]
1257-
#[pyfunction]
1258-
fn chmod(
1271+
#[derive(FromArgs)]
1272+
struct ChmodArgs {
1273+
#[pyarg(any)]
12591274
path: OsPath,
1260-
dir_fd: DirFd<0>,
1275+
#[pyarg(any)]
12611276
mode: u32,
1262-
follow_symlinks: FollowSymlinks,
1263-
vm: &VirtualMachine,
1264-
) -> PyResult<()> {
1277+
#[pyarg(flatten)]
1278+
dir_fd: DirFd<'static, 0>,
1279+
// CPython writes the platform expression; on posix it is always true.
1280+
#[pyarg(named, default = true, py_default = "(os.name != 'nt')")]
1281+
follow_symlinks: bool,
1282+
}
1283+
1284+
#[cfg(target_os = "redox")]
1285+
#[pyfunction]
1286+
fn chmod(args: ChmodArgs, vm: &VirtualMachine) -> PyResult<()> {
1287+
let ChmodArgs {
1288+
path,
1289+
mode,
1290+
dir_fd,
1291+
follow_symlinks,
1292+
} = args;
12651293
_chmod(path, dir_fd, mode, follow_symlinks, vm)
12661294
}
12671295

‎extra_tests/snippets/builtin_signature.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,12 @@
6464
assert sum.__text_signature__ == "($module, iterable, /, start=0)"
6565
assert str(inspect.signature(round)) == "(number, ndigits=None)"
6666
assert str(inspect.signature(sum)) == "(iterable, /, start=0)"
67+
68+
# A keyword-only argument between two positional ones used to put `/` after
69+
# `*`, which inspect cannot parse. Arguments are ordered like CPython's, so
70+
# path and mode take keywords. Windows renders follow_symlinks differently.
71+
if hasattr(os, "chmod"):
72+
chmod_signature = os.chmod.__text_signature__
73+
assert chmod_signature.startswith("($module, /, path, mode, *, dir_fd=None"), (
74+
chmod_signature
75+
)

0 commit comments

Comments
 (0)