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! -- Anthony PERARD
