Re: [Minios-devel] [UNIKRAFT PATCH v4 2/3] build: always produce 2 images: w/ and w/o debug syms

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Hey,

Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:

> Hey Yuri,
>
> thanks for this patch. It looks okay, I have just some minor comments for it. 
> I originally would have preferred a non-python solution for your `sect-strip` 
> wrapper in order to avoid a build dependency to python. However, we may 
> anyway want to update kconfig to the latest version which afaik provides 
> python support.
>
> Thanks,
>
> Simon
>
> On 24.05.19, 12:56, "Minios-devel on behalf of Yuri Volchkov" 
> <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of 
> yuri.volchkov@xxxxxxxxx> wrote:
>
>      
>     -config DEBUG_SYMBOLS
>     - bool "Debugging information"
>     - default n
>     - help
>     -         Build with debugging information.
>     -
>     -if DEBUG_SYMBOLS
>
> Since you are removing this option, could you add "No Debug (-g0) " to the 
> option list?
> For now, it is more for completeness.
That defeats the purpose of this patch. The idea is to have debug
symbols always, if you do not need them just use a stripped image. Which
is effectively the same as it was compiled with -g0.

>     +              $@.o -o $@.dbg)
>     + $(call verbose_cmd,sect-strip,$(notdir $@),\
>
> I would prefer a different verbose string than 'sect-strip'. It is 
> unfortunately longer than 7 characters, and we usually used upper-case. How 
> about 'SSTRIP', 'SESTRIP', or 'SCSTRIP'?
Agreed, I will go with SCSTRIP

>
>     +         $(SCRIPTS_DIR)/sect-strip.py $(STRIP_SECTIONS_FLAGS-y) \
>
> You named the program `sect-strip.py` but the flags variable is called 
> `STRIP_SECTIONS_FLAGS`. Why not calling it SECT_STRIP_FLAGS for consistency?
OK

> Usually we also included two variants to the cmdline for convenience (one 
> with '-y' and the other one without): $(SECT_STRIP_FLAGS) 
> $(SECT_STRIP_FLAGS-y)
Sorry, I do not see convenience here :). But it hurts readability. Could
you elaborate why non-y version is useful?

>
>     +                 --with-objcopy=$(OBJCOPY) \
>     +                 $@.dbg -o $@)
>     + $(call verbose_cmd,STRIP,$(notdir $@), $(STRIP) $@)
>     +
>      ifeq ($(CONFIG_OPTIMIZE_SYMFILE),y)
>       $(call build_cmd,NM,,$@.sym,\
>              $(NM) -n $@ > $@.sym)
>      endif
>     -# TODO: We have to revisit stripping of KVM binaries. We noticed that 
> sometimes
>     -#       the images are broken and cannot be boot with QEMU's direct 
> kernel boot
>     -#       option (fread() error is returned).
>     -#
>
> Does this problem not exist anymore? I am asking because we are always 
> stripping the image with your patch.
I did not see any problem. That's right we always stripping with my
patch. As well as we always keeping a non-stripped version. What I am
trying to say, if the problem strikes back, one would effectively have
the same workaround - just go with a non-stripped version. So he/she
could keep moving after reporting the issue.

Even if the problem is still here, how would you fix it if you never
experience it.

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel