GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Restore free-threaded C API import test with atomic module insertion by youknowdot · Pull Request #9498 · RustPython/RustPython Conversation
Contributor
Summary
The sole capability adapter uses existing Validation
LimitsOne bounded canonical concurrency test is not proof of all interleavings or complete CPython C API compatibility. Non-module/subclass/generic-mapping runtime corner cases were not exercised. Generic mappings remain unsupported. Exportless builds report a capability skip, not native validation; Windows export completeness is separate (#9270). Validated locally on Linux against main AI assistanceOpenAI Codex assisted with source investigation, implementation, updater migration, test orchestration, and independent agent review. The validation above was actually executed in the Linux environment; this does not claim human manual verification or validation on untested platforms.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsUse exact-dict conditional module insertion and non-module replacement, preserving dict-subclass mapping callbacks. Restore the canonical Python 3.15 test through an explicit generic new-file updater adaptation with source hash and protected AST checks. Assisted-by: OpenAI Codex:model version unavailable
| |||||||||||||||||||||||||||||||
There was an error while loading. Please reload this page.
|
Caution Review failedThe pull request is closed.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_free_threading (TODO: 1) dependencies: dependent tests: (no tests depend on free_threading) Legend:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Merging this PR will not alter performance
Comparing Footnotes
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c482764ae
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
|
|
||
| # TODO: RUSTPYTHON; executable C API exports are optional. | ||
| _PyImport_AddModuleRef = get_attribute(ctypes.pythonapi, "PyImport_AddModuleRef") |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Keep the canonical test's missing-export failure visible
When the executable does not export PyImport_AddModuleRef, get_attribute() converts the lookup's AttributeError into SkipTest during module import, bypassing the entire concurrency test. This is a test-logic change rather than an expected-failure/TODO marker edit permitted by AGENTS.md, and it can conceal an accidental export regression in a C-API-enabled build. Preserve the canonical binding and address optional export availability in the Rust/build configuration or test-selection setup instead.
AGENTS.md reference: AGENTS.md:L277-L282
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -0,0 +1,48 @@ | |||
| import ctypes | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Account for builds without ctypes before loading this module
On musl builds, this unconditional import raises ModuleNotFoundError before either the symbol probe or the threading decorator can run: Lib/ctypes/__init__.py imports _ctypes, but the VM explicitly excludes its registration on musl in crates/vm/src/stdlib/mod.rs:119-129. The free-threading package remains discoverable because RustPython advertises Py_GIL_DISABLED = 1, so restoring this file adds a test-collection error on that supported target. Dependency availability needs to be accounted for before loading this module, not only when looking up the C API symbol.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.