FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Clean up deprecations by lsahn-gh · Pull Request #8300 · RustPython/RustPython · GitHub

/ RustPython Public

Clean up deprecations - #8300

Merged
youknowone merged 2 commits into
RustPython:mainfrom
lsahn-gh:clean-up-deprecations
Jul 18, 2026
ConversationCommits (2)ChecksFiles changed
Merged

Clean up deprecations#8300
youknowone merged 2 commits into
RustPython:mainfrom
lsahn-gh:clean-up-deprecations

Conversation

lsahn-gh commented Jul 17, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

This PR has two commits that replace the deprecated methods in libc with new APIs or define a new const value due to compatibility.

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability when tracking function versions at runtime by adjusting how the internal version counter is atomically updated.
    • Improved reliability when updating collection-related type flags, including correct propagation through subclass hierarchies.
    • Ensured Android-specific resource limit constant availability by defining the exported constant explicitly for consistent cross-environment behavior.

coderabbitai Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: ae6f2e6f-f900-40e5-896c-e39f925fe3d7

📥 Commits

Reviewing files that changed from the base of the PR and between 50939de and 4925517.

📒 Files selected for processing (3)
  • crates/host_env/src/resource.rs
  • crates/vm/src/builtins/function.rs
  • crates/vm/src/builtins/type.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/host_env/src/resource.rs
  • crates/vm/src/builtins/function.rs
📝 Walkthrough

Walkthrough

The changes replace the deprecated Android RLIM_NLIMITS re-export and two VM fetch_update calls while preserving existing update and subclass-recursion behavior.

Changes

Deprecation cleanup

Layer / File(s) Summary
Android resource-limit constant
crates/host_env/src/resource.rs		 Android now defines RLIM_NLIMITS locally as a libc::c_int constant with value 16.
VM atomic update calls
crates/vm/src/builtins/function.rs, crates/vm/src/builtins/type.rs		 VM version and ABC collection-flag updates now use try_update instead of fetch_update, retaining existing update and propagation behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: youknowone, shaharnaveh

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the deprecation cleanup across libc and atomic APIs.
Linked Issues check ✅ Passed The PR updates the deprecated libc constant and replaces both fetch_update calls with try_update as requested.
Out of Scope Changes check ✅ Passed The changes stay focused on the deprecated constant and method replacements, with no obvious unrelated scope added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

lsahn-gh force-pushed the clean-up-deprecations branch 2 times, most recently from 592a6bd to 23d0d5e Compare July 17, 2026 04:51
coderabbitai Bot reviewed Jul 17, 2026
View reviewed changes

coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/host_env/src/resource.rs`:
- Around line 36-37: Update the Android-only RLIM_NLIMITS constant in
resource.rs to use libc::c_int, matching libc::RLIM_NLIMITS and the type
expected by crates/stdlib/src/resource.rs; leave its value and other platform
definitions unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 9f309701-c2aa-4bac-959c-2239e0587e44

📥 Commits

Reviewing files that changed from the base of the PR and between 1205fd2 and 820fb4a.

📒 Files selected for processing (3)
  • crates/host_env/src/resource.rs
  • crates/vm/src/builtins/function.rs
  • crates/vm/src/builtins/type.rs

Comment thread crates/host_env/src/resource.rs Outdated
lsahn-gh force-pushed the clean-up-deprecations branch from 23d0d5e to 50939de Compare July 17, 2026 05:07
lsahn-gh added 2 commits July 17, 2026 14:28
vm: replace fetch_update() with try_update()
5dbb880 
fetch_update() in atomic namespace is in the deprecation stage.
host_env: replace libc::RLIM_NLIMITS with const RLIM_NLIMITS
4925517 
Define a new const, RLIM_NLIMITS instead of libc::RLIM_NLIMITS in order
to clean up the deprecated const and to prevent breaking API where the const
is still used.
lsahn-gh force-pushed the clean-up-deprecations branch from 50939de to 4925517 Compare July 17, 2026 05:30
moreal added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 17, 2026
ShaharNaveh approved these changes Jul 18, 2026
View reviewed changes

ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

tysm!

moreal approved these changes Jul 18, 2026
View reviewed changes

moreal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you!

Refs

https://docs.rs/libc/latest/aarch64-linux-android/libc/constant.RLIM_NLIMITS.html

👎Deprecated since 0.2.64:
Not stable across OS versions

https://github.com/python/cpython/blob/323c59a5e348347be2ce2b7ea55fcb30bf68b2d3/Doc/data/python3.14.abi#L2217-L2218

https://doc.rust-lang.org/std/sync/atomic/type.AtomicU64.html#method.fetch_update

👎Deprecating in 1.99.0:
renamed to try_update for consistency

youknowone merged commit b838989 into RustPython:main Jul 18, 2026
26 checks passed
lsahn-gh deleted the clean-up-deprecations branch July 19, 2026 05:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

coderabbitai[bot] coderabbitai[bot] left review comments
moreal moreal approved these changes
ShaharNaveh ShaharNaveh approved these changes

Assignees

No one assigned

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix deprecated constant and method usage in host_env and vm crates.

4 participants

Back | FazBrowse Home | New Git URL