← Back to issue list

elf: only patch elf files that aren't referenced by DT_NEEDED

View original Github issue

Metadata

Project
snapcraft
Number
#1959
Type
pull request
State
merged
Author
jhenstridge
Labels
—
Created
Updated
Closed

Current evaluation

Merged as 64f78f762, adding architecture-aware DT_NEEDED filtering to reduce patchelf usage; PR #2081 later partially reverted it and restored patching every ELF file.

Resolution: closed resolved

Reason: PR #1959 was merged in commit 64f78f762, implementing the ELF filtering and architecture checks. The optimization was later partially reverted by merged PR #2081 in commit 77770452b.

Evaluated by: mr-cal-bot

No scores available.

Issue body

- [x] Have you followed the [guidelines for contributing](https://github.com/snapcore/snapcraft/blob/master/CONTRIBUTING.md)? - [x] Have you signed the [CLA](http://www.ubuntu.com/legal/contributors/)? - [x] If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on [bug reports](https://bugs.launchpad.net/snapcraft)? - [x] If this is a new feature. Have you discussed the design on the [forum](https://forum.snapcraft.io)? - [x] Have you successfully run `./runtests.sh static`? - [x] Have you successfully run `./runtests.sh unit`? ----- This probably needs a bit more testing, but I thought I'd push the PR now to get some CI feedback. This is based on an idea I mentioned at the Snapcraft Summit to minimise the number of files we run patchelf on, as described in [bug 1747097](https://bugs.launchpad.net/snapcraft/+bug/1747097). In brief, when the dynamic linker is searching for DT_NEEDED dependencies of a library, it can use the rpath of the executable or library that pulled in the library. So for classic snaps, it should only be necessary to patch the rpath of executables or shared library plugins. As a simple heuristic, any elf file whose soname is listed in the DT_NEEDED of some other elf file doesn't need patching. I also updated ElfFile to extract architecture information from elf files. This way we don't e.g. treat an ARM library as satisfying a dependency of an AMD64 executable. LP: #1747097

Evaluation history

Date Model Scores Action Summary
stealth/space-bunny-alpha — closed resolved Merged as 64f78f762, adding architecture-aware DT_NEEDED filtering to reduce patchelf usage; PR #2081 later partially reverted it and restored patching every ELF file.
stealth/space-bunny-alpha — closed resolved Merged PR #1959 as 64f78f762, skipping patchelf on ELF files referenced through DT_NEEDED and adding architecture detection to avoid cross-architecture dependency matches.
qwen/qwen3.6-35b-a3b — — Merged after conflict resolution. Optimizes patchelf usage by only patching ELF files not listed in DT_NEEDED dependencies, reducing unnecessary modifications. Adds architecture extraction to ElfFile to prevent cross-architecture mismatches. Resolves LP #1747097.
qwen3.6-35b-a3b-mtp-q6 — — Merged. Optimizes patchelf usage by skipping ELF files referenced in DT_NEEDED dependencies and adds architecture extraction to ElfFile. Conflicts were resolved by @kyrofa before final approval and merge, addressing LP #1747097.

Update history

No update history recorded yet.

Related work

  • Superseded By: snapcraft#2081 (confidence 99%)

    Restored patching of all ELF files and partially reverted PR #1959's DT_NEEDED filtering.

Related issues

Issue Project State Summary Similarity
#1747097 patch fewer elf files snapcraft (launchpad) open Classic ELF patching still iterates every discovered file; DT_NEEDED/DT_SONAME filtering from PR #1959 was merged and then reverted, leaving the optimization open pending safer dlopen handling.
84%
#2081 elf: patch everything instead of a subset of elf files snapcraft merged Merged PR #2081 restored Snapcraft 2.39 behavior by patching every ELF file; commit 77770452b partially reverted 64f78f762a05.
75%
#1666 Refactor to support elf patching snapcraft closed ELF patching support was refactored by merged PR #1744 in commit ec7ba948a, converting the ELF implementation into reusable libraries.
75%
#3841 elf: add elf file patcher snapcraft merged Merged PR #3841 in commit e03a971b3, adding a patchelf-based ELF file patcher ported from Snapcraft 6.x for CRAFT-1191.
74%
#3588 snap: patch patchelf on riscv64 (CRAFT-566) snapcraft merged Merged PR #3588 via commit bf99dbdb2, conditionally patching PatchELF on riscv64 while leaving other architectures unchanged.
73%
#1798 elf: strip the .note.go.buildid to make room for patching elf snapcraft merged Merged #1798 in fa8b7b174, stripping Go ELF .note.go.buildid sections before patching and requiring newer patchelf header-layout support.
73%
#3985 elf: patch elf files for classic mode snapcraft merged Merged PR #3985 as commit a723254b9, adding classic-mode ELF patching so enable-patchelf parts use base-snap libraries or staged libc components.
71%
#1983 snap: update revision of patchelf to use snapcraft closed Closed unmerged; the bundled PatchELF revision bump in #1983 was superseded by #2006, which adopted packaging-based acquisition (commit aa2a40322).
70%
#6455 fix(patchelf): use the build-for architecture's dynamic linker snapcraft closed PR #6455 was closed unmerged; its patchelf cross-compilation linker fix was not delivered, and #5836 remains open.
70%