Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #672 +/- ##
=======================================
Coverage 95.24% 95.24%
=======================================
Files 22 22
Lines 1829 1829
Branches 343 343
=======================================
Hits 1742 1742
Misses 48 48
Partials 39 39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: oraluben <[email protected]>
|
Thanks for the PR and sorry for the delay, maintainer time is scarce. cc @lkollar for further insight/comments on this proposal. |
There was a problem hiding this comment.
Pull request overview
This PR fixes auditwheel repair behavior so that shared libraries with invalid/non-existent RUNPATH/RPATH entries still get their RPATH rewritten to $ORIGIN, preventing dependency resolution failures like the one described in issue #671.
Changes:
- Add a
keep_non_existoption toparse_ld_pathsso callers can retain non-existent directories when desired. - Update
elf_read_rpaths()to preserve non-existent RUNPATH/RPATH entries (socopylib()can detect “has rpath/runpath” even when invalid). - Add a unit test asserting
copylib()sets$ORIGINwhen the library reports a non-existent RUNPATH.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/unit/test_repair.py | Adds a regression test for copylib() ensuring $ORIGIN is set even if RUNPATH points to a non-existent directory. |
| src/auditwheel/lddtree.py | Extends parse_ld_paths() with keep_non_exist to optionally keep invalid paths. |
| src/auditwheel/elfutils.py | Uses keep_non_exist=True when reading DT_RPATH/DT_RUNPATH so invalid entries are retained. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path | ||
| The object actively being parsed (used for $ORIGIN) | ||
| keep_non_exist | ||
| Do not eliminate non-exist rpath from result |
There was a problem hiding this comment.
The new keep_non_exist docstring text has a grammar issue (“non-exist rpath”). Consider rephrasing to “non-existent rpaths” (and it applies to RUNPATH too).
| Do not eliminate non-exist rpath from result | |
| Do not eliminate non-existent rpaths from the result |
| if t.entry.d_tag == "DT_RPATH": | ||
| result["rpaths"] = parse_ld_paths(t.rpath, root="/", path=str(fn)) | ||
| result["rpaths"] = parse_ld_paths( | ||
| t.rpath, | ||
| root="/", | ||
| path=str(fn), | ||
| keep_non_exist=True, | ||
| ) | ||
| elif t.entry.d_tag == "DT_RUNPATH": | ||
| result["runpaths"] = parse_ld_paths(t.runpath, root="/", path=str(fn)) | ||
| result["runpaths"] = parse_ld_paths( | ||
| t.runpath, | ||
| root="/", | ||
| path=str(fn), | ||
| keep_non_exist=True, | ||
| ) |
There was a problem hiding this comment.
The new behavior (preserving non-existent RPATH/RUNPATH entries by passing keep_non_exist=True) is not currently covered by unit tests. Please add a test that exercises elf_read_rpaths when .dynamic contains DT_RPATH/DT_RUNPATH tags and asserts parse_ld_paths is called with keep_non_exist=True (or that non-existent paths are retained).
Closes #671