[PATCH v2] strip: Use debugedit instead of AWK to parse source files

Allan McRae allan at archlinux.org
Sun Jan 2 23:50:30 UTC 2022


On 3/1/22 03:21, Morten Linderud wrote:
> From: Morten Linderud <morten at linderud.pw>
> 
> This moves us from the fairly ugly AWK parsing line to debugedit which
> originally comes out of the rpm project.
> 
> The original code has issues parsing anything that was not straight
> C/C++ and languages like Rust or Go would return invalid source code
> files. debugedit handles all these cases better.
> 
> Fixes FS#66755
> Fixes FS#66888

Fixes FS#65677

I think that is all of them...

> Signed-off-by: Morten Linderud <morten at linderud.pw>

I'm going to make some opinionated changes to this patch.  No need to 
resubmit, but do let me know if the changes below are fine.

> ---
>   scripts/libmakepkg/executable/debugedit.sh.in | 38 +++++++++++++++++++
>   scripts/libmakepkg/tidy/strip.sh.in           | 25 +++++++++---
>   2 files changed, 58 insertions(+), 5 deletions(-)
>   create mode 100644 scripts/libmakepkg/executable/debugedit.sh.in
> 
> diff --git a/scripts/libmakepkg/executable/debugedit.sh.in b/scripts/libmakepkg/executable/debugedit.sh.in
> new file mode 100644
> index 00000000..ec0ab814
> --- /dev/null
> +++ b/scripts/libmakepkg/executable/debugedit.sh.in
> @@ -0,0 +1,38 @@
> +#!/usr/bin/bash
> +#
> +#   debugedit.sh - Confirm presence of debugedit binary
> +#
> +#   Copyright (c) 2011-2022 Pacman Development Team <pacman-dev at lists.archlinux.org>

Copyright (c) 2022

> +#
> +#   This program is free software; you can redistribute it and/or modify
> +#   it under the terms of the GNU General Public License as published by
> +#   the Free Software Foundation; either version 2 of the License, or
> +#   (at your option) any later version.
> +#
> +#   This program is distributed in the hope that it will be useful,
> +#   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#   GNU General Public License for more details.
> +#
> +#   You should have received a copy of the GNU General Public License
> +#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +[[ -n "$LIBMAKEPKG_EXECUTABLE_DEBUGEDIT_SH" ]] && return
> +LIBMAKEPKG_EXECUTABLE_DEBUGEDIT_SH=1
> +
> +LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
> +
> +source "$LIBRARY/util/message.sh"
> +source "$LIBRARY/util/option.sh"
> +
> +executable_functions+=('executable_debugedit')
> +
> +executable_debugedit() {
> +	if check_option "debug" "y"; then
> +		if ! type -p debugedit >/dev/null; then
> +			error "$(gettext "Cannot find the %s binary required for source files in debug packages.")" "debugedit"

required for including source files

> +			return 1
> +		fi
> +	fi
> +}
> diff --git a/scripts/libmakepkg/tidy/strip.sh.in b/scripts/libmakepkg/tidy/strip.sh.in
> index 92a6fb15..84732c6f 100644
> --- a/scripts/libmakepkg/tidy/strip.sh.in
> +++ b/scripts/libmakepkg/tidy/strip.sh.in
> @@ -36,8 +36,23 @@ build_id() {
>   }
>   
>   source_files() {
> -	LANG=C readelf "$1" --debug-dump 2>/dev/null | \
> -		awk '/DW_AT_name +:/{name=$NF}/DW_AT_comp_dir +:/{{if (name == "<artificial>") next}{if (name !~ /^[<\/]/) {printf "%s/", $NF}}{print name}}'
> +	# debugedit rewrites the binary. In most cases this is handled by the gcc
> +	# prefix-map switches which rewrites $srcdir to $dbgsrcdir. debugedit will
> +	# not do anything in those cases. However for binaries that does not have a
> +	# prefix-map debugedit is going to modify the binary correcting the paths.
> +	#
> +	# From the mailing list discussion:
> +	# * Without -b/-d: it just lists all source files.
> +	# * With -b, it prints all source files rooted in the specified directory, but strips the
> +	#   prefix from the output.
> +	# * With both -b and -d, it replaces any base-dir prefixes with dest-dir
> +	#   (modifying the binary), then prints all paths rooted in the dest-dir
> +	#   (again, stripping the dest-dir prefix in the output).

I found this overkill, and not exactly clear.  I'm suggesting replacing 
with:

# This function does two things:
#
# 1) rewrites source file locations for packages not respecting prefix-
#    map switches.  This ensures all source file references in debug
#    info point to $dbgsrcdir.
#
# 2) outputs a list of files from the package source files to stdout
#    while stripping the $dbgsrcdir prefix

> +	LANG=C debugedit --no-recompute-build-id \
> +		--base-dir "${srcdir}" \
> +		--dest-dir "${dbgsrcdir}" \
> +		--list-file /dev/stdout "$1" \
> +		| sort -zu | tr '\0' '\n'
>   }
>   
>   strip_file() {
> @@ -58,9 +73,9 @@ strip_file() {
>   		# copy source files to debug directory
>   		local file dest t
>   		while IFS= read -r t; do
> -			file=${t/${dbgsrcdir}/"$srcdir"}
> -			dest="${dbgsrc/"$dbgsrcdir"/}$t"
> -			if ! [[ -f $dest ]]; then
> +			file="${srcdir}/${t}"
> +			dest="${dbgsrc}/${t}"
> +			if [[ -f "$file" ]] && ! [[ -f $dest ]]; then
>   				mkdir -p "${dest%/*}"
>   				cp -- "$file" "$dest"
>   			fi



More information about the pacman-dev mailing list