autoPatchelfHook: fix symlink handling #210908
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The auto-patchelf python script assembles a list of library (so=shared object) file names and their paths. This helps speed up the discovery of library files later when patching elf files. As further optimization, if a symlink points to a library file, the script uses the resolved path and file name. However, this produces a broken list entry if the symlink's target name doesn't match the symlink's name. A symptom of the bug, affecting the `tsm-client` package, is fixed in NixOS#172372 . The commit at hand stops resolving symlinks if the target name differs from the symlink's name. The commit has been authored by layus (Guillaume Maudoux <layus.on@gmail.com>) in pull request comment NixOS#172372 (comment)
This reverts commit 1ed9ba0. After the underlying bug in `auto-patchelf.py` got fixed, this workaround is no longer necessary.
ofborg
bot
added
11.by: package-maintainer
|
Now, improvements would be to either
Because as-is, this change risks to defeat the the optimization, as most links will be of the form /foo/lib.so.3 and resolved to lib.so.3.0.1.so, and as the names do not match, the optimization will be disabled. So implementing 1) is less overkill than it seems. But 🤷 . At least now the code is correct. Thanks again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
The new
autoPatchelfHookbuilds a list of library files/paths to speed up library discovery when patching elf files. However, under certain circumstances, this list gets faulty entries, leading to broken RPATHs.In #172372 a workaround for
tsm-clientwas implemented as its build process triggered the bug. In #172372 (comment)autoPatchelfHookauthor @layus uncovered and analysed the underlying bug, and drafted a patch to fixauto-patchelf. Please see there or in the commit message for more details on the bug and the fix.The pull-request at hand implements that patch, and removes the workaround from
tsm-client.autoPatchelfHookdoes not have a maintainer. Notifying its author and CODEOWNER:Dear @layus , I took the liberty to wrap your patch in the pull request at hand. Please have a look to verify if this is according to your intentions and could be merged.
Things done
I tested the fixed
autoPatchelfHookwithtsm-clientandzoom-us:sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes