Skip to content

Commit cee8a5d

Browse files
committed
fix: allow Rust consumers to link datafusion-python
Gate PyO3 extension-module behind a default-on crate feature, retain it in wheel builds, and run a separate executable linking regression in CI. Generated-by: OpenAI Codex
1 parent b6c6f5b commit cee8a5d

7 files changed

Lines changed: 95 additions & 17 deletions

File tree

.github/actions/build-wheel/action.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,15 @@ runs:
6060
run: |
6161
set -euo pipefail
6262
FEATURES="${{ inputs.features }}"
63+
# All wheels need extension-module, including builds that disable defaults.
64+
FEATURES="extension-module${FEATURES:+,${FEATURES}}"
6365
TAG="${{ inputs.python-tag }}"
6466
if [ "$TAG" = "abi3" ]; then
6567
# Default features include the `abi3` cargo feature.
6668
# One wheel covers Python 3.10..3.14 (GIL builds only).
6769
BUILD_ARGS="--features ${FEATURES}"
6870
else
69-
# Free-threaded build: disable abi3, force mimalloc back in, pin interpreter.
71+
# Disable abi3, restore wheel features, and pin the free-threaded interpreter.
7072
if [ "${RUNNER_OS:-}" = "Windows" ]; then
7173
# Windows free-threaded builds ship as `python.exe` (no `tN`
7274
# suffix). Resolve sys.executable so the path is independent of

.github/workflows/build.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,33 @@ jobs:
5959
- name: Check formatting
6060
run: cargo +nightly fmt --all -- --check
6161

62+
test-rust:
63+
runs-on: ubuntu-latest
64+
steps:
65+
- uses: actions/checkout@v6
66+
67+
- name: Setup Rust
68+
uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4
69+
70+
- name: Setup Python
71+
uses: actions/setup-python@v5
72+
with:
73+
python-version: "3.12"
74+
75+
- name: Install Protoc
76+
uses: arduino/setup-protoc@v3
77+
with:
78+
version: "27.4"
79+
repo-token: ${{ secrets.GITHUB_TOKEN }}
80+
81+
- name: Cache Cargo
82+
uses: Swatinem/rust-cache@v2
83+
84+
# Select core explicitly: the FFI example workspace members enable
85+
# pyo3/extension-module, which prevents executables from linking libpython.
86+
- name: Run Rust tests, including the downstream linking smoke test
87+
run: cargo test --locked -p datafusion-python --no-default-features --features substrait
88+
6289
lint-python:
6390
runs-on: ubuntu-latest
6491
steps:

AGENTS.md

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,16 @@ Always prefer Python coverage — a doctest example in a docstring, or a pytest
8888
case. The user-facing Python surface is the first line of defense and the
8989
primary focus, so behavior should be pinned where users actually meet it.
9090

91-
**CI does not run Rust tests.** No workflow invokes `cargo test`; the only
92-
Rust checks are `cargo fmt --check` and
93-
`cargo clippy --no-deps --all-targets`. `--all-targets` compiles
94-
`#[cfg(test)]` code, so a Rust test cannot rot into a non-compiling state, but
95-
it is never executed and a behavioral regression will not fail the build. A
96-
Rust test added today is dead weight.
97-
98-
Adding a `cargo test` job is not a one-line change: `crates/core/Cargo.toml`
99-
enables `pyo3/extension-module` unconditionally, so the test binary fails to
100-
link against `Py_*` symbols on Linux. The feature would have to be gated first.
91+
**CI runs core Rust tests** with
92+
`cargo test --locked -p datafusion-python --no-default-features --features substrait`.
93+
Disabling the default `extension-module` feature allows test executables to
94+
link libpython. Select core explicitly rather than `--workspace`: the FFI
95+
example crates enable `pyo3/extension-module` through Cargo feature unification.
96+
The `rust_link` integration test consumes the rlib in a separate executable
97+
and executes Python bindings, guarding against unresolved `Py_*` symbols.
10198

10299
Write a Rust test only when the behavior is genuinely unreachable from Python,
103-
and wire up CI in the same change so it actually runs. Before concluding it is
100+
and ensure CI actually runs it. Before concluding it is
104101
unreachable, check the suites that already exist:
105102

106103
- `python/tests/` — the main suite. Run `pytest python/`, **not**

crates/core/Cargo.toml

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,7 @@ tokio = { workspace = true, features = [
4040
"rt-multi-thread",
4141
"sync",
4242
] }
43-
pyo3 = { workspace = true, features = [
44-
"extension-module",
45-
"generate-import-lib",
46-
] }
43+
pyo3 = { workspace = true, features = ["generate-import-lib"] }
4744
pyo3-async-runtimes = { workspace = true, features = ["tokio-runtime"] }
4845
pyo3-log = { workspace = true }
4946
chrono = { workspace = true }
@@ -76,7 +73,9 @@ prost-types = { workspace = true }
7673
pyo3-build-config = { workspace = true }
7774

7875
[features]
79-
default = ["mimalloc", "abi3"]
76+
default = ["mimalloc", "abi3", "extension-module"]
77+
# Disable for Rust tests and executables, which need to link libpython.
78+
extension-module = ["pyo3/extension-module"]
8079
# Stable ABI build — single wheel covers Python 3.10..3.14 (GIL builds only).
8180
# Mutually exclusive with free-threaded interpreters (cp313t / cp314t); the
8281
# free-threaded wheel build must pass --no-default-features.

crates/core/build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,6 @@
1616
// under the License.
1717

1818
fn main() {
19+
#[cfg(feature = "extension-module")]
1920
pyo3_build_config::add_extension_module_link_args();
2021
}

crates/core/tests/rust_link.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
use datafusion_python::context::PySessionContext;
19+
use pyo3::prelude::*;
20+
21+
// An integration test is a separate executable consuming the rlib. Exercise
22+
// Python calls as well as Rust construction so linking must resolve Py_* symbols.
23+
#[test]
24+
fn rust_consumer_can_execute_python_bindings() -> PyResult<()> {
25+
Python::initialize();
26+
Python::attach(|py| {
27+
let context = Bound::new(py, PySessionContext::new(None, None)?)?;
28+
let dataframe = context.call_method1("sql_with_options", ("SELECT 1 AS value",))?;
29+
let count = dataframe.call_method0("count")?.extract::<usize>()?;
30+
assert_eq!(count, 1);
31+
Ok(())
32+
})
33+
}

docs/source/contributor-guide/introduction.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,25 @@ maturin develop --uv
7070
python -m pytest
7171
```
7272

73+
### Rust tests and downstream Rust dependencies
74+
75+
Run the core Rust tests, including the executable linking smoke test, with:
76+
77+
```shell
78+
cargo test --locked -p datafusion-python --no-default-features --features substrait
79+
```
80+
81+
An installed Python interpreter and its development libraries are required.
82+
Set `PYO3_PYTHON` to select an interpreter if needed. Do not use `--workspace`
83+
for these tests: the FFI example crates enable `pyo3/extension-module`, which
84+
prevents linking libpython into an executable.
85+
86+
Rust executables depending on `datafusion-python` should set
87+
`default-features = false` and enable optional features such as `substrait`
88+
as needed. The default `extension-module` feature is for Python extension
89+
builds. Free-threaded wheel builds disable defaults to avoid `abi3` and must
90+
explicitly enable `extension-module` (and `mimalloc` to retain the default allocator).
91+
7392
## Running & Installing pre-commit hooks
7493

7594
arrow-datafusion-python takes advantage of [pre-commit](https://pre-commit.com/) to assist developers with code linting to help reduce the number of commits that ultimately fail in CI due to linter errors. Using the pre-commit hooks is optional for the developer but certainly helpful for keeping PRs clean and concise.

0 commit comments

Comments
 (0)