Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 1, 2026

Summary by CodeRabbit

  • New Features

    • Added follow_symlinks parameter control to the link function for enhanced symlink handling.
  • Bug Fixes

    • Improved error propagation in warnings functionality—errors are now properly reported instead of silently ignored.
  • Chores

    • Refined internal error handling and resource management for improved stability.

✏️ Tip: You can customize this high-level summary in your review settings.

- Use linkat() with AT_SYMLINK_FOLLOW on Unix
- Raise NotImplementedError on non-Unix when follow_symlinks=False
- Register link in supports_follow_symlinks
- Replace manual exception creation with new_os_subtype_error in termios
- Remove redundant .to_owned() calls in os.link() error messages
- Prevent closing underlying fd when stdio wrappers are dropped
- Remove expectedFailure from test_fdopen in test_os.py
@youknowone youknowone marked this pull request as ready for review February 1, 2026 17:01
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

This PR updates error handling and OS-specific function behavior across multiple stdlib modules. Changes include refactoring termios error construction to use a structured pattern, adding follow_symlinks parameter support to os.link with platform-specific implementations, propagating warning errors instead of silencing them, and adjusting file descriptor closure behavior for standard IO streams.

Changes

Cohort / File(s) Summary
Error Construction Refactor
crates/stdlib/src/termios.rs
Replaces generic exception creation with structured os_subtype_error pattern, updating parameter arrangement to supply OS-specific errno separately from error message.
OS Link Enhancement
crates/vm/src/stdlib/os.rs
Adds follow_symlinks parameter to link function; implements platform-specific behavior using libc::linkat on Unix with flag control, and conditional NotImplementedError on non-Unix platforms when follow_symlinks is false. Updates support metadata to advertise conditional availability.
Error Propagation
crates/vm/src/stdlib/warnings.rs
Changes warning error handling from ignored (let _ =) to propagated (?), allowing failures to contribute to PyResult flow.
File Descriptor Management
crates/vm/src/vm/mod.rs
Adds closefd: false to OpenArgs for standard IO wrappers to prevent closing underlying file descriptors on drop.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 Errors now flow where once they were hushed,
Symlinks dance conditional, platform-blessed,
File descriptors cling to life as they should,
A refactored warren of errno so good! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly lists all four main changes made in the PR: posix.link enhancement, termios.error refactoring, warn() error propagation, and closefd=False for standard I/O.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone marked this pull request as draft February 1, 2026 17:01
@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2026

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] lib: cpython/Lib/os.py
[ ] test: cpython/Lib/test/test_os.py (TODO: 16)
[ ] test: cpython/Lib/test/test_popen.py

dependencies:

  • os (native: os.path, sys)
    • _collections_abc, abc, stat

dependent tests: (158 tests)

  • os: test___all__ test__osx_support test_argparse test_ast test_asyncio test_atexit test_base64 test_baseexception test_bdb test_bool test_builtin test_bytes test_bz2 test_c_locale_coercion test_calendar test_cmd_line test_cmd_line_script test_codecs test_compile test_compileall test_concurrent_futures test_configparser test_contextlib test_ctypes test_dbm test_dbm_dumb test_dbm_sqlite3 test_decimal test_devpoll test_doctest test_dtrace test_eintr test_email test_ensurepip test_enum test_epoll test_exception_hierarchy test_exceptions test_faulthandler test_fcntl test_file test_file_eintr test_filecmp test_fileinput test_fileio test_float test_fnmatch test_fork1 test_fractions test_fstring test_ftplib test_future_stmt test_genericalias test_genericpath test_getpass test_gettext test_glob test_graphlib test_gzip test_hash test_hashlib test_http_cookiejar test_httplib test_httpservers test_importlib test_inspect test_io test_ioctl test_json test_kqueue test_largefile test_linecache test_locale test_logging test_lzma test_mailbox test_marshal test_math test_mimetypes test_mmap test_msvcrt test_multiprocessing_fork test_multiprocessing_forkserver test_multiprocessing_main_handling test_multiprocessing_spawn test_ntpath test_openpty test_optparse test_os test_pathlib test_pkg test_pkgutil test_platform test_plistlib test_poll test_popen test_posix test_posixpath test_pty test_py_compile test_pyexpat test_random test_regrtest test_repl test_reprlib test_robotparser test_runpy test_script_helper test_selectors test_shelve test_shutil test_signal test_site test_smtpnet test_socket test_socketserver test_sqlite3 test_ssl test_stat test_string_literals test_structseq test_subprocess test_support test_sys test_sysconfig test_tabnanny test_tarfile test_tempfile test_termios test_thread test_threading test_tokenize test_tools test_trace test_tty test_typing test_unicode_file test_unicode_file_functions test_unittest test_univnewlines test_urllib test_urllib2 test_urllib2_localnet test_urllib2net test_urllibnet test_uuid test_venv test_wait3 test_wait4 test_webbrowser test_winapi test_winreg test_wsgiref test_xml_etree test_zipfile test_zipimport test_zoneinfo test_zstd

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone youknowone marked this pull request as ready for review February 1, 2026 23:05
@youknowone youknowone merged commit ab6114d into RustPython:main Feb 1, 2026
14 checks passed
@youknowone youknowone deleted the os-fix branch February 1, 2026 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant