[pacman-dev] makepkg: Only change debug prefix if making debug package

Message ID 20190729045203.18706-1-austin.lund@gmail.com
State New
Headers show
Series
  • [pacman-dev] makepkg: Only change debug prefix if making debug package
Related show

Commit Message

Austin Lund July 29, 2019, 4:52 a.m. UTC
Currently all debug builds will prefix /usr/src/build to the source
paths.  If debugging symbols are stripped into a separate package then
these sources files are indeed in this path.  But if the debug build is
not stripped then one is left with a complex path rewriting to perform
debugging.  It's much easier to perform sensible path rewriting in this
instance when the path is left untouched.

Signed-off-by: Austin Lund <austin.lund@gmail.com>
---
 scripts/libmakepkg/buildenv/debugflags.sh.in | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Eli Schwartz July 29, 2019, 5:37 a.m. UTC | #1
On 7/29/19 12:52 AM, Austin Lund wrote:
> Currently all debug builds will prefix /usr/src/build to the source
> paths.  If debugging symbols are stripped into a separate package then
> these sources files are indeed in this path.  But if the debug build is
> not stripped then one is left with a complex path rewriting to perform
> debugging.  It's much easier to perform sensible path rewriting in this
> instance when the path is left untouched.
> 
> Signed-off-by: Austin Lund <austin.lund@gmail.com>
> ---
>  scripts/libmakepkg/buildenv/debugflags.sh.in | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/libmakepkg/buildenv/debugflags.sh.in b/scripts/libmakepkg/buildenv/debugflags.sh.in
> index ce9c1556..ff715803 100644
> --- a/scripts/libmakepkg/buildenv/debugflags.sh.in
> +++ b/scripts/libmakepkg/buildenv/debugflags.sh.in
> @@ -30,8 +30,10 @@ buildenv_functions+=('buildenv_debugflags')
>  
>  buildenv_debugflags() {
>  	if check_option "debug" "y"; then
> -		DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> -		DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> +		if check_option "strip" "y"; then

if check_option "debug" "y" && check_option "strip" "y";

> +			DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> +			DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> +		fi
>  		CFLAGS+=" $DEBUG_CFLAGS"
>  		CXXFLAGS+=" $DEBUG_CXXFLAGS"
>  	fi

But I would expect if people wished to debug the build, they would use
debug + strip -- the default is to strip and not debug, which is two
negatives for debugging, so the user is clearly changing something, and
relying on the source files for stepping through the debugger to be
available after the build, is not the optimal move anyway. Depending on
how you build, it may be in a regularly purged container, a tmpfs, or
simply that makepkg -c will delete it

The debug prefix map is also valued by people who do not wish build
environment artifacts to be exposed in the final binaries.
(Granted they may wish for -fmacro-prefix-map too, which makepkg does
not handle.)

...

Note that AFAIK it should be a simple matter of using set
substitute-path in gdb to reverse the -fdebug-prefix-map in makepkg, so
it's not all that complex anyway (but is package-specific).
Austin Lund July 30, 2019, 4:57 a.m. UTC | #2
On Mon, 29 Jul 2019 at 15:37, Eli Schwartz <eschwartz@archlinux.org> wrote:
>
> On 7/29/19 12:52 AM, Austin Lund wrote:
> > Currently all debug builds will prefix /usr/src/build to the source
> > paths.  If debugging symbols are stripped into a separate package then
> > these sources files are indeed in this path.  But if the debug build is
> > not stripped then one is left with a complex path rewriting to perform
> > debugging.  It's much easier to perform sensible path rewriting in this
> > instance when the path is left untouched.
> >
> > Signed-off-by: Austin Lund <austin.lund@gmail.com>
> > ---
> >  scripts/libmakepkg/buildenv/debugflags.sh.in | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/libmakepkg/buildenv/debugflags.sh.in b/scripts/libmakepkg/buildenv/debugflags.sh.in
> > index ce9c1556..ff715803 100644
> > --- a/scripts/libmakepkg/buildenv/debugflags.sh.in
> > +++ b/scripts/libmakepkg/buildenv/debugflags.sh.in
> > @@ -30,8 +30,10 @@ buildenv_functions+=('buildenv_debugflags')
> >
> >  buildenv_debugflags() {
> >       if check_option "debug" "y"; then
> > -             DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> > -             DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> > +             if check_option "strip" "y"; then
>
> if check_option "debug" "y" && check_option "strip" "y";
>
> > +                     DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> > +                     DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
> > +             fi
> >               CFLAGS+=" $DEBUG_CFLAGS"
> >               CXXFLAGS+=" $DEBUG_CXXFLAGS"
> >       fi
>
> But I would expect if people wished to debug the build, they would use
> debug + strip -- the default is to strip and not debug, which is two
> negatives for debugging, so the user is clearly changing something, and
> relying on the source files for stepping through the debugger to be
> available after the build, is not the optimal move anyway. Depending on
> how you build, it may be in a regularly purged container, a tmpfs, or
> simply that makepkg -c will delete it

There are a handful of packages that force !strip (e.g. glibc,
firefox, bash).  The current prefix rewriting to /usr/src/debug
essentially ensures a symbol lookup failure when compiling with debug
enabled for these packages (unless you build in /usr/src/debug).  The
gcc substitute path can only handle one FROM path hence you cannot
simultaneously get symbols for these packages.

Would a relative path for the debug !strip case make more sense?  In
that instance gdb could be configured using "set directories" to
resolve to the build sources should they not be purged.
Eli Schwartz July 30, 2019, 5:33 a.m. UTC | #3
On 7/30/19 12:57 AM, Austin Lund wrote:
> There are a handful of packages that force !strip (e.g. glibc,
> firefox, bash). 

The Firefox package is apparently not supposed to use pacman-compatible
debugging symbols at all. The symbols are separately handled, then
uploaded to Mozilla; I guess you're supposed to use Mozilla's tracing
services.

glibc is forcing !strip because it needs to manually strip and thereby
exclude a few files from being stripped. I have a better solution in
aur/glibc-git for this which could probably be used.

The bash package, last I checked, was stripped normally.

> The current prefix rewriting to /usr/src/debug
> essentially ensures a symbol lookup failure when compiling with debug
> enabled for these packages (unless you build in /usr/src/debug).  The
> gcc substitute path can only handle one FROM path hence you cannot
> simultaneously get symbols for these packages.

Good point.

> Would a relative path for the debug !strip case make more sense?  In
> that instance gdb could be configured using "set directories" to
> resolve to the build sources should they not be purged.

All three possible approaches (do nothing, do your initial suggestion,
do relative paths) are rather subjective. I merely point out perspectives.

It's possibly worth noting that even if the paths are still mapped to
remove $srcdir, it's still encoded as "builddir" in the .BUILDINFO file.
You can make the binaries directory-agnostic, but not the overall package.

...

I guess if we're going to make this behavior conditional at all, simply
disabling it entirely as your initial patch does would be the simplest
approach and also works OOTB for the debugging case (assuming the build
directory is still extant).
Allan McRae July 30, 2019, 6:06 a.m. UTC | #4
On 30/7/19 3:33 pm, Eli Schwartz wrote:
> glibc is forcing !strip because it needs to manually strip and thereby
> exclude a few files from being stripped. I have a better solution in
> aur/glibc-git for this which could probably be used.

The Arch glibc package does this because Arch still does not have debug
symbol package repositories and valgrind+gdb rely on them.

That is an Arch limitation, and not something that concerns pacman/makepkg.

Allan
Austin Lund July 30, 2019, 11:34 a.m. UTC | #5
On Tue, 30 Jul 2019 at 15:33, Eli Schwartz <eschwartz@archlinux.org> wrote:
>
> > The current prefix rewriting to /usr/src/debug
> > essentially ensures a symbol lookup failure when compiling with debug
> > enabled for these packages (unless you build in /usr/src/debug).  The
> > gcc substitute path can only handle one FROM path hence you cannot
> > simultaneously get symbols for these packages.
>
> Good point.
>
> > Would a relative path for the debug !strip case make more sense?  In
> > that instance gdb could be configured using "set directories" to
> > resolve to the build sources should they not be purged.

> I guess if we're going to make this behavior conditional at all, simply
> disabling it entirely as your initial patch does would be the simplest
> approach and also works OOTB for the debugging case (assuming the build
> directory is still extant).

I had a play around with some of the options.  The way gdb handles the
lookup of sources and symbols is somewhat strange.  The only way I
have managed to get it to work without having a reference to the
original build directory is to change the prefix to something that is
easily searchable and be substituted on a per-source-tree basis.
Unlike the recommendation in the gcc manual page, this is _not_ "." as
you can't point different trees to different directories.  For the
"debug" and "!strip" case, I used:

DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=arch-${pkgname}"

That seemed to work OK as I could add a set subsitiute-path into the
build sources for each package which was "debug" and "!strip".

Patch

diff --git a/scripts/libmakepkg/buildenv/debugflags.sh.in b/scripts/libmakepkg/buildenv/debugflags.sh.in
index ce9c1556..ff715803 100644
--- a/scripts/libmakepkg/buildenv/debugflags.sh.in
+++ b/scripts/libmakepkg/buildenv/debugflags.sh.in
@@ -30,8 +30,10 @@  buildenv_functions+=('buildenv_debugflags')
 
 buildenv_debugflags() {
 	if check_option "debug" "y"; then
-		DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
-		DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
+		if check_option "strip" "y"; then
+			DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
+			DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=${DBGSRCDIR:-/usr/src/debug}"
+		fi
 		CFLAGS+=" $DEBUG_CFLAGS"
 		CXXFLAGS+=" $DEBUG_CXXFLAGS"
 	fi