Re: [pacman-dev] [PATCH] makepkg: tell the compiler to record debugging info for debug packages

[Allan McRae]

On 09/10/17 13:41, Eli Schwartz wrote:
> On 10/08/2017 09:53 AM, Allan McRae wrote:
>> On 08/10/17 16:38, Eli Schwartz wrote:
>>> In commit 8b0d59b83a60eb504567590346119fe4cd891cad support was added for
>>> storing the source files of binaries in debug packages. This made use of
>>> the debugedit program which is part of the RPM package manager, which is
>>> not very standalone.
>>>
>>> The same effect can be achieved using -fdebug-prefix-map, an option
>>> accepted by both the gcc and clang compilers which modifies how the
>>> compiler itself stores the references to the source files rather than
>>> requiring us to later edit the produced binaries. This also removes the
>>> dependency on external programs like debugedit.
>>>
>>> As a result of this change, source files will only be effectively added
>>> for programs which actually use the exported CFLAGS. This is a reasonable
>>> tradeoff as we expect our CFLAGS to be used rather than ignored. Upstream
>>> software which do not produce useful debug packages are expected to fix
>>> their build systems to respect the environment CFLAGS.
>>>
>>> Signed-off-by: Eli Schwartz 
>>
>> This patch is doing many things at once.
>>
>> 1) Add the DBGSRCDIR configuration option
>> 2) Add -fdebug-prefix-map
>> 3) What ever the changes in the while loop are...
>> (at first glance they seem wrong)
>>
>> Please submit as separate patches.
> 
> I can split it into multiple patches, though these really seem to me to
> all be one thing. Or at least the changes to the while loop are a direct
> result of modifying the debug prefixes at build time rather than after
> that while loop during strip_file(). Using -fdebug-prefix-map means
> source_files() no longer reports what it originally did when you
> authored commit 8b0d59b83a60eb504567590346119fe4cd891cad. I think I
> correctly modified that function to work in reverse but I could be wrong
> as I never did take a close look at what it does.
> 
> I suppose I could split out the DBGSRCDIR configuration option you asked
> for, but I'd prefer to keep the other bits together as this ensures
> bisecting each patch still works as expected.
> 

Yes - keep #2 and #3 together.  I did not realise what the changes in
the while loop did.

Allan

Re: [pacman-dev] [PATCH] makepkg: tell the compiler to record debugging info for debug packages

[Eli Schwartz]

On 10/08/2017 09:53 AM, Allan McRae wrote:
> On 08/10/17 16:38, Eli Schwartz wrote:
>> In commit 8b0d59b83a60eb504567590346119fe4cd891cad support was added for
>> storing the source files of binaries in debug packages. This made use of
>> the debugedit program which is part of the RPM package manager, which is
>> not very standalone.
>>
>> The same effect can be achieved using -fdebug-prefix-map, an option
>> accepted by both the gcc and clang compilers which modifies how the
>> compiler itself stores the references to the source files rather than
>> requiring us to later edit the produced binaries. This also removes the
>> dependency on external programs like debugedit.
>>
>> As a result of this change, source files will only be effectively added
>> for programs which actually use the exported CFLAGS. This is a reasonable
>> tradeoff as we expect our CFLAGS to be used rather than ignored. Upstream
>> software which do not produce useful debug packages are expected to fix
>> their build systems to respect the environment CFLAGS.
>>
>> Signed-off-by: Eli Schwartz 
> 
> This patch is doing many things at once.
> 
> 1) Add the DBGSRCDIR configuration option
> 2) Add -fdebug-prefix-map
> 3) What ever the changes in the while loop are...
> (at first glance they seem wrong)
> 
> Please submit as separate patches.

I can split it into multiple patches, though these really seem to me to
all be one thing. Or at least the changes to the while loop are a direct
result of modifying the debug prefixes at build time rather than after
that while loop during strip_file(). Using -fdebug-prefix-map means
source_files() no longer reports what it originally did when you
authored commit 8b0d59b83a60eb504567590346119fe4cd891cad. I think I
correctly modified that function to work in reverse but I could be wrong
as I never did take a close look at what it does.

I suppose I could split out the DBGSRCDIR configuration option you asked
for, but I'd prefer to keep the other bits together as this ensures
bisecting each patch still works as expected.

-- 
Eli Schwartz



signature.asc
Description: OpenPGP digital signature

Re: [pacman-dev] [PATCH] makepkg: tell the compiler to record debugging info for debug packages

[Allan McRae]

On 08/10/17 16:38, Eli Schwartz wrote:
> In commit 8b0d59b83a60eb504567590346119fe4cd891cad support was added for
> storing the source files of binaries in debug packages. This made use of
> the debugedit program which is part of the RPM package manager, which is
> not very standalone.
> 
> The same effect can be achieved using -fdebug-prefix-map, an option
> accepted by both the gcc and clang compilers which modifies how the
> compiler itself stores the references to the source files rather than
> requiring us to later edit the produced binaries. This also removes the
> dependency on external programs like debugedit.
> 
> As a result of this change, source files will only be effectively added
> for programs which actually use the exported CFLAGS. This is a reasonable
> tradeoff as we expect our CFLAGS to be used rather than ignored. Upstream
> software which do not produce useful debug packages are expected to fix
> their build systems to respect the environment CFLAGS.
> 
> Signed-off-by: Eli Schwartz 

This patch is doing many things at once.

1) Add the DBGSRCDIR configuration option
2) Add -fdebug-prefix-map
3) What ever the changes in the while loop are...
(at first glance they seem wrong)

Please submit as separate patches.

> ---
>  doc/makepkg.conf.5.txt  |  6 ++
>  etc/makepkg.conf.in |  2 ++
>  scripts/libmakepkg/tidy/strip.sh.in | 11 ---
>  scripts/makepkg.sh.in   |  4 
>  4 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt
> index d61858ca..ca74c2dd 100644
> --- a/doc/makepkg.conf.5.txt
> +++ b/doc/makepkg.conf.5.txt
> @@ -215,6 +215,12 @@ Options
>   instruct makepkg which files to remove from the package. This is
>   useful for index files that are added by multiple packages.
>  
> +**DBGSRCDIR=**"/usr/src/debug"::
> + If `strip` and `debug` are specified in the `OPTIONS` array, this 
> variable
> + will instruct makepkg where to place source files for installed 
> binaries.
> + The compiler will be instructed to link this directory for the debugger
> + search path.
> +
>  **PKGDEST=**"/path/to/directory"::
>   If this value is not set, packages will, by default, be placed in the
>   current directory (location of the linkman:PKGBUILD[5]). Many people
> diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in
> index d2beef50..33c4e92d 100644
> --- a/etc/makepkg.conf.in
> +++ b/etc/makepkg.conf.in
> @@ -99,6 +99,8 @@ MAN_DIRS=({usr{,/local}{,/share},opt/*}/{man,info})
>  DOC_DIRS=(usr/{,local/}{,share/}{doc,gtk-doc} opt/*/{doc,gtk-doc})
>  #-- Files to be removed from all packages (if purge is specified)
>  PURGE_TARGETS=(usr/{,share}/info/dir .packlist *.pod)
> +#-- Directory, if any, to store source code in for debug packages
> +DBGSRCDIR="/usr/src/debug"
>  
>  #
>  # PACKAGE OUTPUT
> diff --git a/scripts/libmakepkg/tidy/strip.sh.in 
> b/scripts/libmakepkg/tidy/strip.sh.in
> index 76562808..49e8aacd 100644
> --- a/scripts/libmakepkg/tidy/strip.sh.in
> +++ b/scripts/libmakepkg/tidy/strip.sh.in
> @@ -58,14 +58,11 @@ strip_file() {
>   # copy source files to debug directory
>   local f t
>   while read -r f; do
> - t=${f/"$srcdir"/$dbgsrc}
> - mkdir -p "${t%/*}"
> - cp -- "$f" "$t"
> + t=${f/$dbgsrc/"$srcdir"}
> + mkdir -p "${f%/*}"
> + cp -- "$t" "$f"
>   done < <(source_files "$binary")
>  
> - # adjust debug symbols to point at sources
> - debugedit -b "${srcdir}" -d /usr/src/debug/ -i "$binary" &> 
> /dev/null
> -
>   # copy debug symbols to debug directory
>   mkdir -p "$dbgdir/${binary%/*}"
>   objcopy --only-keep-debug "$binary" "$dbgdir/$binary.debug"
> @@ -107,7 +104,7 @@ tidy_strip() {
>   if check_option "debug" "y"; then
>  
>   
> dbgdir="$pkgdirbase/$pkgbase-@DEBUGSUFFIX@/usr/lib/debug"
> - 
> dbgsrc="$pkgdirbase/$pkgbase-@DEBUGSUFFIX@/usr/src/debug"
> + dbgsrc="$pkgdirbase/$pkgbase-@DEBUGSUFFIX@$DBGSRCDIR"
>   mkdir -p "$dbgdir" "$dbgsrc"
>   fi
>  
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index b25e4106..b74da0a5 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -379,6 +379,10 @@ prepare_buildenv() {
>   fi
>  
>   if check_option "debug" "y"; then
> + if [[ -v DBGSRCDIR ]]; then
> + DEBUG_CFLAGS+=" -fdebug-prefix-map=$srcdir=$DBGSRCDIR"
> + DEBUG_CXXFLAGS+=" -fdebug-prefix-map=$srcdir=$DBGSRCDIR"
> + fi
>   CFLAGS+=" $DEBUG_CFLAGS"
>   CXXFLAGS+=" $DEBUG_CXXFLAGS"
>