commit: 4400139732bf183b3eee90fe4fe4765d6943ba07
Author: Kerin Millar <kfm <AT> plushkava <DOT> net>
AuthorDate: Wed Jun 4 12:20:31 2025 +0000
Commit: Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Wed Jun 4 19:57:05 2025 +0000
URL: https://gitweb.gentoo.org/proj/portage.git/commit/?id=44001397
estrip: revamp tool name detection and existence-testing procedures
Historically - that is, from March 2018 onwards - estrip would determine
the preferred pathnames of the external utilities that it invokes and
store them in dynamically declared scalar variables. For example, in
view of the strip(1) utility, it would declare a 'STRIP' variable that
contains either of the following values.
a) "${CHOST}-strip" # for example, "x86_64-pc-linux-gnu-strip"
b) "strip" # always if the CHOST-prefixed variant isn't found
Elsewhere, estrip would then expand the 'STRIP' variable in the case
that it needs to execute the utility in question.
Now, recently I altered the code to store the perferred pathnames in an
associative array. All well and good. Thereafter, I altered the manner
in which the utilities are discovered in such a way that, if a given
utility isn't found, its value shall be taken as the empty string. To
understand the ramfications of this, one must first consider how the old
code worked, which was as follows.
# Correct, because 'debugedit' was treated as a special case and would
# indeed be the empty string if non-existent.
[[ ${debugedit} ]] && debugedit_found=true || debugedit_found=false
# Correct, for the same reasons.
if ! ${debugedit_found}; then
: issue warning here
fi
Taking the 'debugedit_found' test as a case in point, I effectively
changed it be as follows.
# Correct, though only because the value is "" for an unfound utility.
if [[ ! ${name_of[debugedit]} ]]; then
: issue warning here
fi
So, what is the problem with this? Well, upon further consideration, it
occurred to me that estrip doesn't exhaustively guard itself against
missing utilities, partly owing to its lack of error checks in places.
The use of objcopy(1) makes for a fine case in point. Here is an example
of how it might be invoked.
# No existence check; no error check; the command name might be ""
"${name_of[objcopy]}" "${objcopy_flags[@]}" "${src}" "${dst}"
Notwithstanding the unfortunate lack of error checking, consider what
happens in the case that objcopy was determined not to exist; the
expansion of "${name_of[objcopy]}" will thus be "". While there remains
no risk of any of the following options and operands being treated as
the command name, the diagnostic message will be "-bash: : command not
found", which is rather confusing.
All of which leads to the nature of this commit, whose changes are
described herewith.
Firstly, restore the previous behaviour of discerning a preferred name
for each utility, irrespective of whether it actually exists. This means
that the 'name_of' array shall always contain exactly six keys, and that
their values shall never be empty. Naturally, the intentional behaviour
of preferring the CHOST-prefixed variants has been retained.
Secondly, use the hash builtin to subsequently determine whether a given
utility actually exists. For example:
if ! hash "${name_of[debugedit]}" 2>/dev/null; then
: issue warning here
fi
Doing so takes advantage of the fact that the shell already maintains a
lookup table for commands that are not specified as an absolute path
name. As such, there is precious little sense in re-inventing the
proverbial wheel.
Fixes: 86b4553f5c38bc96b876ce625c7990e559f40885
Signed-off-by: Kerin Millar <kfm <AT> plushkava.net>
Signed-off-by: Sam James <sam <AT> gentoo.org>
bin/estrip | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/bin/estrip b/bin/estrip
index cb66a77841..f1b7140c02 100755
--- a/bin/estrip
+++ b/bin/estrip
@@ -157,16 +157,18 @@ if [[ ${KERNEL} == linux ]] && (( has_feature[xattr] ));
then
fi
fi
-# Look up the tools we might be using
+# Determine the names of the tools that might subsequently be used. For several
+# of these, their ${CHOST}-prefixed variants are preferred, if found to exist.
declare -A name_of
-for bin in debugedit dwz {"${CHOST}-",}{'objcopy','ranlib','readelf','strip'};
do
- if [[ ! ${name_of[$bin]} ]]; then
- name_of[$bin]=$(type -P -- "${bin}" 2>/dev/null)
+for bin in debugedit dwz {,"${CHOST}-"}{'objcopy','ranlib','readelf','strip'};
do
+ key=${bin#"${CHOST}-"}
+ if [[ ! ${name_of[$key]} ]] || hash "${bin}" 2>/dev/null; then
+ name_of[$key]=${bin}
fi
done
# If debugedit does not exist, consider some alternative locations for it.
-if [[ ! ${name_of[debugedit]} ]]; then
+if ! hash "${name_of[debugedit]}" 2>/dev/null; then
debugedit_paths=(
"${EPREFIX}/usr/libexec/rpm/debugedit"
)
@@ -217,7 +219,7 @@ save_elf_sources() {
if (( ! has_feature[installsources] || has_restriction[installsources]
)); then
save_elf_sources() { :; }
return
- elif [[ ! ${name_of[debugedit]} ]]; then
+ elif ! hash "${name_of[debugedit]}" 2>/dev/null; then
ewarn "FEATURES=installsources is enabled but the debugedit
binary could not be"
ewarn "found. This feature will not work unless debugedit is
installed!"
save_elf_sources() { :; }
@@ -259,7 +261,7 @@ dedup_elf_debug() {
if (( ! has_feature[dedupdebug] || has_restriction[dedupdebug] )); then
dedup_elf_debug() { :; }
return
- elif [[ ! ${name_of[dwz]} ]]; then
+ elif ! hash "${name_of[dwz]}" 2>/dev/null; then
ewarn "FEATURES=dedupdebug is enabled but the dwz binary could
not be"
ewarn "found. This feature will not work unless dwz is
installed!"
dedup_elf_debug() { :; }
@@ -339,7 +341,7 @@ save_elf_debug() {
# This should only happen with
FEATURES=-installsources, as
# it's done in save_elf_sources.
if [[ -z ${buildid} ]] ; then
- if [[ ${name_of[debugedit]} ]]; then
+ if hash "${name_of[debugedit]}" 2>/dev/null;
then
# Salt the build ID to avoid collisions
on
# bundled libraries.
buildid=$("${name_of[debugedit]}" -i \
@@ -614,7 +616,7 @@ __multijob_finish
cd "${tmpdir}"/sources/ && cat -- * > "${tmpdir}/debug.sources" 2>/dev/null
if [[ -s ${tmpdir}/debug.sources ]] \
&& (( has_feature[installsources] && ! has_restriction[installsources]
)) \
- && [[ ${name_of[debugedit]} ]]
+ && hash "${name_of[debugedit]}" 2>/dev/null
then
__vecho "installsources: rsyncing source files"
[[ -d ${D%/}/${prepstrip_sources_dir#/} ]] || mkdir -p
"${D%/}/${prepstrip_sources_dir#/}"
