Skip to content

If the host_env feature is enabled, then enable the _ctypes module on the Android platform.#7402

Merged
youknowone merged 2 commits intoRustPython:mainfrom
EtherealPsyche:main
Mar 12, 2026
Merged

If the host_env feature is enabled, then enable the _ctypes module on the Android platform.#7402
youknowone merged 2 commits intoRustPython:mainfrom
EtherealPsyche:main

Conversation

@EtherealPsyche
Copy link
Contributor

@EtherealPsyche EtherealPsyche commented Mar 12, 2026

Summary by CodeRabbit

  • New Features

    • Added Android platform support.
  • Chores

    • Updated native library dependencies for expanded platform support.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

The pull request adds Android platform support to the RustPython VM by extending platform-specific target dependencies and module configurations to include the Android operating system alongside Linux, macOS, and Windows.

Changes

Cohort / File(s) Summary
Android Platform Support
crates/vm/Cargo.toml
Updated conditional target dependencies cfg condition to include Android, and added two new dependencies: libffi (with workspace = true and "system" features) and libloading version 0.9.
Android Module Support
crates/vm/src/stdlib/mod.rs
Expanded target_os gating for the _ctypes module to include Android alongside Linux, macOS, and Windows.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • youknowone

Poem

🐰 Android joins the VM's grand stage,
With libffi dancing, libloading on the page,
_ctypes spreads its wings so wide,
RustPython hops with mobile pride! 🚀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title directly describes the main change: enabling the _ctypes module on Android when host_env feature is enabled, which matches the file-level changes.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan for PR comments
  • Generate coding plan

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
Cargo.toml (1)

15-15: Consider propagating to rustpython-stdlib for consistency.

Other features like host_env (line 14) propagate to both rustpython-vm and rustpython-stdlib:

host_env = ["rustpython-vm/host_env", "rustpython-stdlib?/host_env"]

While ctypes only propagates to rustpython-vm. Since rustpython-stdlib/ctypes just forwards to rustpython-vm/ctypes, this works correctly, but for consistency you might consider:

ctypes = ["rustpython-vm/ctypes", "rustpython-stdlib?/ctypes"]

This is a minor consistency suggestion and not required for correctness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 15, Update the Cargo.toml feature entry for "ctypes" so
it also propagates to rustpython-stdlib like other features do; specifically
modify the "ctypes" feature definition (currently referencing
"rustpython-vm/ctypes") to include the rustpython-stdlib propagation (add
"rustpython-stdlib?/ctypes") so the feature list becomes
["rustpython-vm/ctypes", "rustpython-stdlib?/ctypes"] and maintains consistency
with other features such as "host_env".
crates/vm/Cargo.toml (1)

87-88: Potential redundancy with target-specific dependencies.

These optional dependencies overlap with the non-optional target-specific dependencies on lines 111-113:

[target.'cfg(all(any(target_os = "linux", target_os = "macos", target_os = "windows"), not(any(target_env = "musl", target_env = "sgx"))))'.dependencies]
libffi = { workspace = true, features = ["system"] }
libloading = "0.9"

On supported platforms (Linux/macOS/Windows, non-musl, non-SGX), libffi and libloading are always included regardless of the ctypes feature. The optional deps here are only meaningful for platforms outside that target condition where a user explicitly enables --features ctypes.

This design works but creates a subtle distinction: the ctypes feature is only necessary for "unsupported" platforms. Consider adding a comment to clarify this intent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/vm/Cargo.toml` around lines 87 - 88, The optional dependencies libffi
and libloading declared in the crate root (libffi = { workspace = true, features
= ["system"], optional = true } and libloading = { version = "0.9", optional =
true }) overlap with the non-optional target-specific dependencies under the
target.'cfg(... )'.dependencies block, so add a brief comment above the optional
deps explaining that they exist to enable the "ctypes" feature on unsupported
platforms (i.e., when the target cfg does not match Linux/macOS/Windows
non-musl/non-SGX) and that on supported platforms the target-specific section
will always supply these crates regardless of the ctypes feature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/vm/src/stdlib/mod.rs`:
- Around line 41-49: Document that enabling the "ctypes" feature bypasses the
platform guards around the mod _ctypes and therefore may try to build
dependencies (libffi, libloading) that are unsupported on musl and SGX targets;
update Cargo.toml (feature description for "ctypes") and/or the README to
explicitly state the platform constraints and the potential for compile errors
on musl/SGX when users pass --features ctypes, and mention that the conditional
cfg(any(feature = "ctypes", ...)) in mod _ctypes is the reason this opt-in can
override the target checks.

---

Nitpick comments:
In `@Cargo.toml`:
- Line 15: Update the Cargo.toml feature entry for "ctypes" so it also
propagates to rustpython-stdlib like other features do; specifically modify the
"ctypes" feature definition (currently referencing "rustpython-vm/ctypes") to
include the rustpython-stdlib propagation (add "rustpython-stdlib?/ctypes") so
the feature list becomes ["rustpython-vm/ctypes", "rustpython-stdlib?/ctypes"]
and maintains consistency with other features such as "host_env".

In `@crates/vm/Cargo.toml`:
- Around line 87-88: The optional dependencies libffi and libloading declared in
the crate root (libffi = { workspace = true, features = ["system"], optional =
true } and libloading = { version = "0.9", optional = true }) overlap with the
non-optional target-specific dependencies under the target.'cfg(...
)'.dependencies block, so add a brief comment above the optional deps explaining
that they exist to enable the "ctypes" feature on unsupported platforms (i.e.,
when the target cfg does not match Linux/macOS/Windows non-musl/non-SGX) and
that on supported platforms the target-specific section will always supply these
crates regardless of the ctypes feature.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 2641b466-5ad6-4df5-bee3-2485151d769b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f20619 and 2b06f50.

📒 Files selected for processing (4)
  • Cargo.toml
  • crates/stdlib/Cargo.toml
  • crates/vm/Cargo.toml
  • crates/vm/src/stdlib/mod.rs

Comment on lines 41 to 49
#[cfg(any(
feature = "ctypes",
all(
feature = "host_env",
any(target_os = "linux", target_os = "macos", target_os = "windows"),
not(any(target_env = "musl", target_env = "sgx"))
)
))]
mod _ctypes;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if _ctypes module or its submodules have any platform guards that would protect against musl/sgx
echo "=== Checking _ctypes module for platform guards ==="
rg -n "cfg.*musl|cfg.*sgx" crates/vm/src/stdlib/_ctypes/

echo ""
echo "=== Checking top-level imports in _ctypes files ==="
rg -n "^use (libffi|libloading)" crates/vm/src/stdlib/_ctypes/

Repository: RustPython/RustPython

Length of output: 454


🏁 Script executed:

#!/bin/bash
# Read the mod.rs file to check the cfg conditions
echo "=== Reading crates/vm/src/stdlib/mod.rs around lines 41-49 ==="
sed -n '35,55p' crates/vm/src/stdlib/mod.rs

echo ""
echo "=== Checking for second cfg block around lines 91-98 ==="
sed -n '85,105p' crates/vm/src/stdlib/mod.rs

echo ""
echo "=== Full view of cfg conditions for _ctypes ==="
rg -B2 -A8 'mod _ctypes' crates/vm/src/stdlib/mod.rs

Repository: RustPython/RustPython

Length of output: 1668


🏁 Script executed:

#!/bin/bash
# Check if _ctypes.rs has any top-level cfg guards
echo "=== Checking _ctypes.rs for top-level cfg ==="
head -20 crates/vm/src/stdlib/_ctypes.rs

echo ""
echo "=== Check if libffi and libloading are available on musl/sgx ==="
# Check if there are any feature flags or conditional compilation for these deps
rg -n "libffi|libloading" Cargo.toml | head -20

Repository: RustPython/RustPython

Length of output: 609


🏁 Script executed:

#!/bin/bash
# Check for documentation about ctypes feature
echo "=== Searching for ctypes in README or docs ==="
find . -name "README*" -o -name "Cargo.toml" -o -name "FEATURES*" | head -10 | xargs grep -l "ctypes" 2>/dev/null || echo "No direct ctypes documentation found"

echo ""
echo "=== Check Cargo.toml for ctypes feature definition ==="
rg -A5 '\[features\]' Cargo.toml | grep -A100 "ctypes"

Repository: RustPython/RustPython

Length of output: 294


🏁 Script executed:

#!/bin/bash
# Check if there are any comments in mod.rs explaining this design
echo "=== Searching for comments around the ctypes cfg condition ==="
sed -n '35,55p' crates/vm/src/stdlib/mod.rs | cat -n

echo ""
echo "=== Check if libloading/libffi have musl support ==="
rg -B3 -A3 "libffi|libloading" Cargo.toml

Repository: RustPython/RustPython

Length of output: 1108


🏁 Script executed:

#!/bin/bash
# Check if there are any issues or PRs mentioning musl/sgx and ctypes
echo "=== Searching for any musl-related comments in stdlib ==="
rg -n "musl" crates/vm/src/stdlib/ | head -20

echo ""
echo "=== Check if libffi/libloading have documented musl issues ==="
# Check the Cargo.lock or dependency versions for any indicators
rg -B5 -A5 "libffi.*5\|libloading" Cargo.lock 2>/dev/null | head -30 || echo "Cargo.lock check skipped"

Repository: RustPython/RustPython

Length of output: 1938


🏁 Script executed:

#!/bin/bash
# Check if there's any issue or discussion about ctypes and musl in git history or issues
echo "=== Check if there's context in _ctypes.rs about musl/sgx ==="
grep -n "musl\|sgx\|platform" crates/vm/src/stdlib/_ctypes.rs || echo "No platform mentions in _ctypes.rs"

echo ""
echo "=== Verify the exact behavior: what happens when ctypes feature is enabled on musl ==="
# Check if there's a CI configuration that tests this
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "ctypes\|musl" 2>/dev/null | head -5

Repository: RustPython/RustPython

Length of output: 396


🏁 Script executed:

#!/bin/bash
# Check CI configuration for ctypes testing
echo "=== Checking CI for ctypes feature tests ==="
grep -n "ctypes" .github/workflows/ci.yaml

echo ""
echo "=== Checking if musl builds are tested ==="
grep -n "musl" .github/workflows/ci.yaml | head -20

echo ""
echo "=== Check if ctypes and musl are tested together ==="
grep -B5 -A5 "ctypes" .github/workflows/ci.yaml | grep -A10 -B10 "musl"

Repository: RustPython/RustPython

Length of output: 282


🏁 Script executed:

#!/bin/bash
# Check if there's any documentation or indication of libffi/libloading support on musl
echo "=== Check Cargo.toml dependencies for any musl-related features ==="
grep -B2 -A2 "libffi\|libloading" Cargo.toml

echo ""
echo "=== Look for any conditional dependencies based on target_env ==="
sed -n '/^\[target\./,/^\[/p' Cargo.toml | grep -A5 "musl"

echo ""
echo "=== Check if the ctypes feature is explicitly documented anywhere ==="
find . -name "*.md" -type f | xargs grep -i "ctypes" 2>/dev/null | head -10

Repository: RustPython/RustPython

Length of output: 716


Document the platform constraints for the ctypes feature.

The feature = "ctypes" flag bypasses platform checks and allows enabling ctypes on musl and SGX targets. Since _ctypes and its submodules unconditionally depend on libffi and libloading (which are not available on these platforms), users who enable --features ctypes on musl or SGX will encounter compile-time errors. This appears intentional—to allow users to opt-in despite platform restrictions—but should be documented in Cargo.toml or the project README so users understand the platform requirements before attempting to enable this feature.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/mod.rs` around lines 41 - 49, Document that enabling the
"ctypes" feature bypasses the platform guards around the mod _ctypes and
therefore may try to build dependencies (libffi, libloading) that are
unsupported on musl and SGX targets; update Cargo.toml (feature description for
"ctypes") and/or the README to explicitly state the platform constraints and the
potential for compile errors on musl/SGX when users pass --features ctypes, and
mention that the conditional cfg(any(feature = "ctypes", ...)) in mod _ctypes is
the reason this opt-in can override the target checks.

Copy link
Contributor Author

@EtherealPsyche EtherealPsyche left a comment

Choose a reason for hiding this comment

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

libffi can actually be used on Android systems, but why is the _ctypes module only enabled when
cfg(all(any(target_os = "linux", target_os = "macos", target_os = "windows"),
not(any(target_env = "musl", target_env = "sgx")))) is satisfied?
For example, Termux provides a libffi (-static) package that can be used to link against a precompiled libffi.
This modification was originally done on my Android phone using Termux, but I accidentally lost the original changes.
Now I have recreated the changes on my computer from memory and created a PR, so there may be mistakes (previously it worked fine when tested on Termux).

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

I believe the ideal cfg for ctypes will be only cfg(feature = "host_env").
Could you please remove ctypes feature but add android (or termux) to the cfg condition? I believe it is not excluded by intend.

@youknowone youknowone requested a review from ShaharNaveh March 12, 2026 07:52
@ShaharNaveh
Copy link
Contributor

@youknowone should we start to officially support android as a platform? (like cpython does it from 3.13)

Copy link
Contributor

@ShaharNaveh ShaharNaveh left a comment

Choose a reason for hiding this comment

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

Just line @youknowone said, I don't see the benefit of having a feature of ctypes, but to gate it behind the host_env feature, unless we would want to go with a feature gate per module (which has it's pros&cons)

@youknowone
Copy link
Member

How do you define official and unofficial platform? If you mean enhancing CI to guarantee it works, I agree.

@ShaharNaveh
Copy link
Contributor

How do you define official and unofficial platform? If you mean enhancing CI to guarantee it works, I agree.

https://peps.python.org/pep-0738/


And sure, I'll try to implement a CI step that checks CI on android as well

@EtherealPsyche
Copy link
Contributor Author

Okay, let's simply add target_os = "android" to avoid introducing a new ctypes feature.

@EtherealPsyche EtherealPsyche changed the title Add feature ctypes to force enable _ctypes module If the host_env feature is enabled, then enable the _ctypes module on the Android platform. Mar 12, 2026
Copy link
Contributor

@ShaharNaveh ShaharNaveh left a comment

Choose a reason for hiding this comment

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

Looks great! tysm!

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

Thank you so much! welcome to RustPython project

@youknowone youknowone merged commit 0f61464 into RustPython:main Mar 12, 2026
15 checks passed
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.

3 participants