-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
wrapQtAppsHook: use patchelf --print-interpreter
instead of isELFExec
#68513
Conversation
…xec` Some executables are built as PIEs (e.g. keepassxc) and are technically isELFDyn, not isELFExec. Without this change those executables will not be wrapped.
Prevents messages like this in the build log: grep: <PATH>/bin: Is a directory
Am I correct that scripts won't be patched then? I proposed to patch those as well in #65372. |
Right, it doesn't wrap scripts, only ELFs. I didn't know about that PR before. For me, this PR only fixes a regression introduced when the wrapQtAppsHook switched from using isELF to isELFExec, since at that time it stopped wrapping keepassc (and other PIEs). |
It's weird how many library files (*.so) are installed with the executable bit set. This would be so much easier if we could just iterate over executable files and wrap them. But doing that currently includes lots of *.so files. |
Is this a bugfix that needs backporting to 19.09? |
|
so... yes, it does need a backport? |
Yes @lheckemann, though in my opinion I would have waited for @ttuegel approval of the change... |
LGTM 👍 Thanks! |
Does this mean we should remove isELFExec and isELFDyn? It looks to me like they are not accurate enough to be useful. |
This reverts commit e1b80a5. This is broken in PIE (NixOS#68513). Best to not keep it in until something else starts using it.
This is broken in PIE (NixOS#68513). Best to not keep it in otherwise something else will start using it. This reverts commit e1b80a5.
Motivation for this change
Wrap all executables, even PIEs. Without this change, programs like keepassxc do not start anymore since recent Qt wrapper changes.
Fixes #68404.
References #65399.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @