[WIP] autoPatchelfHook: search a valid interpreter, or fail #66620

layus · 2019-08-14T12:53:45Z

Motivation for this change

autoPachelfHook patches everything with the same interpreter
($NIX_CC/nix-support/dynamic-linker) even on incompatible binaries.
This happens in particular when building with multiStdenv.

See https://discourse.nixos.org/t/binary-still-not-working-even-when-properly-patchelfed/1570/3?u=layus for details.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
Fits CONTRIBUTING.md.

Notify maintainers

cc @tadfisher

layus · 2019-08-14T13:17:46Z

Tests finished. It seems that this patch breaks a lot of tools based on autoPatchelfHook. For example apktool fails with

searching an interpreted for /nix/store/yypk47fzqcbv0dkd9saaxfnlhcqj7qay-build-tools-28.0.3/libexec/android-sdk/build-tools/28.0.3/zipalign
found interpreter '/nix/store/iykxb0bmfjmi7s53kfg6pjbfpd8jmza6-glibc-2.27/lib/ld-linux-x86-64.so.2' for '/nix/store/yypk47fzqcbv0dkd9saaxfnlhcqj7qay-build-tools-28.0.3/libexec/android-sdk/build-tools/28.0.3/zipalign'
searching for dependencies of /nix/store/yypk47fzqcbv0dkd9saaxfnlhcqj7qay-build-tools-28.0.3/libexec/android-sdk/build-tools/28.0.3/zipalign
  libc++.so -> found: /nix/store/yypk47fzqcbv0dkd9saaxfnlhcqj7qay-build-tools-28.0.3/libexec/android-sdk/build-tools/28.0.3/lib64/libc++.so
setting RPATH to: /nix/store/yypk47fzqcbv0dkd9saaxfnlhcqj7qay-build-tools-28.0.3/libexec/android-sdk/build-tools/28.0.3/lib64
searching an interpreted for /nix/store/yypk47fzqcbv0dkd9saaxfnlhcqj7qay-build-tools-28.0.3/libexec/android-sdk/build-tools/28.0.3/aarch64-linux-android-ld
/nix/store/s50lkmcg73qpd1gdybryndpk46fb8xw9-auto-patchelf-hook/nix-support/setup-hook: line 110: /nix/store/hpzj855nkgjvg58nrhq4910sb9q3kss1-gcc-wrapper-7.4.0/nix-support/dynamic-linker-m32: No such file or directory
objdump: '': No such file
No interpreter found for arch i386 needed by ''
builder for '/nix/store/ffhs8yvjwg51p436dpjjc58a669hky5f-build-tools-28.0.3.drv' failed with exit code 1
cannot build derivation '/nix/store/h1pm5pgkvaaxsg51nknlrlap9kv8swwr-apktool-2.4.0.drv': 1 dependencies couldn't be built

layus · 2019-09-29T23:53:47Z

@aszlig Spacechem (openlab-aux/vuizvui#18) needs this to patch correclty the 32bit binary rgb2theora used for solution saves.

aszlig

Thanks for the pull request :-)

However, besides the comments mentioned, I'm against introducing unnecessary complexity if there is not a really compelling reason to do so. For the Spacechem example you mentioned, can't you just use callPackage_i686 like this, because it seems that the game is 32bit only?

aszlig · 2019-09-30T01:23:36Z

pkgs/development/mobile/androidenv/build-tools.nix

@@ -6,6 +6,8 @@ deployAndroidPackage {
    lib.optional (os == "linux") [ pkgs.glibc pkgs.zlib pkgs.ncurses5 pkgs_i686.glibc pkgs_i686.zlib pkgs_i686.ncurses5 ];
  patchInstructions = ''
    ${lib.optionalString (os == "linux") ''
+      rm -r $packageBaseDir/{i686,aarch64,mipsel,arm}-linux*


Cc: @svanderburg

aszlig · 2019-09-30T01:24:53Z

pkgs/build-support/setup-hooks/auto-patchelf.sh

+    )
+
+    echo "searching an '$(getSoArch "$toPatch")' interpreter for $toPatch" >&2
+    for f in "${linkers[@]}"; do


Suggested change

for f in "${linkers[@]}"; do

for interpreter in "${interpreters[@]}"; do

aszlig · 2019-09-30T01:26:30Z

pkgs/build-support/setup-hooks/auto-patchelf.sh

+patchElfInterpreter() {
+    local toPatch=$1
+    local linkers=( \
+        "$NIX_CC/nix-support/dynamic-linker" \


You don't need to use backslashes for continuations here, besides, why have autoPatchelfLinkers and linkers here again?

aszlig · 2019-09-30T01:30:59Z

pkgs/build-support/setup-hooks/auto-patchelf.sh


 gatherLibraries() {
    autoPatchelfLibs+=("$1/lib")
+    if [ -f "$1/nix-support/dynamic-linker" ]; then
+        autoPatchelfLinkers+=("$1/nix-support/dynamic-linker")


Suggested change

autoPatchelfLinkers+=("$1/nix-support/dynamic-linker")

autoPatchelfLinkers+="$(< "$1/nix-support/dynamic-linker")"

aszlig · 2019-09-30T01:31:26Z

pkgs/build-support/setup-hooks/auto-patchelf.sh

+        autoPatchelfLinkers+=("$1/nix-support/dynamic-linker")
+    fi
+    if [ -f "$1/nix-support/dynamic-linker-m32" ]; then
+        autoPatchelfLinkers+=("$1/nix-support/dynamic-linker-m32")


Suggested change

autoPatchelfLinkers+=("$1/nix-support/dynamic-linker-m32")

autoPatchelfLinkers+="$(< "$1/nix-support/dynamic-linker-m32")"

aszlig · 2019-09-30T01:33:58Z

pkgs/applications/audio/bitwig-studio/bitwig-studio1.nix

@@ -55,6 +55,10 @@ stdenv.mkDerivation rec {
    substitute usr/share/applications/bitwig-studio.desktop \
      $out/share/applications/bitwig-studio.desktop \
      --replace /usr/bin/bitwig-studio $out/bin/bitwig-studio
+
+    # We only support x86_64-linux anyway,
+    # and these files cannot be correctly autoPatchelfHooked


Why can't they be correctly autoPatchelfHooked?

layus · 2019-09-30T10:15:11Z

The compelling reason was that I needed it for #65069. Not sure if switching to 32bit would be enough. It seems that the canon capt drivers have both 32 and 64 bits binaries packaged.

tadfisher · 2019-10-08T00:36:04Z

I'm curious if you tried the (somewhat simpler) patch in #51588?

layus · 2019-10-08T07:38:08Z

Not sure how much more simple it is. I did not test it, but the fact that it seemingly does not introduce issues in other packages either means that it is much better than mine, or was just not tested against nixpkgs. Using dynamic-linker* is a good idea however. It makes the code more generic.

stale · 2020-06-01T08:22:47Z

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

Search for maintainers and people that previously touched the
related code and @ mention them in a comment.
Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
irc.freenode.net.

stale · 2021-07-21T01:11:54Z

I marked this as stale due to inactivity. → More info

Artturin · 2022-05-17T02:38:57Z

The hook is now python https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/setup-hooks/auto-patchelf.py

autoPatchelfHook: search a valid interpreter, or fail

aa7c214

layus requested a review from Ericson2314 as a code owner August 14, 2019 12:53

ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Aug 14, 2019

layus changed the title ~~autoPatchelfHook: search a valid interpreter, or fail~~ [WIP] autoPatchelfHook: search a valid interpreter, or fail Aug 14, 2019

layus added 6 commits August 14, 2019 15:21

autoPatchelfHook: fixup mistakes

9d1f085

autoPatchelfHook: refactor implementation

b837c7f

autoPatchelfHook: find linkers in dependencies

b73d431

androidenv: fix build-tools with new autoPatchelfHook

d2e0625

bitwig-studio: fixes for the new autoPatchelfHook

6ff9b14

autoPatchelfHook: fix typo

254214e

Mic92 requested a review from aszlig August 19, 2019 07:42

aszlig requested changes Sep 30, 2019

View reviewed changes

aszlig mentioned this pull request Sep 30, 2019

Add Spacechem as an humblebundle game openlab-aux/vuizvui#18

Open

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020

ryantm marked this pull request as draft October 23, 2020 03:05

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 23, 2020

SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021

impl mentioned this pull request Sep 22, 2021

autoPatchelfHook: improve arch/ABI compatibility, fix packages that use stdenvNoCC #137886

Merged

12 tasks

Artturin closed this May 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] autoPatchelfHook: search a valid interpreter, or fail #66620

[WIP] autoPatchelfHook: search a valid interpreter, or fail #66620

layus commented Aug 14, 2019 •

edited

Loading

layus commented Aug 14, 2019

layus commented Sep 29, 2019

aszlig left a comment

aszlig Sep 30, 2019

aszlig Sep 30, 2019

aszlig Sep 30, 2019

aszlig Sep 30, 2019

aszlig Sep 30, 2019

aszlig Sep 30, 2019

layus commented Sep 30, 2019

tadfisher commented Oct 8, 2019

layus commented Oct 8, 2019

stale bot commented Jun 1, 2020

stale bot commented Jul 21, 2021

Artturin commented May 17, 2022

	for f in "${linkers[@]}"; do
	for interpreter in "${interpreters[@]}"; do

	autoPatchelfLinkers+=("$1/nix-support/dynamic-linker")
	autoPatchelfLinkers+="$(< "$1/nix-support/dynamic-linker")"

[WIP] autoPatchelfHook: search a valid interpreter, or fail #66620

[WIP] autoPatchelfHook: search a valid interpreter, or fail #66620

Conversation

layus commented Aug 14, 2019 • edited Loading

Motivation for this change

Things done

Notify maintainers

layus commented Aug 14, 2019

layus commented Sep 29, 2019

aszlig left a comment

Choose a reason for hiding this comment

aszlig Sep 30, 2019

Choose a reason for hiding this comment

aszlig Sep 30, 2019

Choose a reason for hiding this comment

aszlig Sep 30, 2019

Choose a reason for hiding this comment

aszlig Sep 30, 2019

Choose a reason for hiding this comment

aszlig Sep 30, 2019

Choose a reason for hiding this comment

aszlig Sep 30, 2019

Choose a reason for hiding this comment

layus commented Sep 30, 2019

tadfisher commented Oct 8, 2019

layus commented Oct 8, 2019

stale bot commented Jun 1, 2020

stale bot commented Jul 21, 2021

Artturin commented May 17, 2022

layus commented Aug 14, 2019 •

edited

Loading