On 27/06/2019 13:10, Jan Beulich wrote:
>>>> On 27.06.19 at 13:51, <andrew.coop...@citrix.com> wrote:
>> On 27/06/2019 10:33, Roger Pau Monne wrote:
>>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>>> index 8a8d8f060f..94e6c9aee3 100644
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -99,9 +99,14 @@ endif
>>> syms-warn-dup-y := --warn-dup
>>> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>>>
>>> +$(TARGET): TMP = $(@D)/.$(@F)
>> I'd suggest giving this a .elf32 suffix to make it clear which pass of
>> the build it comes from.
>>
>>> $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
>>> - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET)
>>> $(XEN_IMG_OFFSET) \
>>> + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \
>>> `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) .
>>> __2M_rwdata_end$$/0x\1/p'`
>>> + # Check for multiboot{1,2} headers
>>> + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null ||
>>> + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null
>> This works, but
>>
>> Makefile:104: recipe for target '/local/xen.git/xen/xen' failed
>>
>> Isn't helpful to identify what went wrong. Sadly, we can't use $(error
>> ...) in a shell snippet,
> I think we could:
>
> $(if $(shell od -t x4 -N 8192 $(TMP) | grep 1badb002),,$(error
> ...)exit 1)
>
> But I admit I didn't check whether it is well defined that only one of
> the last two operands of $(if ) get actually evaluated.
Of the two options, I think the || { ; false } is clearer to follow.
>
>> but:
>>
>> andrewcoop@andrewcoop:/local/xen.git/xen$ git d
>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>> index 94e6c9aee3..a1d6605a8b 100644
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -99,13 +99,14 @@ endif
>> syms-warn-dup-y := --warn-dup
>> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>>
>> -$(TARGET): TMP = $(@D)/.$(@F)
>> +$(TARGET): TMP = $(@D)/.$(@F).elf32
>> $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
>> ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP)
>> $(XEN_IMG_OFFSET) \
>> `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) .
>> __2M_rwdata_end$$/0x\1/p'`
>> - # Check for multiboot{1,2} headers
>> - od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null
>> - od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null
>> + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \
>> + { echo "No Multiboot1 header found"; false; }
>> + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null || \
>> + { echo "No Multiboot2 header found"; false; }
>> mv $(TMP) $(TARGET)
>>
>> ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o
>> $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
>>
>> results in:
>>
>> No Multiboot1 header found
>> Makefile:104: recipe for target '/local/xen.git/xen/xen' failed
>> make[2]: *** [/local/xen.git/xen/xen] Error 1
>> Makefile:136: recipe for target '/local/xen.git/xen/xen' failed
>> make[1]: *** [/local/xen.git/xen/xen] Error 2
>> Makefile:45: recipe for target 'build' failed
>> make: *** [build] Error 2
>>
>> Which is far more clear.
>>
>> Thoughts?
> Good idea. The only further request I have is to add >&2 to the
> echo commands.
Done.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
