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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.04.2020 18:58, Anthony PERARD wrote:
> On Wed, Apr 08, 2020 at 02:46:42PM +0200, Jan Beulich wrote:
>> On 31.03.2020 12:30, Anthony PERARD wrote:
>>>     - 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).
> 
> The only macro that will consumes cmd_cc_lds_S (and other cmd_*) is
> "cmd", it is defined as:
>     cmd = @set -e; $(echo-cmd) $(cmd_$(1))
> So, "set -e" is already there, and using semicolons in commands is
> equivalent to using "&&".
> 
> With "cmd" alone, multi-line command would work as expected (unless
> $(echo-cmd) is is trying to print the command line).
> 
> It's "if_changed" macro that doesn't work with multi-line commands.
> It does:
>     $(cmd); printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
> With a multiple line command, $(make-cmd) get's expanded to multiple
> line, so the second argument of "printf" is going to be spread over
> multiple line in make, and thus multiple shell. We run into this error:
>     /bin/sh: -c: line 0: unexpected EOF while looking for matching `''
>     /bin/sh: -c: line 1: syntax error: unexpected end of file
> 
> This is why we need to have commands on a single line.
> 
> I hope the explanation is clear enough.

Yes, thanks. One question remains though: Why do we need multiple
commands here in the first place, when Linux gets away with one?

Two other remarks: For one the command's name, aiui, ought to be
cmd_cpp_lds_S (see Linux). And there ought to be cpp_flags, which
would then also be used by e.g. cmd_s_S (instead of both having
$(filter-out -Wa$(comma)%,$(a_flags)) open-coded).

Jan