support/scripts: fix fix-rpath

Commit 134900401f (support/scripts/fix-rpath: parallelize patching
files) broke the rpath fixup, because it improperly quoted or expanded
variables:

  - $@ was expanded in the main() context, rather than in the sub-bash
    as expected, propagating incorrect parameters to patch_file();

  - an array was passed without array expansion, so only the first item
    was passed; that was in turn assigned to a string, anyway loosign
    the array. Liuckily, we only ever put a single item in that array,
    so that worked by chance.

We fix that by inverting the parameters to patch_elf(), where the extra
args are passed last, so we can put as many we want in the future. We
also pass every variables as positional parameters outside the bash -c
command, which allows us proper quoting of all variables, specifically
of the extra args array which now comes last.

The ultralong line was split, too, in a hopefully easier-to-read form.

Fixing all that also required fixing the many shellcheck issues at the
same time (wome were pre-existing before 134900401f).

While at it, expand two TABs into spaces like the rest of the script.

Note: shellcheck does not seem to warn when a variable expansion will be
used as the command to run, i.e. ${PATCHELF} does not trigger the
quoting error. Still, for consistency, we also double-quote it (we know
it is a single word, as it is already double-quoted once in the script).

Fixes: 134900401f

Cc: Victor Dumas <dumasv.dev@gmail.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
This commit is contained in:
Yann E. MORIN 2023-08-07 23:04:10 +02:00
parent a0c0bc9d33
commit 3b7c7e6106
2 changed files with 24 additions and 17 deletions

View File

@ -1663,7 +1663,6 @@ support/scripts/check-bin-arch Shellcheck
support/scripts/check-host-rpath Shellcheck
support/scripts/expunge-gconv-modules Shellcheck
support/scripts/fix-configure-powerpc64.sh EmptyLastLine
support/scripts/fix-rpath Shellcheck
support/scripts/generate-gitlab-ci-yml Shellcheck
support/scripts/mkmakefile ConsecutiveEmptyLines Shellcheck
support/scripts/mkusers Shellcheck

View File

@ -53,7 +53,7 @@ Returns: 0 if success or 1 in case of error
EOF
}
: ${PATCHELF:=${HOST_DIR}/bin/patchelf}
: "${PATCHELF:=${HOST_DIR}/bin/patchelf}"
# ELF files should not be in these sub-directories
HOST_EXCLUDEPATHS="/share/terminfo"
@ -61,19 +61,23 @@ STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo"
TARGET_EXCLUDEPATHS="/lib/firmware"
patch_file() {
local PATCHELF rootdir file
local -a sanitize_extra_args
PATCHELF="${1}"
rootdir="${2}"
sanitize_extra_args="${3}"
file="${4}"
file="${3}"
shift 3
sanitize_extra_args=("${@}")
# check if it's an ELF file
rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1)
rpath="$("${PATCHELF}" --print-rpath "${file}" 2>&1)"
if test $? -ne 0 ; then
return 0
fi
# make files writable if necessary
changed=$(chmod -c u+w "${file}")
changed="$(chmod -c u+w "${file}")"
# With per-package directory support, most RPATH of host
# binaries will point to per-package directories. This won't
@ -81,26 +85,27 @@ patch_file() {
# the per-package host directory is not within ${rootdir}. So,
# we rewrite all RPATHs pointing to per-package directories so
# that they point to the global host directry.
changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@")
# shellcheck disable=SC2001 # ${var//search/replace} hard when search or replace have / in them
changed_rpath="$(echo "${rpath}" | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@")"
if test "${rpath}" != "${changed_rpath}" ; then
${PATCHELF} --set-rpath ${changed_rpath} "${file}"
"${PATCHELF}" --set-rpath "${changed_rpath}" "${file}"
fi
# call patchelf to sanitize the rpath
${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}"
"${PATCHELF}" --make-rpath-relative "${rootdir}" "${sanitize_extra_args[@]}" "${file}"
# restore the original permission
test "${changed}" != "" && chmod u-w "${file}"
}
main() {
local rootdir
local tree="${1}"
local find_args=( )
local sanitize_extra_args=( )
local rootdir tree
local -a find_args sanitize_extra_args
tree="${1}"
if ! "${PATCHELF}" --version > /dev/null 2>&1; then
echo "Error: can't execute patchelf utility '${PATCHELF}'"
exit 1
echo "Error: can't execute patchelf utility '${PATCHELF}'"
exit 1
fi
case "${tree}" in
@ -161,7 +166,10 @@ main() {
export -f patch_file
# Limit the number of cores used
find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${PARALLEL_JOBS} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
# shellcheck disable=SC2016 # ${@} has to be expanded in the sub-shell.
find "${rootdir}" "${find_args[@]}" \
| xargs -0 -r -P "${PARALLEL_JOBS:-1}" -I {} \
bash -c 'patch_file "${@}"' _ "${PATCHELF}" "${rootdir}" {} "${sanitize_extra_args[@]}"
# Restore patched patchelf utility
test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
@ -170,4 +178,4 @@ main() {
return 0
}
main ${@}
main "${@}"