From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id A70ED15807A for ; Wed, 04 Jun 2025 19:57:21 +0000 (UTC) Received: from lists.gentoo.org (bobolink.gentoo.org [140.211.166.189]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: relay-lists.gentoo.org@gentoo.org) by smtp.gentoo.org (Postfix) with ESMTPSA id 8E9DB343157 for ; Wed, 04 Jun 2025 19:57:21 +0000 (UTC) Received: from bobolink.gentoo.org (localhost [127.0.0.1]) by bobolink.gentoo.org (Postfix) with ESMTP id E08551103DD; Wed, 04 Jun 2025 19:57:16 +0000 (UTC) Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by bobolink.gentoo.org (Postfix) with ESMTPS id DBA491103DD for ; Wed, 04 Jun 2025 19:57:16 +0000 (UTC) Received: from oystercatcher.gentoo.org (oystercatcher.gentoo.org [148.251.78.52]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id 91C4C3430F5 for ; Wed, 04 Jun 2025 19:57:16 +0000 (UTC) Received: from localhost.localdomain (localhost [IPv6:::1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id 3684A28EC for ; Wed, 04 Jun 2025 19:57:15 +0000 (UTC) From: "Sam James" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Sam James" Message-ID: <1749067025.4400139732bf183b3eee90fe4fe4765d6943ba07.sam@gentoo> Subject: [gentoo-commits] proj/portage:master commit in: bin/ X-VCS-Repository: proj/portage X-VCS-Files: bin/estrip X-VCS-Directories: bin/ X-VCS-Committer: sam X-VCS-Committer-Name: Sam James X-VCS-Revision: 4400139732bf183b3eee90fe4fe4765d6943ba07 X-VCS-Branch: master Date: Wed, 04 Jun 2025 19:57:15 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply X-Archives-Salt: 1b1ec124-80e9-4c7f-be48-ebf6490676c3 X-Archives-Hash: 242ca85dced42f7012003b29312d13de commit: 4400139732bf183b3eee90fe4fe4765d6943ba07 Author: Kerin Millar plushkava net> AuthorDate: Wed Jun 4 12:20:31 2025 +0000 Commit: Sam James gentoo 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 plushkava.net> Signed-off-by: Sam James 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#/}"