[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v4 12/18] xen/build: factorise generation of the linker scripts



On 31.03.2020 12:30, Anthony PERARD wrote:
> In Arm and X86 makefile, generating the linker script is the same, so
> we can simply have both call the same macro.
> 
> We need to add *.lds files into extra-y so that Rules.mk can find the
> .*.cmd dependency file and load it.
> 
> Change made to the command line:
> - Use of $(CPP) instead of $(CC) -E
> - Remove -Ui386.
>   We don't compile Xen for i386 anymore, so that macro is never
>   defined. Also it doesn't make sense on Arm.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> 
> Notes:
>     v4:
>     - fix rebuild by adding FORCE as dependency
>     - Use $(CPP)
>     - remove -Ui386
>     - avoid using "define" for cmd_cc_lds_S, as adding '; \' on each line is
>       still mandatory for if_changed (or cmd) macro to work.

I still don't believe in there being a need for "; \" there. This
actually breaks things, after all:

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -236,6 +236,12 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) 
> $< -o $@
>  %.s: %.S FORCE
>       $(call if_changed,cpp_s_S)
>  
> +# Linker scripts, .lds.S -> .lds
> +quiet_cmd_cc_lds_S = LDS     $@
> +cmd_cc_lds_S = $(CPP) -P $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \
> +    sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new; \
> +    mv -f $(dot-target).d.new $(dot-target).d

if $(CPP) or sed fail, previously the whole rule would have failed,
which no longer is the case with your use of semicolons. There
ought to be a solution to this, ideally one better than adding
"set -e" as the first command ("define" would at least deal with
the multi-line make issue, but without it being clear to me why the
semicolons would be needed I don't think I can suggest anything
there at the moment).

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.