Re: [PATCH v3 2/8] lib: collect library files in an archive

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Dec 11, 2020 at 11:00:19AM +0100, Jan Beulich wrote:
> On 10.12.2020 15:47, Anthony PERARD wrote:
> > On Mon, Nov 23, 2020 at 04:21:19PM +0100, Jan Beulich wrote:
> >> --- a/xen/Rules.mk
> >> +++ b/xen/Rules.mk
> >> @@ -60,7 +64,14 @@ include Makefile
> >>  # 
> >> ---------------------------------------------------------------------------
> >>  
> >>  quiet_cmd_ld = LD      $@
> >> -cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(real-prereqs)
> >> +cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out %.a,$(real-prereqs)) \
> >> +               --start-group $(filter %.a,$(real-prereqs)) --end-group
> > 
> > It might be a bit weird to modify the generic LD command for the benefit
> > of only prelink.o objects but it's probably fine as long as we only use
> > archives for lib.a. libelf and libfdt will just have --start/end-group
> > added to there ld command line. So I guess the change is fine.
> 
> I'm afraid I don't understand what the concern is. Neither libelf
> nor libfdt use any %.a right now. Or are you referring to them
> merely because it's just them which have got converted to using
> $(call if-changed ...), and your remark would eventually apply to
> e.g. built_in.o as well? And then further is all you're worried
> about the fact that there may be "--start-group  --end-group" on
> the command line, i.e. with nothing inbetween? If so, besides
> possibly looking a little odd if someone inspected the command
> lines closely, what possible issue do you see? (If there is one,
> making the addition of both options conditional upon there being
> any/multiple %.a in the first place wouldn't be a big problem,
> albeit Linux also doesn't care whether ${KBUILD_VMLINUX_LIBS} is
> empty.)

Well, maybe one day we might want to collect objects in build_in.a archives
rather than build_in.o, like Linux does, but Xen is probably too small
for that to have any impact on the time it takes to build. And if that
happen, we will have to change the linker's command anyway to use
--whole-archive. So the change is fine.

As for "--start-group --end-group", yes it would just look odd, but I don't
think there would by any issue with option added to the command line.

> > The rest looks good,
> > Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> Thanks, but I'd prefer the above clarified.
> 
> Jan

-- 
Anthony PERARD