> On 11 Jul 2022, at 14:38, Anthony PERARD <[email protected]> wrote: > > On Fri, Jul 08, 2022 at 03:39:00PM +0000, Luca Fancellu wrote: >>> On 24 Jun 2022, at 17:04, Anthony PERARD <[email protected]> wrote: > [...] >>> For "roms.inc" target, use "$(SHELL)" instead of plain "sh". And use >>> full path to "mkhex" instead of a relative one. Lastly, add "-f" flag >>> to "mv", in case the target already exist. >>> >>> Signed-off-by: Anthony PERARD <[email protected]> >>> --- >>> diff --git a/tools/firmware/hvmloader/Makefile >>> b/tools/firmware/hvmloader/Makefile >>> index b754220839..fc20932110 100644 >>> --- a/tools/firmware/hvmloader/Makefile >>> +++ b/tools/firmware/hvmloader/Makefile >>> @@ -87,21 +89,21 @@ roms.inc: $(ROMS) >>> >>> ifneq ($(ROMBIOS_ROM),) >>> echo "#ifdef ROM_INCLUDE_ROMBIOS" >> [email protected] >>> - sh ../../misc/mkhex rombios $(ROMBIOS_ROM) >> [email protected] >>> + $(SHELL) $(XEN_ROOT)/tools/misc/mkhex rombios $(ROMBIOS_ROM) >> [email protected] >>> echo "#endif" >> [email protected] >>> endif >>> >>> ifneq ($(STDVGA_ROM),) >>> echo "#ifdef ROM_INCLUDE_VGABIOS" >> [email protected] >>> - sh ../../misc/mkhex vgabios_stdvga $(STDVGA_ROM) >> [email protected] >>> + $(SHELL) $(XEN_ROOT)/tools/misc/mkhex vgabios_stdvga $(STDVGA_ROM) >> >>> [email protected] >>> echo "#endif" >> [email protected] >>> endif >>> ifneq ($(CIRRUSVGA_ROM),) >>> echo "#ifdef ROM_INCLUDE_VGABIOS" >> [email protected] >>> - sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> [email protected] >>> + $(SHELL) $(XEN_ROOT)/tools/misc/mkhex vgabios_cirrusvga >>> $(CIRRUSVGA_ROM) >> [email protected] >>> echo "#endif" >> [email protected] >>> endif >>> - mv [email protected] $@ >>> + mv -f [email protected] $@ >> >> Here, instead of -f, is it safer -u? What’s your opinion on that? The patch >> looks good to me. > > make want to rebuild the target, so there is no reason to keep the > existing target. We do need to overwrite the existing target if it > exist. > > Thanks for the reviews!
Ok thanks for the clarification, as I said the changes looks good to me: Reviewed-by: Luca Fancellu <[email protected]> > > -- > Anthony PERARD
