diff mbox series

build: fix dependency tracking for preprocessed files

Message ID bb246053-f49b-58af-5381-fe0674f645de@suse.com (mailing list archive)
State Superseded
Headers show
Series build: fix dependency tracking for preprocessed files | expand

Commit Message

Jan Beulich June 4, 2020, 10:22 a.m. UTC
While the issue is more general, I noticed that asm-macros.i not getting
re-generated as needed. This was due to its .*.d file mentioning
asm-macros.o instead of asm-macros.i. Use -MQ here as well, and while at
it also use -MQ to avoid the somewhat fragile sed-ary on the *.lds
dependency tracking files. While there, further avoid open-coding $(CPP)
and drop the bogus -Ui386.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Andrew Cooper June 4, 2020, 11:35 a.m. UTC | #1
On 04/06/2020 11:22, Jan Beulich wrote:
> While the issue is more general, I noticed that asm-macros.i not getting
> re-generated as needed. This was due to its .*.d file mentioning
> asm-macros.o instead of asm-macros.i. Use -MQ here as well, and while at
> it also use -MQ to avoid the somewhat fragile sed-ary on the *.lds
> dependency tracking files. While there, further avoid open-coding $(CPP)
> and drop the bogus -Ui386.

Its not bogus.  It really is needed to prevent OUTPUT_ARCH(i386:x86-64)
being preprocessed to OUTPUT_ARCH(1:x86-64)

This explodes properly with 32bit builds, but we might get away with it
now on a 64bit build (preprocessing without -m32 does appear to skip
this transformation).

However, the robust way to deal with it is:

/* Don't clobber the ld directive */
#undef i386

unconditionally in xen.lds.S

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -201,13 +201,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y)
>  	$(call if_changed,obj_init_o)
>  
>  quiet_cmd_cpp_i_c = CPP     $@
> -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
> +cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@ -MQ $@

Please can -MQ come before $<, so the input and output files are still
at the end of the command.  It is a very useful property of the current
setup, when playing build system surgery.

If you're happy with both of these suggestions, Reviewed-by: Andrew
Cooper <andrew.cooper3@citrix.com> to save another round trip.

Alternatively, I'm happy to submit the i386 as a prereq patch, seeing as
it isn't now such a trivial change any more.

~Andrew
Jan Beulich June 4, 2020, 11:55 a.m. UTC | #2
On 04.06.2020 13:35, Andrew Cooper wrote:
> On 04/06/2020 11:22, Jan Beulich wrote:
>> While the issue is more general, I noticed that asm-macros.i not getting
>> re-generated as needed. This was due to its .*.d file mentioning
>> asm-macros.o instead of asm-macros.i. Use -MQ here as well, and while at
>> it also use -MQ to avoid the somewhat fragile sed-ary on the *.lds
>> dependency tracking files. While there, further avoid open-coding $(CPP)
>> and drop the bogus -Ui386.
> 
> Its not bogus.  It really is needed to prevent OUTPUT_ARCH(i386:x86-64)
> being preprocessed to OUTPUT_ARCH(1:x86-64)
> 
> This explodes properly with 32bit builds, but we might get away with it
> now on a 64bit build (preprocessing without -m32 does appear to skip
> this transformation).

We haven't been doing 32-bit builds for quite a while, hence I continue
to assert the -U option is bogus; I'm not claiming it always has been
(that's true just for Arm).

> However, the robust way to deal with it is:
> 
> /* Don't clobber the ld directive */
> #undef i386
> 
> unconditionally in xen.lds.S

This would mean to add code with no effect, which I'd prefer to avoid.

>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -201,13 +201,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y)
>>  	$(call if_changed,obj_init_o)
>>  
>>  quiet_cmd_cpp_i_c = CPP     $@
>> -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
>> +cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@ -MQ $@
> 
> Please can -MQ come before $<, so the input and output files are still
> at the end of the command.  It is a very useful property of the current
> setup, when playing build system surgery.

Ah yes, but then I'll make it " -MQ $@ -o $@ $<", better matching the
.lds rules (where I'll also move -MQ ahead of -o).

> If you're happy with both of these suggestions, Reviewed-by: Andrew
> Cooper <andrew.cooper3@citrix.com> to save another round trip.

As per above, only the latter, so for now I won't put it into the
patch.

> Alternatively, I'm happy to submit the i386 as a prereq patch, seeing as
> it isn't now such a trivial change any more.

As per above, I don't think such an adjustment is wanted or needed.

Jan
diff mbox series

Patch

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -201,13 +201,13 @@  $(filter %.init.o,$(obj-y) $(obj-bin-y)
 	$(call if_changed,obj_init_o)
 
 quiet_cmd_cpp_i_c = CPP     $@
-cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
+cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@ -MQ $@
 
 quiet_cmd_cc_s_c = CC      $@
 cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
 
 quiet_cmd_s_S = CPP     $@
-cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
+cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@ -MQ $@
 
 %.i: %.c FORCE
 	$(call if_changed,cpp_i_c)
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -123,9 +123,7 @@  asm-offsets.s: $(TARGET_SUBARCH)/asm-off
 	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
 
 xen.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(a_flags) -o $@ $<
-	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
-	mv -f .xen.lds.d.new .xen.lds.d
+	$(CPP) -P $(a_flags) -o $@ -MQ $@ $<
 
 dtb.o: $(CONFIG_DTB_FILE)
 
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -244,9 +244,7 @@  $(BASEDIR)/include/asm-x86/asm-macros.h:
 
 efi.lds: AFLAGS-y += -DEFI
 xen.lds efi.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<
-	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
-	mv -f .$(@F).d.new .$(@F).d
+	$(CPP) -P $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ -MQ $@ $<
 
 boot/mkelf32: boot/mkelf32.c
 	$(HOSTCC) $(HOSTCFLAGS) -o $@ $<