| Message ID | 20230523163811.30792-4-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk | expand |
On 23/05/2023 5:37 pm, Anthony PERARD wrote: > 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 > @@ -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 $@ I'd suggest that this be LD32 because it is different to most other LD's in the log. However, this is a definite improvement. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> Happy to fix up on commit.
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 something). Plus if such behavior was intended, shouldn't $(targets) be made a prereq of .PRECIOUS in generic makefile logic? > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > 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? Jan > + > +.PRECIOUS: %.lnk
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@citrix.com> > > --- > > 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,
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) head-bin-objs := $(addprefix $(obj)/,$(head-bin-objs)) @@ -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 + +.PRECIOUS: %.lnk
We are adding %.lnk to .PRECIOUS or make treat them as intermediate targets and remove them. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen/arch/x86/boot/Makefile | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)