Re: [XEN PATCH 03/15] build, x86: clean build log for boot/ targets

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, May 24, 2023 at 04:16:54PM +0200, Jan Beulich wrote:
> On 23.05.2023 18:37, Anthony PERARD wrote:
> > We are adding %.lnk to .PRECIOUS or make treat them as intermediate
> > targets and remove them.
> 
> What's wrong with them getting removed? Note also that's no different from
> today, so it's an orthogonal change in any event (unless I'm overlooking

Indeed, those targets are been removed today. That doesn't cause any
issue because make knows that they are just intermediate and it doesn't
have to rebuilt them if there are just missing.

But, the macro $(if_changed,) doesn't know about intermediates, and if
the target is missing, then it's going to be rebuilt. So with
$(if_changed,) the %.lnk targets are been rebuilt at every incremental
build which cause make to relink "xen" when there's otherwise nothing
to be done.
(I'm using a command like `XEN_BUILD_DATE=today XEN_BUILD_TIME=now make`
to avoid "compile.h" from been regenerated every time)

So, the change isn't orthogonal, but needs a better explanation in the
commit message.

> something). Plus if such behavior was intended, shouldn't $(targets) be
> made a prereq of .PRECIOUS in generic makefile logic?

I think I need to reevaluate what to do here. Maybe .PRECIOUS isn't the
right one to use. But yes, we probably want something generic to tell
make to never delete any $(targets) when they are intermediate.

Linux uses .SECONDARY or .NOTINTERMEDIATE, and .SECONDARY might be
better than .PRECIOUS. PRECIOUS also prevent make from delete a target
when make is interrupted or killed, which might not be desired.

> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/boot/Makefile | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> > index 03d8ce3a9e..2693b938bd 100644
> > --- a/xen/arch/x86/boot/Makefile
> > +++ b/xen/arch/x86/boot/Makefile
> > @@ -5,6 +5,8 @@ head-bin-objs := cmdline.o reloc.o
> >  nocov-y   += $(head-bin-objs)
> >  noubsan-y += $(head-bin-objs)
> >  targets   += $(head-bin-objs)
> > +targets   += $(head-bin-objs:.o=.bin)
> > +targets   += $(head-bin-objs:.o=.lnk)
> 
> Leaving aside the question of whether .lnk really should be part
> of $(targets), don't these two lines also ...
> 
> > @@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
> >  LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := 
> > --no-warn-rwx-segments
> >  LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
> >  
> > -%.bin: %.lnk
> > -   $(OBJCOPY) -j .text -O binary $< $@
> > +%.bin: OBJCOPYFLAGS := -j .text -O binary
> > +%.bin: %.lnk FORCE
> > +   $(call if_changed,objcopy)
> >  
> > -%.lnk: %.o $(src)/build32.lds
> > -   $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) 
> > -o $@ $<
> > +quiet_cmd_ld_lnk_o = LD      $@
> > +cmd_ld_lnk_o = $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter 
> > %.lds,$^) -o $@ $<
> > +
> > +%.lnk: %.o $(src)/build32.lds FORCE
> > +   $(call if_changed,ld_lnk_o)
> >  
> >  clean-files := *.lnk *.bin
> 
> ... eliminate the need for this?

Yes, but that doesn't seems to work here.

I think there's a "targets:=" missing in "Makefile.clean". So, new patch
to fix that, and then I can eliminate the "clean-files".

Cheers,

-- 
Anthony PERARD