Re: [PATCH v2] Makefile: support compressed debug info

[Nick Desaulniers]

On Wed, May 20, 2020 at 7:48 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> > Suggested-by: Fangrui Song <maskray@xxxxxxxxxx>
>
>
> Suggested-by -> Reviewed-by
>
> https://patchwork.kernel.org/patch/11524939/#23349551

Yes, my mistake.

> > Suggested-by: Nick Clifton <nickc@xxxxxxxxxx>
>
>
> I do not know where this tag came from.
>
> Nick Clifton taught us the version rule of binutils,but did not state
> anything about this patch itself.
>
> https://patchwork.kernel.org/patch/11524939/#23355175
>
>
> > Suggested-by: Sedat Dilek <sedat.dilek@xxxxxxxxx>
>
> I do not see the source of this tag, either...

Not all contributions to open source need to be in the form of
patches.  Both Sedat and Nick gave suggestions which ultimately
informed the contents of this patch.  They should be rewarded for
their efforts, and incentivized to help improve the code base further.
I think suggested by tags are a good way to do that; but if it's
against a written convention or if you still disagree, it's not the
end of the world to me, and you may drop those tags from the v3.

> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -225,6 +225,21 @@ config DEBUG_INFO_REDUCED
> >           DEBUG_INFO build and compile times are reduced too.
> >           Only works with newer gcc versions.
> >
> > +config DEBUG_INFO_COMPRESSED
> > +       bool "Compressed debugging information"
> > +       depends on DEBUG_INFO
> > +       depends on $(cc-option,-gz=zlib)
> > +       depends on $(as-option,-Wa,--compress-debug-sections=zlib)
>
> This does not work. (always false)

Technically, always true. `-Wa` disables all warnings from the
assembler.  Also, I did test this via `make menuconfig`.

> You cannot enable this option.
>
> The comma between -Wa and --compress-debug-sections=zlib
> is eaten by Kconfig parser because commas are delimiters
> of function parameters.
>
>
> Please write like this.
>
>     depends on $(as-option,-Wa$(comma)--compress-debug-sections=zlib)

You're right, I knew this bug forgot. Will fix in v3.

> > +       depends on $(ld-option,--compress-debug-sections=zlib)
> > +       help
> > +         Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> > +         5.0+, binutils 2.26+, and zlib.
> > +
> > +         Users of dpkg-deb via scripts/package/builddeb may
> > +         wish to set the $KDEB_COMPRESS env var to "none" to avoid recompressing
> > +         the debug info again with a different compression scheme, which can
> > +         result in larger binaries.
>
> No. This is not correct.
>
> CONFIG_DEBUG_INFO_COMPRESSED compresses the only debug info part.
> The other parts still get by benefit from the default KDEB_COMPRESS=xz.
>
>
> The numbers are here:
>
>
> CONFIG_DEBUG_INFO_COMPRESSED=y
> -rw-r--r-- 1 masahiro masahiro 209077584 May 21 11:19
> linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-26_amd64.deb
>
>
> CONFIG_DEBUG_INFO_COMPRESSED=y and KDEB_COMPRESS=none
> -rw-r--r-- 1 masahiro masahiro 643051712 May 21 11:22
> linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-27_amd64.deb
>
>
> CONFIG_DEBUG_INFO_COMPRESSED=n
> -rw-r--r-- 1 masahiro masahiro 112200308 May 21 11:40
> linux-image-5.7.0-rc5+-dbg_5.7.0-rc5+-30_amd64.deb
>
>
>
>
> For the deb package size perspective,
> it is better to keep KDEB_COMPRESS as default.
>
> The main motivation for changing KDEB_COMPRESS
> is the build speed.  (see commit 1a7f0a34ea7d05)
>
>
>
>
> CONFIG_DEBUG_INFO_COMPRESSED has a downside
> for the debug deb package, but we need to accept it.

Ah, I see. Thank you for those measurements.  I'll send a v3 with
fixed up help text, but ultimately, I don't really care what it says
here.  Please feel empowered to reword it should you choose to accept
+ apply it.
-- 
Thanks,
~Nick Desaulniers