Re: [PATCH] x86, build: Improve the isolinux searching of isoimage generation

[Masahiro Yamada]

2017-10-31 18:39 GMT+09:00 Ingo Molnar <mingo@xxxxxxxxxx>:
>
> * changbin.du@xxxxxxxxx <changbin.du@xxxxxxxxx> wrote:
>
>> From: Changbin Du <changbin.du@xxxxxxxxx>
>>
>> Recently I failed to build isoimage target, because the path of isolinux.bin
>> changed to /usr/xxx/ISOLINUX/isolinux.bin, as well as ldlinux.c32 which
>> changed to /usr/xxx/syslinux/modules/bios/ldlinux.c32.
>>
>> This patch has a improvement of the file search:
>>   - Don't print the raw shell commands. It doesn't make sense to show the
>>     entire big block.
>>   - Show a error message instead of silent fail.
>>   - Add above new paths.
>>
>> Now it becomes:
>> Kernel: arch/x86/boot/bzImage is ready  (#62)
>> rm -rf arch/x86/boot/isoimage
>> mkdir arch/x86/boot/isoimage
>> Using /usr/lib/ISOLINUX/isolinux.bin
>> Using /usr/lib/syslinux/modules/bios/ldlinux.c32
>> cp arch/x86/boot/bzImage arch/x86/boot/isoimage/linux
>> ...
>>
>> Before:
>> Kernel: arch/x86/boot/bzImage is ready  (#63)
>> rm -rf arch/x86/boot/isoimage
>> mkdir arch/x86/boot/isoimage
>> for i in lib lib64 share end ; do \
>>       if [ -f /usr/$i/syslinux/isolinux.bin ] ; then \
>>               cp /usr/$i/syslinux/isolinux.bin arch/x86/boot/isoimage ; \
>>               if [ -f /usr/$i/syslinux/ldlinux.c32 ]; then \
>>                       cp /usr/$i/syslinux/ldlinux.c32 arch/x86/boot/isoimage ; \
>>               fi ; \
>>               break ; \
>>       fi ; \
>>       if [ $i = end ] ; then exit 1 ; fi ; \
>> done
>> arch/x86/boot/Makefile:161: recipe for target 'isoimage' failed
>> make[1]: *** [isoimage] Error 1
>
> I like these changes. Could we please further improve it: for example the boot
> image build messages are still pretty unstructured, while regular build system
> messages come in the following format:
>
>   CC      arch/x86/events/msr.o
>   RELOCS  arch/x86/realmode/rm/realmode.relocs
>   OBJCOPY arch/x86/realmode/rm/realmode.bin
>   CC      arch/x86/kernel/signal.o
>   AS      arch/x86/realmode/rmpiggy.o
>   CC      ipc/msg.o
>   AR      arch/x86/ia32/built-in.o
>   CC      arch/x86/events/amd/iommu.o
>   CC      init/do_mounts.o
>   AR      arch/x86/realmode/built-in.o
>
> So instead of:
>
>> Kernel: arch/x86/boot/bzImage is ready  (#62)
>> rm -rf arch/x86/boot/isoimage
>> mkdir arch/x86/boot/isoimage
>> Using /usr/lib/ISOLINUX/isolinux.bin
>> Using /usr/lib/syslinux/modules/bios/ldlinux.c32
>> cp arch/x86/boot/bzImage arch/x86/boot/isoimage/linux
>
> Could we make it something more streamlined and similar to the rest of the build
> as well, like:
>
>   GEN     arch/x86/boot/bzImage
>   GEN     arch/x86/boot/isoimage
>   GEN     arch/x86/boot/isoimage/linux
>
> I.e. only mention the new files built, with an appropriate prefix.
>
> I've Cc:-ed the kbuild maintainers, maybe they have a better suggestion instead of
> the 'GEN' abbreviation?
>

Generally, the abbreviation is the tool that has processed the target,
but if you do not find an appropriate one, 'GEN' is fine.




-- 
Best Regards
Masahiro Yamada