Message ID | 20220102172159.2902783-1-foxboron@archlinux.org |
---|---|
State | New, archived |
Headers | show |
Series | [v2] strip: Use debugedit instead of AWK to parse source files | expand |
On 3/1/22 03:21, Morten Linderud wrote: > From: Morten Linderud <morten@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@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@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
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@lists.archlinux.org> +# +# 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" + 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). + 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