Integrate OSError creations into OSErrorBuilder by youknowone · Pull Request #6443 · RustPython/RustPython

21-68: Well-structured platform-specific error handling.

The implementation correctly handles platform-specific strerror logic with proper null pointer checks for the unsafe blocks. The #[allow(unused_mut)] annotation on line 57 is necessary since builder is only mutated on Windows.

---

70-74: Clean delegation to builder pattern.

The ToPyException implementation correctly delegates to the new ToOSErrorBuilder trait, maintaining a consistent error construction flow.

---

4332-4335: Imports updated appropriately for the new builder pattern.

---

4531-4531: Consistent error construction with filename context.

The OSErrorBuilder::with_filename usage correctly propagates the filename to the OSError, improving error messages for users.

---

4546-4560: Unified error handling across platform-specific branches.

The error construction is consistent across Windows and Unix code paths, using OSErrorBuilder::with_filename to include the filename in all error cases. This provides better error messages compared to the previous approach.

1196-1197: Re-exporting OSErrorBuilder/ToOSErrorBuilder from exceptions looks good

This makes the builder and trait conveniently available to the rest of the crate and matches how they’re consumed in other files.

2-15: Using OSErrorBuilder in new_os_subtype_error keeps behavior while centralizing construction

The new OSErrorBuilder::with_subtype(...).build(self) path preserves the previous semantics (including the basicsize debug‑assert on exc_type) while routing OS error creation through the shared builder. This looks correct for both errno: None (OSError(msg)) and Some(errno) flows.

Also applies to: 110-124

29-35: Consistent use of OSErrorBuilder::with_filename for POSIX filesystem errors

All these call sites now route std::io::Error (or errors converted into it) through OSErrorBuilder::with_filename, which:

• Preserves the original IO error’s errno and message via ToOSErrorBuilder.
• Attaches the relevant OsPath / OsPathOrFd as filename (and filename2 where applicable).
• Returns a PyBaseExceptionRef suitable for PyResult error types.

The patterns in access, _chmod, lchmod, rmdir, listdir, scandir, pathconf, truncate, and utime_impl are all coherent and should yield better CPython‑style OSError instances with proper filename context.

Also applies to: 415-417, 535-538, 1035-1036, 1097-1098, 360-363, 375-386, 861-871, 2130-2135, 1564-1571, 1339-1344, 1386-1403

153-168: Unified OSErrorBuilder usage across os filesystem operations

The new imports and map_err sites in this file consistently use:

• OSErrorBuilder::with_filename(&err, path_or_fd, vm) for IO errors that originate from a single pathname/FD.
• vm.new_errno_error(...) (now backed by OSErrorBuilder) when constructing errno‑driven errors.
• OSErrorBuilder::with_filename(&io::Error::from(Errno::last()), path, vm) for errno‑based pathconf failures.

This centralizes OS error creation and ensures that errno, strerror, and filename are wired the same way across open, remove/unlink, mkdir, rmdir, listdir (path branch), readlink, stat/fstat, chdir, truncate, utime, and pathconf. The patterns are type‑correct and align with how OSErrorBuilder/ToOSErrorBuilder are intended to be used.

Also applies to: 235-269, 290-323, 324-352, 359-363, 373-387, 546-553, 861-871, 1088-1092, 1118-1121, 1564-1571, 2130-2135

---

1128-1137: rename/replace and link now produce fully populated OSError with both filenames

The new error handling for rename/replace and link:

fs::rename(&src.path, &dst.path).map_err(|err| {
    let builder = err.to_os_error_builder(vm);
    let builder = builder.filename(src.filename(vm));
    let builder = builder.filename2(dst.filename(vm));
    builder.build(vm).upcast()
})

and similarly for link, ensures that:

• The underlying IO error is converted via to_os_error_builder, capturing errno and a platform‑appropriate message.
• filename and filename2 are set to src and dst (respecting OutputMode for str/bytes).
• The final Python exception is of the appropriate OSError subtype as determined by errno_to_exc_type.

This is a nice improvement over ad‑hoc construction and should match CPython’s behavior more closely.

Also applies to: 1221-1229

145-158: OSErrorBuilder::with_filename helper is well-factored and matches call sites

The inherent helper:

pub(crate) fn with_filename<'a>(
    error: &std::io::Error,
    filename: impl Into<OsPathOrFd<'a>>,
    vm: &VirtualMachine,
) -> crate::builtins::PyBaseExceptionRef {
    use crate::exceptions::ToOSErrorBuilder;
    let builder = error.to_os_error_builder(vm);
    let builder = builder.filename(filename.into().filename(vm));
    builder.build(vm).upcast()
}

nicely encapsulates the common “IO error + path → OSError with filename set” pattern used across os and posix. The API surface (taking any Into<OsPathOrFd<'a>>) keeps call sites clean and avoids duplicating the to_os_error_builder().filename(...).build(vm) sequence everywhere.

The TODO about returning PyRef<PyOSError> instead of PyBaseExceptionRef can be addressed later without affecting current users.