indygreg / PyOxidizer

A modern Python application packaging and distribution tool
Mozilla Public License 2.0
5.4k stars 234 forks source link

Regression with bundling `cryptography` between 0.22.0 and 0.23.0 #663

Closed tsibley closed 1 year ago

tsibley commented 1 year ago

I recently encountered issues with PyOxidizer when bundling cryptography and tracked it down (seemingly) to a difference between PyOxidizer 0.23.0 (bad) and 0.22.0 (good). With 0.23.0, builds still succeed but at runtime when cryptography.hazmat.bindings._rust is imported the following failure happens:

ImportError: […]/lib/cryptography/hazmat/bindings/_rust.abi3.so: undefined symbol: PyExc_BaseException

This does not happen with builds produced by 0.22.0.

Reviewing the release notes for 0.23.0, I thought perhaps it was the default Python version change (3.10.4 → 3.10.8), but using that version on 0.22.0 as well doesn't cause the error to arise. The error is also seemingly independent of the cryptography version itself (I tried several older versions).

There are a few other changes in the 0.23.0 release notes that seem to me like they could be culpable, but I don't know enough to immediately test them myself.

Reproduction

I reduced my actual usage to the following test case:

def make_exe():
    # Not using default_python_distribution() to demonstrate that the issue
    # isn't related to the default version change (3.10.4 → 3.10.8) between
    # 0.22.0 and 0.23.0.
    python_dist = PythonDistribution(
        url="https://github.com/indygreg/python-build-standalone/releases/download/20221106/cpython-3.10.8%2B20221106-x86_64-unknown-linux-gnu-pgo-full.tar.zst",
        sha256="c75e0db9988cf3bd7b07a1172aea099354ae3b2bc5ded3eb97458286716d3175",
        flavor="standalone")

    packaging_policy = python_dist.make_python_packaging_policy()
    packaging_policy.resources_location = "in-memory"
    packaging_policy.resources_location_fallback = "filesystem-relative:lib"

    python_config = python_dist.make_python_interpreter_config()

    exe = python_dist.to_python_executable(
        name = "python",
        packaging_policy = packaging_policy,
        config = python_config)

    # version doesn't seem to matter, but pinning so it's repeatable over time
    exe.add_python_resources(exe.pip_install(["cryptography==38.0.3"]))

    return exe

def make_installation(exe):
    files = FileManifest()
    files.add_python_resource(".", exe)
    return files

register_target("exe", make_exe)
register_target("installation", make_installation, depends=["exe"], default=True)
resolve_targets()

When I pyoxidizer build with 0.23.0, I get a ./build/x86_64-unknown-linux-gnu/debug/installation/python binary that exhibits the error:

$ ./build/x86_64-unknown-linux-gnu/debug/installation/python <<<'import cryptography.hazmat.bindings._rust'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: [...]/build/x86_64-unknown-linux-gnu/debug/installation/lib/cryptography/hazmat/bindings/_rust.abi3.so: undefined symbol: PyExc_BaseException

When I build with 0.22.0, the same command exits without error.

The resulting build tree looks like this on both versions:

$ tree build/
build/
└── x86_64-unknown-linux-gnu
    └── debug
        └── installation
            ├── COPYING.txt
            ├── lib
            │   ├── _cffi_backend.cpython-310-x86_64-linux-gnu.so
            │   └── cryptography
            │       └── hazmat
            │           └── bindings
            │               ├── _openssl.abi3.so
            │               └── _rust.abi3.so
            └── python

7 directories, 5 files

I noted I had to pyoxidizer cache-clear between builds on different versions to make sure nothing was shared.

indygreg commented 1 year ago

This feels like fallout from changes like https://github.com/indygreg/PyOxidizer/commit/798a09afe6b385515410a927901066b8a001e852.

The linker arguments involved in this commit (and some that came minutes before it) are related to which symbols are exported from binaries. It certainly looks like we aren't exporting CPython public symbols from binaries any more, therefore preventing extension modules / shared libraries referencing these symbols from loading into the process without symbol resolution errors.

There should have been no change in behavior as a result of the refactors.

Can you please paste the full output of pyoxidizer build --verbose? (I only really care about the linker invocations, as we should see -export-dynamic in there. If we aren't then there was an unexpected change in behavior causing required linker argument to not be present.

tsibley commented 1 year ago

pyoxidizer build --verbose build log from 0.23.0, which does indeed lack the string -export-dynamic. But the same log from 0.22.0 ~(well, actually with 3 --verbose options)~ also lacks that string...

tsibley commented 1 year ago

When I stop a build mid-way and poke around the dirs its using in /tmp, I do find /tmp/pyoxidizermNVqk8/artifacts/pyo3-build-config-file.txt, which contains the following line:

extra_build_script_line=cargo:rustc-link-arg=-Wl,-export-dynamic
pquentin commented 1 year ago

I'm facing the same issue with psutil. Is there something useful I can do to help? Like testing before and after https://github.com/indygreg/PyOxidizer/commit/798a09afe6b385515410a927901066b8a001e852 or coming up with a minimal Starlark file.

dae commented 1 year ago

FWIW, I use a custom Rust project via init-rust-project, and encountered similar errors after updating. IIRC, they went away after I added the following to build.rs when in prebuilt artifacts mode:

        let link_arg = if cfg!(target_os = "macos") {
            "-rdynamic"
        } else {
            "-Wl,-export-dynamic"
        };
        println!("cargo:rustc-link-arg={link_arg}");
indygreg commented 1 year ago

The issue is due to the refactor of the linker argument handling. Python symbols should be exported from binaries embedding libpython. You can confirm this by e.g. readelf --dyn-syms ~/tmp/myapp/build/x86_64-unknown-linux-gnu/debug/install/myapp | grep PyExc_BaseException.

A source control bisection reveals the regression was caused by 798a09afe6b385515410a927901066b8a001e852.

tsibley commented 1 year ago

@indygreg Thanks for the fix!