Re: [PATCH] tools: Add install/uninstall targets to tests/x86_emulator

[Anthony PERARD <anthony@xenproject.org>]

On Tue, May 20, 2025 at 10:02:05PM +0100, Andrew Cooper wrote:
> On 16/05/2024 12:07 pm, Alejandro Vallejo wrote:
> > Bring test_x86_emulator in line with other tests by adding
> > install/uninstall rules.
> >
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> > ---
> >  tools/tests/x86_emulator/Makefile | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
> > index 834b2112e7fe..30edf7e0185d 100644
> > --- a/tools/tests/x86_emulator/Makefile
> > +++ b/tools/tests/x86_emulator/Makefile
> > @@ -269,8 +269,15 @@ clean:
> >  .PHONY: distclean
> >  distclean: clean
> >  
> > -.PHONY: install uninstall
> > -install uninstall:
> > +.PHONY: install
> > +install: all
> > +	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
> > +	$(if $(TARGET-y),$(INSTALL_PROG) $(TARGET-y) $(DESTDIR)$(LIBEXEC_BIN))
> > +
> > +.PHONY: uninstall
> > +uninstall:
> > +	$(RM) -- $(addprefix $(DESTDIR)$(LIBEXEC_BIN)/,$(TARGET-y))
> > +
> >  
> >  .PHONY: run32 clean32
> >  ifeq ($(XEN_COMPILE_ARCH),x86_64)
> 
> [starting a clean thread]
> 
> x86_emulator is not special enough to behave differently to the rest of
> tools/.
> 
> Theoretical concerns over cross compiling test_x86_emulator for non-x86
> can be fixed by whomever first wants to do this.

I think we are fine with regards to cross compiling. It's probably
broken.

Sometime, XEN_COMPILE_ARCH is used as XEN_TARGET_ARCH.

Also the makefile is executed only if the target is x86 (in the makefile
in the parent directory). So if the target is not x86, nothing is
install or built. But if the target is x86 and the host is !x86 then the
build is probably going to fail.

> The very real problem is that this doesn't run in x86 CI, because and
> only because it doesn't have an install target.

So, the patch seems fine to me:
Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD