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

Implement auditwheel repair with patchelf #742

Merged
merged 11 commits into from
Dec 19, 2021
Merged

Conversation

messense
Copy link
Member

Linux dylib dependency resolution built with lddtree-rs, patch dylib's soname and rpath is done with patchelf which means user needs to install it beforehand. Would be great if there is a pure Rust version but unfortunately I don't have the expertise to do that.

The code isn't well organized yet, but hey it works™.

Closes #649

@netlify
Copy link

netlify bot commented Dec 12, 2021

✔️ Deploy Preview for maturin-guide canceled.

🔨 Explore the source changes: 1af539a

🔍 Inspect the deploy log: https://app.netlify.com/sites/maturin-guide/deploys/61bf28a28ef35e00075972af

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

This is fancy!

The code isn't well organized yet, but hey it works™.

I couldn't even implement this, I'm happy that you're doing that :D

One other thing I've noticed is https://github.com/messense/lddtree-rs/blob/aa3ba6af1c7e00b1d084596fecc82dc3886a1a34/src/lib.rs#L138. Would this mean that a bogus lib could silently lead to a broken wheel?

BTW have you seen https://twitter.com/andy_kelley/status/1470338795266457600? It's not ready yet, but if it's completed we could drop the docker container and just compile for manylinux2010 directly. Also zig cc is on pypi so we could just list it as a dependency.

src/auditwheel/audit.rs Outdated Show resolved Hide resolved
src/auditwheel/audit.rs Show resolved Hide resolved
src/auditwheel/audit.rs Show resolved Hide resolved
"Error checking for manylinux/musllinux compliance".to_string()
}
})?;
let external_libs = if should_repair && !self.editable {
Copy link
Member

Choose a reason for hiding this comment

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

Is this even executed for editable installs?

Copy link
Member Author

@messense messense Dec 15, 2021

Choose a reason for hiding this comment

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

I shouldn't, I was being defensive there.

@messense
Copy link
Member Author

messense commented Dec 15, 2021

One other thing I've noticed is https://github.com/messense/lddtree-rs/blob/aa3ba6af1c7e00b1d084596fecc82dc3886a1a34/src/lib.rs#L138. Would this mean that a bogus lib could silently lead to a broken wheel?

Thanks for reporting. I forgot to remove that in messense/lddtree-rs@28d4c27#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759 now that it uses a placeholder Library for reporting library not found errors later.

Edit: fixed in messense/lddtree-rs@fdf698d

@messense
Copy link
Member Author

BTW have you seen https://twitter.com/andy_kelley/status/1470338795266457600? It's not ready yet, but if it's completed we could drop the docker container and just compile for manylinux2010 directly. Also zig cc is on pypi so we could just list it as a dependency.

I have seen that. Hopefully when it's ready it can be used when C++ is not involved!

@messense
Copy link
Member Author

Hmm, looks like Python 3.10.1 is broken.

src/build_context.rs Outdated Show resolved Hide resolved
@messense messense merged commit fb112e3 into PyO3:main Dec 19, 2021
@messense messense deleted the patchelf branch December 19, 2021 13:05
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Dec 23, 2021
https://build.opensuse.org/request/show/942187
by user mia + dimstar_suse
- Update to 0.12.5
  * Fix docs for new and init commands in maturin --help
    gh#PyO3/maturin#734
  * Fix undefined auditwheel policy panic
    gh#PyO3/maturin#740
  * Fix upload::canonicalize_name() regex subst
    gh#PyO3/maturin#741
  * Bump serde from 1.0.130 to 1.0.131
    gh#PyO3/maturin#745
  * Bump sha2 from 0.9.8 to 0.10.0
    gh#PyO3/maturin#746
  * Add Cargo.lock to sdist when --locked or --frozen specified
    gh#PyO3/maturin#749
  * Implement auditwheel repair with patchelf
    gh#PyO3/maturin#742
  * Support pyo3 abi3-py310 feature
    gh#PyO3/maturin#750
- Changes in 0.12.4:
  * Bump anyhow from 1.0.50 to 1.0.51
    gh#PyO3/maturin#717
  * init: new command similar to cargo init
    gh#PyO3/maturin#719
  * Don't package non-path-dep crates in sdist for workspaces
    gh#PyO
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Jan 3, 2022
* Add support for repairing cross compiled linux wheels in [#754](PyO3/maturin#754)
* Add support for `manylinux_2_28` and `manylinux_2_31` in [#755](PyO3/maturin#755)
* Remove existing so file first in `maturin develop` command to avoid triggering SIGSEV in running process in [#760](PyO3/maturin#760)

* Fix docs for `new` and `init` commands in `maturin --help` in [#734](PyO3/maturin#734)
* Add support for x86_64 Haiku in [#735](PyO3/maturin#735)
* Fix undefined auditwheel policy panic in [#740](PyO3/maturin#740)
* Fix sdist upload for packages where the pkgname contains multiple underscores in [#741](PyO3/maturin#741)
* Implement auditwheel repair with patchelf in [#742](PyO3/maturin#742)
* Add `Cargo.lock` to sdist when `--locked` or `--frozen` specified in [#749](PyO3/maturin#749)
* Infer readme file if not specified in [#751](PyO3/maturin#751)

* Add a `maturin init` command as a companion to `maturin new` in [#719](PyO3/maturin#719)
* Don't package non-path-dep crates in sdist for workspaces in [#720](PyO3/maturin#720)
* Build release packages with `password-storage` feature in [#725](PyO3/maturin#725)
* Add support for x86_64 DargonFly BSD in [#727](PyO3/maturin#727)
* Add a Python import hook in [#729](PyO3/maturin#729)
* Allow pip warnings in `maturin develop` command in [#732](PyO3/maturin#732)
@messense messense mentioned this pull request Oct 30, 2022
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.

No automatic musllinux compliance
2 participants