From 5c382b7f0e4705f447ac554dcfc7069849766710 Mon Sep 17 00:00:00 2001 From: DavHau Date: Tue, 20 Oct 2020 19:09:32 +0700 Subject: [PATCH 1/6] autoPatchelfHook: optimize performance, better error handling --- .../setup-hooks/auto-patchelf.sh | 72 ++++++++++++------- 1 file changed, 45 insertions(+), 27 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 4f7c0c14304c90c..7df454f4f017570 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -1,9 +1,16 @@ declare -a autoPatchelfLibs +declare -Ag autoPatchelfFailedDeps gatherLibraries() { autoPatchelfLibs+=("$1/lib") } +# wrapper around patchelf to raise proper error messages +# containing the tried file name and command +runPatchelf() { + patchelf $@ || (echo "Command failed: patchelf $@" && exit 1) +} + addEnvHooks "$targetOffset" gatherLibraries isExecutable() { @@ -23,14 +30,19 @@ isExecutable() { # We cache dependencies so that we don't need to search through all of them on # every consecutive call to findDependency. -declare -a cachedDependencies +declare -Ag autoPatchelfCachedDepsAssoc +declare -ag autoPatchelfCachedDeps + addToDepCache() { - local existing - for existing in "${cachedDependencies[@]}"; do - if [ "$existing" = "$1" ]; then return; fi - done - cachedDependencies+=("$1") + if [[ ${autoPatchelfCachedDepsAssoc[$1]+f} ]]; then return; fi + + # store deps in an assoc. array for efficient lookups + # otherwise findDependency would have quadratic complexity + autoPatchelfCachedDepsAssoc["$1"]="" + + # also store deps in normal array to maintian their order + autoPatchelfCachedDeps+=("$1") } declare -gi depCacheInitialised=0 @@ -43,7 +55,7 @@ getDepsFromSo() { populateCacheWithRecursiveDeps() { local so found foundso - for so in "${cachedDependencies[@]}"; do + for so in "${autoPatchelfCachedDeps[@]}"; do for found in $(getDepsFromSo "$so"); do local libdir="${found%/*}" local base="${found##*/}" @@ -76,7 +88,7 @@ findDependency() { depCacheInitialised=1 fi - for dep in "${cachedDependencies[@]}"; do + for dep in "${autoPatchelfCachedDeps[@]}"; do if [ "$filename" = "${dep##*/}" ]; then if [ "$(getSoArch "$dep")" = "$arch" ]; then foundDependency="$dep" @@ -103,7 +115,7 @@ autoPatchelfFile() { local interpreter="$(< "$NIX_CC/nix-support/dynamic-linker")" if isExecutable "$toPatch"; then - patchelf --set-interpreter "$interpreter" "$toPatch" + runPatchelf --set-interpreter "$interpreter" "$toPatch" if [ -n "$runtimeDependencies" ]; then for dep in $runtimeDependencies; do rpath="$rpath${rpath:+:}$dep/lib" @@ -115,7 +127,7 @@ autoPatchelfFile() { # We're going to find all dependencies based on ldd output, so we need to # clear the RPATH first. - patchelf --remove-rpath "$toPatch" + runPatchelf --remove-rpath "$toPatch" local missing="$( ldd "$toPatch" 2> /dev/null | \ @@ -134,18 +146,13 @@ autoPatchelfFile() { echo "found: $foundDependency" >&2 else echo "not found!" >&2 - depNotFound=1 + autoPatchelfFailedDeps["$dep"]="" fi done - # This makes sure the builder fails if we didn't find a dependency, because - # the stdenv setup script is run with set -e. The actual error is emitted - # earlier in the previous loop. - [ $depNotFound -eq 0 -o -n "$autoPatchelfIgnoreMissingDeps" ] - if [ -n "$rpath" ]; then echo "setting RPATH to: $rpath" >&2 - patchelf --set-rpath "$rpath" "$toPatch" + runPatchelf --set-rpath "$rpath" "$toPatch" fi } @@ -168,10 +175,10 @@ addAutoPatchelfSearchPath() { esac done - cachedDependencies+=( - $(find "$@" "${findOpts[@]}" \! -type d \ - \( -name '*.so' -o -name '*.so.*' \)) - ) + for file in \ + $(find "$@" "${findOpts[@]}" \! -type d \ + \( -name '*.so' -o -name '*.so.*' \)) + do addToDepCache "$file"; done } autoPatchelf() { @@ -197,14 +204,9 @@ autoPatchelf() { echo "automatically fixing dependencies for ELF files" >&2 # Add all shared objects of the current output path to the start of - # cachedDependencies so that it's choosen first in findDependency. + # autoPatchelfCachedDeps so that it's choosen first in findDependency. addAutoPatchelfSearchPath ${norecurse:+--no-recurse} -- "$@" - # Here we actually have a subshell, which also means that - # $cachedDependencies is final at this point, so whenever we want to run - # findDependency outside of this, the dependency cache needs to be rebuilt - # from scratch, so keep this in mind if you want to run findDependency - # outside of this function. while IFS= read -r -d $'\0' file; do isELF "$file" || continue segmentHeaders="$(LANG=C $READELF -l "$file")" @@ -215,8 +217,24 @@ autoPatchelf() { # Skip if the executable is statically linked. [ -n "$(echo "$segmentHeaders" | grep "^ *INTERP\\>")" ] || continue fi + # Jump file if patchelf is unable to parse it + # Some programs contain binary blobs for testing, + # which are identified as ELF but fail to be parsed by patchelf + patchelf $file || continue autoPatchelfFile "$file" done < <(find "$@" ${norecurse:+-maxdepth 1} -type f -print0) + + # fail if any dependencies were not found and + # autoPatchelfIgnoreMissingDeps is not set + local depsMissing=0 + for failedDep in "${!autoPatchelfFailedDeps[@]}"; do + echo "autoPatchelfHook could not satisfy dependency $failedDep" + depsMissing=1 + done + if [ $depsMissing == 1 -a -z "$autoPatchelfIgnoreMissingDeps" ]; then + echo "Add the missing dependencies to the build inputs or set autoPatchelfIgnoreMissingDeps=true" + exit 1 + fi } # XXX: This should ultimately use fixupOutputHooks but we currently don't have From 11a08bcfad2523aa94a3cf54abce5894e78feaca Mon Sep 17 00:00:00 2001 From: DavHau Date: Thu, 22 Oct 2020 10:15:42 +0700 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: symphorien Co-authored-by: Sandro --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 7df454f4f017570..a48071e24ebc3fe 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -8,7 +8,7 @@ gatherLibraries() { # wrapper around patchelf to raise proper error messages # containing the tried file name and command runPatchelf() { - patchelf $@ || (echo "Command failed: patchelf $@" && exit 1) + patchelf "$@" || (echo "Command failed: patchelf $*" && exit 1) } addEnvHooks "$targetOffset" gatherLibraries @@ -231,7 +231,7 @@ autoPatchelf() { echo "autoPatchelfHook could not satisfy dependency $failedDep" depsMissing=1 done - if [ $depsMissing == 1 -a -z "$autoPatchelfIgnoreMissingDeps" ]; then + if [[ $depsMissing == 1 && -z "$autoPatchelfIgnoreMissingDeps" ]]; then echo "Add the missing dependencies to the build inputs or set autoPatchelfIgnoreMissingDeps=true" exit 1 fi From f833f0406fb561e788de239a0ed9070d3818e442 Mon Sep 17 00:00:00 2001 From: DavHau Date: Fri, 23 Oct 2020 13:16:23 +0700 Subject: [PATCH 3/6] autoPatchelfHook: print dependant for missing deps --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index a48071e24ebc3fe..df2cbb4e9a28d6a 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -228,7 +228,7 @@ autoPatchelf() { # autoPatchelfIgnoreMissingDeps is not set local depsMissing=0 for failedDep in "${!autoPatchelfFailedDeps[@]}"; do - echo "autoPatchelfHook could not satisfy dependency $failedDep" + echo "autoPatchelfHook could not satisfy dependency $failedDep wanted by ${autoPatchelfFailedDeps[$failedDep]}" depsMissing=1 done if [[ $depsMissing == 1 && -z "$autoPatchelfIgnoreMissingDeps" ]]; then From b9d2541a3700ec2ad4c75df11821c4f387a4512d Mon Sep 17 00:00:00 2001 From: DavHau Date: Fri, 23 Oct 2020 13:21:08 +0700 Subject: [PATCH 4/6] autoPatchelfHook: store dependant for dependency --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index df2cbb4e9a28d6a..55481200b8bf927 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -146,7 +146,7 @@ autoPatchelfFile() { echo "found: $foundDependency" >&2 else echo "not found!" >&2 - autoPatchelfFailedDeps["$dep"]="" + autoPatchelfFailedDeps["$dep"]="$toPatch" fi done From 112f275f4db95ef465ea4d24c0be2a2ca0b3decc Mon Sep 17 00:00:00 2001 From: DavHau Date: Mon, 26 Oct 2020 17:17:07 +0700 Subject: [PATCH 5/6] autoPatchelfHook: fix typos in comments --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index 55481200b8bf927..f51f016c334d0ff 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -41,7 +41,7 @@ addToDepCache() { # otherwise findDependency would have quadratic complexity autoPatchelfCachedDepsAssoc["$1"]="" - # also store deps in normal array to maintian their order + # also store deps in normal array to maintain their order autoPatchelfCachedDeps+=("$1") } @@ -204,7 +204,7 @@ autoPatchelf() { echo "automatically fixing dependencies for ELF files" >&2 # Add all shared objects of the current output path to the start of - # autoPatchelfCachedDeps so that it's choosen first in findDependency. + # autoPatchelfCachedDeps so that it's chosen first in findDependency. addAutoPatchelfSearchPath ${norecurse:+--no-recurse} -- "$@" while IFS= read -r -d $'\0' file; do From 05fa0f1a2ec5ad455e16a72bdb304f7c0e098b76 Mon Sep 17 00:00:00 2001 From: DavHau Date: Sat, 7 Nov 2020 18:08:48 +0700 Subject: [PATCH 6/6] improve things shellcheck complains about --- pkgs/build-support/setup-hooks/auto-patchelf.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/build-support/setup-hooks/auto-patchelf.sh b/pkgs/build-support/setup-hooks/auto-patchelf.sh index f51f016c334d0ff..49e84f84ceb335b 100644 --- a/pkgs/build-support/setup-hooks/auto-patchelf.sh +++ b/pkgs/build-support/setup-hooks/auto-patchelf.sh @@ -57,7 +57,6 @@ populateCacheWithRecursiveDeps() { local so found foundso for so in "${autoPatchelfCachedDeps[@]}"; do for found in $(getDepsFromSo "$so"); do - local libdir="${found%/*}" local base="${found##*/}" local soname="${base%.so*}" for foundso in "${found%/*}/$soname".so*; do @@ -113,7 +112,8 @@ findDependency() { autoPatchelfFile() { local dep rpath="" toPatch="$1" - local interpreter="$(< "$NIX_CC/nix-support/dynamic-linker")" + local interpreter + interpreter="$(< "$NIX_CC/nix-support/dynamic-linker")" if isExecutable "$toPatch"; then runPatchelf --set-interpreter "$interpreter" "$toPatch" if [ -n "$runtimeDependencies" ]; then @@ -129,7 +129,8 @@ autoPatchelfFile() { # clear the RPATH first. runPatchelf --remove-rpath "$toPatch" - local missing="$( + local missing + missing="$( ldd "$toPatch" 2> /dev/null | \ sed -n -e 's/^[\t ]*\([^ ]\+\) => not found.*/\1/p' )" @@ -137,7 +138,6 @@ autoPatchelfFile() { # This ensures that we get the output of all missing dependencies instead # of failing at the first one, because it's more useful when working on a # new package where you don't yet know its dependencies. - local -i depNotFound=0 for dep in $missing; do echo -n " $dep -> " >&2 @@ -220,7 +220,7 @@ autoPatchelf() { # Jump file if patchelf is unable to parse it # Some programs contain binary blobs for testing, # which are identified as ELF but fail to be parsed by patchelf - patchelf $file || continue + patchelf "$file" || continue autoPatchelfFile "$file" done < <(find "$@" ${norecurse:+-maxdepth 1} -type f -print0)