Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maturin strips PYO3_CROSS_LIB_DIR from the build command env #824

Closed
2 tasks done
ravenexp opened this issue Mar 1, 2022 · 10 comments · Fixed by #848
Closed
2 tasks done

maturin strips PYO3_CROSS_LIB_DIR from the build command env #824

ravenexp opened this issue Mar 1, 2022 · 10 comments · Fixed by #848
Labels
bug Something isn't working

Comments

@ravenexp
Copy link
Contributor

ravenexp commented Mar 1, 2022

Bug Description

When cross-compiling a Windows wheel by calling cargo build everything works correctly:

PYO3_CROSS_LIB_DIR=/x/python3-dll cargo build --release --target x86_64-pc-windows-gnu

When using maturin build as follows:

PYO3_CROSS_LIB_DIR=/x/python3-dll maturin build --release --strip --target x86_64-pc-windows-gnu

I get the following error:

  = note: /usr/lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld: cannot find -lpython3
          collect2: error: ld returned 1 exit status

💥 maturin failed
  Caused by: Failed to build a native library through cargo
  Caused by: Cargo build finished with "exit status: 101": `cargo rustc --message-format json --manifest-path Cargo.toml --target x86_64-pc-windows-gnu --release --lib -- -C link-arg=-s -L native=/x/python3-dll`

The error is occurring when linking one of the dependencies, not the target crate itself.
This dependency is both a PyO3 extension module and a Rust library, and cargo always generates both outputs.
Of these, only the Rust library is used to link the target extension module.

If I re-add the missing PYO3_CROSS_LIB_DIR variable the failing command succeeds:

PYO3_CROSS_LIB_DIR="$PWD"/target/python3-dll cargo rustc --message-format json --manifest-path Cargo.toml --target x86_64-pc-windows-gnu --release --lib -- -C link-arg=-s -L native=/home/kvachenok_sv/pyspock-legacy/target/python3-dll

Maybe maturin should just pass PYO3_CROSS_LIB_DIR to cargo rustc directly and not try to modify the library search path manually here:

maturin/src/compile.rs

Lines 179 to 194 in 8c849a1

let pythonxy_lib_folder;
if let BridgeModel::BindingsAbi3(_, _) = bindings_crate {
// NB: We set PYO3_NO_PYTHON further below.
// On linux, we can build a shared library without the python
// providing these symbols being present, on mac we can do it with
// the `-undefined dynamic_lookup` we use above anyway. On windows
// however, we get an exit code 0xc0000005 if we try the same with
// `/FORCE:UNDEFINED`, so we still look up the python interpreter
// and pass the location of the lib with the definitions.
if target.is_windows() {
let python_interpreter = python_interpreter
.expect("Must have a python interpreter for building abi3 on windows");
pythonxy_lib_folder = format!("native={}", python_interpreter.libs_dir.display());
rustc_args.extend(&["-L", &pythonxy_lib_folder]);
}
}

Your Python version (python -V)

Python 3.10.0

Your pip version (pip -V)

pip 21.0

What bindings you're using

pyo3

Does cargo build work?

  • Yes, it works

If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash /)?

  • Yes

Steps to Reproduce

  1. Try to cross-compile Windows a PyO3 extension crate that also has a PyO3 extension crate as a dependency.
  2. The dependency fails to build.
@ravenexp ravenexp added the bug Something isn't working label Mar 1, 2022
@messense
Copy link
Member

messense commented Mar 2, 2022

I don't remember maturin strips PYO3_CROSS_LIB_DIR, the code you referenced only added -L but didn't remove the env var I think?

Meanwhile I think you can workaround it by pass MATURIN_PYTHON_SYSCONFIGDATA_DIR env var instead though it shouldn't be relied on since it's an undocumented internal hack.

maturin/src/compile.rs

Lines 259 to 261 in 8c849a1

if let Some(lib_dir) = env::var_os("MATURIN_PYTHON_SYSCONFIGDATA_DIR") {
build_command.env("PYO3_CROSS_LIB_DIR", lib_dir);
}

@ravenexp
Copy link
Contributor Author

ravenexp commented Mar 2, 2022

I don't remember maturin strips PYO3_CROSS_LIB_DIR, the code you referenced only added -L but didn't remove the env var I think?

It doesn't strip any env vars per se, but builds the command environment from scratch.
I think PYO3_CROSS_LIB_DIR is not inherited and should be added explicitly.

Meanwhile I think you can workaround it by pass MATURIN_PYTHON_SYSCONFIGDATA_DIR env var instead though it shouldn't be relied on since it's an undocumented internal hack.

This doesn't work because setting MATURIN_PYTHON_SYSCONFIGDATA_DIR has some additional side-effects:

MATURIN_PYTHON_SYSCONFIGDATA_DIR="$PWD"/target/python3-dll maturin build --release --target x86_64-pc-windows-gnu

🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.7
💥 maturin failed
  Caused by: Failed to find a python interpreter

@messense
Copy link
Member

messense commented Mar 2, 2022

but builds the command environment from scratch.
I think PYO3_CROSS_LIB_DIR is not inherited and should be added explicitly.

I'm not sure about this, according to https://doc.rust-lang.org/std/process/struct.Command.html#method.new, it should inherit the current process’s environment.

@ravenexp
Copy link
Contributor Author

ravenexp commented Mar 2, 2022

That looks correct, but it doesn't seem to happen in practice. When invoked from the shell with PYO3_CROSS_LIB_DIR set, both cargo build and cargo rustc commands work, but when maturin invokes cargo rustc it fails.

Unfortunately I can't share the crate to reproduce this on.

@ravenexp
Copy link
Contributor Author

ravenexp commented Mar 2, 2022

Ok, by trial and error I've found a workaround:

PYO3_CROSS_LIB_DIR="$PWD"/target/python3-dll RUSTFLAGS="-L native=$PWD/target/python3-dll"  maturin build --release --strip --target x86_64-pc-windows-gnu

This command works, and my hypothesis is it works because of:

let mut rust_flags = env::var_os("RUSTFLAGS").unwrap_or_default();

.env("RUSTFLAGS", rust_flags)

here RUSTFLAGS is passed down explicitly, and not inherited.

@ravenexp
Copy link
Contributor Author

Ok, everything I've written above is wrong, except for the workaround, which works for a completely different reason.

The true cause of this issue is PYO3_NO_PYTHON environment variable use by maturin.
Setting this variable prevents PyO3 from generating correct linker flags for the PyO3 extension dependency. The linker flags maturin adds to the compile command apply to the current crate only, and are not passed down to its dependencies.

That's why setting RUSTFLAGS manually works, and the default maturin compile command doesn't.

This issue is solved by #847

@messense
Copy link
Member

I'm going to remove PYO3_NO_PYTHON only for Windows in #848, please give it try.

@ravenexp
Copy link
Contributor Author

#848 works as well.

I had to pass --no-sdist to maturin build, but it's a different issue I'll look into tomorrow.

@messense
Copy link
Member

@ravenexp Is it the same issue as #844 (comment) ? It was fixed in #844 by reverting #843.

@ravenexp
Copy link
Contributor Author

Yes, the error message looks the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants