Skip to content

Give os.chmod a valid __text_signature__ and keyword arguments #8968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 2 commits into from
Oct 5, 2026
+67 −18
Merged

Give os.chmod a valid __text_signature__ and keyword arguments #8968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bf3c17
Order chmod's arguments the way CPython declares them
leehanjeong Oct 5, 2026
465e8dc
Reject a text signature that puts `/` after `*` or `**`
leehanjeong Oct 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion crates/vm/src/function/signature.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ struct St {
emitted: bool,
po_left: usize,
var_pos_seen: bool,
var_kw_seen: bool,
star_emitted: bool,
}

Expand All @@ -255,6 +256,7 @@ const fn write_signature_prefix(buf: &mut [u8], name: &str, args: &[SigArg]) ->
emitted: false,
po_left: count_po_args(args),
var_pos_seen: false,
var_kw_seen: false,
star_emitted: false,
},
args,
Expand Down Expand Up @@ -575,6 +577,12 @@ const fn write_one(buf: &mut [u8], mut st: St, param: Param, fallback: &str) ->
st = emit_named(buf, st, "", name, param.default);
st.po_left -= 1;
if st.po_left == 0 {
// Python rejects `/` after `*` or `**`. The Rust arguments put a
// keyword or variadic one before a positional one; reorder them.
assert!(
!st.star_emitted && !st.var_pos_seen && !st.var_kw_seen,
"positional-only parameter after `*` or `**`: reorder the Rust arguments"
);
st = emit_text(buf, st, "/");
}
st
Expand All @@ -591,7 +599,10 @@ const fn write_one(buf: &mut [u8], mut st: St, param: Param, fallback: &str) ->
st.var_pos_seen = true;
emit_named(buf, st, "*", name, param.default)
}
ParamKind::VarKeyword => emit_named(buf, st, "**", name, param.default),
ParamKind::VarKeyword => {
st.var_kw_seen = true;
emit_named(buf, st, "**", name, param.default)
}
}
}

Expand Down Expand Up @@ -631,6 +642,7 @@ mod tests {
emitted: false,
po_left: 0,
var_pos_seen: false,
var_kw_seen: false,
star_emitted: true,
},
Param {
Expand Down
62 changes: 45 additions & 17 deletions crates/vm/src/stdlib/posix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ pub mod module {
path: OsPath,
dir_fd: DirFd<'_, 0>,
mode: u32,
follow_symlinks: FollowSymlinks,
follow_symlinks: bool,
vm: &VirtualMachine,
) -> PyResult<()> {
let [] = dir_fd.0;
Expand All @@ -1195,7 +1195,7 @@ pub mod module {
not(target_os = "redox"),
not(any(target_os = "macos", target_os = "freebsd", target_os = "netbsd"))
))]
if !follow_symlinks.0 {
if !follow_symlinks {
let err_path = path.clone();
let c_path = path.into_cstring(vm)?;
return rustpython_host_env::posix::fchmodat(
Expand All @@ -1219,7 +1219,7 @@ pub mod module {
let err_path = path.clone();
let body = move || {
use std::os::unix::fs::PermissionsExt;
let meta = fs_metadata(&path, follow_symlinks.0)?;
let meta = fs_metadata(&path, follow_symlinks)?;
let mut permissions = meta.permissions();
permissions.set_mode(mode);
fs::set_permissions(&path, permissions)
Expand All @@ -1233,18 +1233,32 @@ pub mod module {
}

#[cfg(not(target_os = "redox"))]
#[pyfunction]
fn chmod(
path: OsPathOrFd<'_>,
dir_fd: DirFd<'_, 0>,
#[derive(FromArgs)]
struct ChmodArgs<'fd> {
#[pyarg(any)]
path: OsPathOrFd<'fd>,
#[pyarg(any)]
mode: u32,
follow_symlinks: FollowSymlinks,
vm: &VirtualMachine,
) -> PyResult<()> {
#[pyarg(flatten)]
dir_fd: DirFd<'fd, 0>,
// CPython writes the platform expression; on posix it is always true.
#[pyarg(named, default = true, py_default = "(os.name != 'nt')")]
follow_symlinks: bool,
}

#[cfg(not(target_os = "redox"))]
#[pyfunction]
fn chmod(args: ChmodArgs<'_>, vm: &VirtualMachine) -> PyResult<()> {
let ChmodArgs {
path,
mode,
dir_fd,
follow_symlinks,
} = args;
match path {
OsPathOrFd::Path(path) => {
#[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "netbsd",))]
if !follow_symlinks.0 && dir_fd == Default::default() {
if !follow_symlinks && dir_fd == Default::default() {
return lchmod(LchmodArgs { path, mode }, vm);
}
_chmod(path, dir_fd, mode, follow_symlinks, vm)
Expand All @@ -1254,14 +1268,28 @@ pub mod module {
}

#[cfg(target_os = "redox")]
#[pyfunction]
fn chmod(
#[derive(FromArgs)]
struct ChmodArgs {
#[pyarg(any)]
path: OsPath,
dir_fd: DirFd<0>,
#[pyarg(any)]
mode: u32,
follow_symlinks: FollowSymlinks,
vm: &VirtualMachine,
) -> PyResult<()> {
#[pyarg(flatten)]
dir_fd: DirFd<'static, 0>,
// CPython writes the platform expression; on posix it is always true.
#[pyarg(named, default = true, py_default = "(os.name != 'nt')")]
follow_symlinks: bool,

@youknowone youknowone Oct 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the rationale not to use FollowSymlinks anymore?

@leehanjeong leehanjeong Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it to match CPython's __text_signature__. In CPython only os.chmod and os.link show follow_symlinks=(os.name != 'nt'), and the others show True. FollowSymlinks always renders True, and a flattened field can't take py_default, so I used a plain bool, the same way LinkArgs already does for os.link.

Because of that expression, inspect.signature(os.chmod) raises ValueError, same as in CPython. If you prefer to keep FollowSymlinks and show True, I'll switch it back.

@youknowone youknowone Oct 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably we can tweak FollowSymlinks to show proper value. but it is good for now

}

#[cfg(target_os = "redox")]
#[pyfunction]
fn chmod(args: ChmodArgs, vm: &VirtualMachine) -> PyResult<()> {
let ChmodArgs {
path,
mode,
dir_fd,
follow_symlinks,
} = args;
_chmod(path, dir_fd, mode, follow_symlinks, vm)
}

Expand Down
9 changes: 9 additions & 0 deletions extra_tests/snippets/builtin_signature.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,12 @@
assert sum.__text_signature__ == "($module, iterable, /, start=0)"
assert str(inspect.signature(round)) == "(number, ndigits=None)"
assert str(inspect.signature(sum)) == "(iterable, /, start=0)"

# A keyword-only argument between two positional ones used to put `/` after
# `*`, which inspect cannot parse. Arguments are ordered like CPython's, so
# path and mode take keywords. Windows renders follow_symlinks differently.
if hasattr(os, "chmod"):
chmod_signature = os.chmod.__text_signature__
assert chmod_signature.startswith("($module, /, path, mode, *, dir_fd=None"), (
chmod_signature
)
Loading