RE: [PATCH] test/vsock: add install target

[Peng Fan]

> Subject: Re: [PATCH] test/vsock: add install target
> 
> On Wed, Jul 10, 2024 at 08:11:32AM GMT, Peng Fan wrote:
> >> Subject: Re: [PATCH] test/vsock: add install target
> >>
> >> On Tue, Jul 09, 2024 at 09:50:51PM GMT, Peng Fan (OSS) wrote:
> >> >From: Peng Fan <peng.fan@xxxxxxx>
> >> >
> >> >Add install target for vsock to make Yocto easy to install the
> images.
> >> >
> >> >Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> >> >---
> >> > tools/testing/vsock/Makefile | 12 ++++++++++++
> >> > 1 file changed, 12 insertions(+)
> >> >
> >> >diff --git a/tools/testing/vsock/Makefile
> >> >b/tools/testing/vsock/Makefile index a7f56a09ca9f..5c8442fa9460
> >> 100644
> >> >--- a/tools/testing/vsock/Makefile
> >> >+++ b/tools/testing/vsock/Makefile
> >> >@@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o
> >> msg_zerocopy_common.o
> >> > vsock_uring_test: LDLIBS = -luring
> >> > vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o
> >> >msg_zerocopy_common.o
> >> >
> >> >+VSOCK_INSTALL_PATH ?= $(abspath .)
> >> >+# Avoid changing the rest of the logic here and lib.mk.
> >> >+INSTALL_PATH := $(VSOCK_INSTALL_PATH)
> >> >+
> >> > CFLAGS += -g -O2 -Werror -Wall -I. -I../../include
> >> > -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow
> >> > -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -
> >> D_GNU_SOURCE
> >> > .PHONY: all test clean
> >> > clean:
> >> > 	${RM} *.o *.d vsock_test vsock_diag_test vsock_perf
> >> vsock_uring_test
> >> > -include *.d
> >> >+
> >> >+install: all
> >> >+	@# Ask all targets to install their files
> >> >+	mkdir -p $(INSTALL_PATH)/vsock
> >>
> >> why using the "vsock" subdir?
> >>
> >> IIUC you were inspired by selftests/Makefile, but it installs under
> >> $(INSTALL_PATH)/kselftest/ the scripts used by the main one
> >> `run_kselftest.sh`, which is installed in $(INSTALL_PATH instead.
> >> So in this case I would install everything in $(INSTALL_PATH).
> >>
> >> WDYT?
> >
> >I agree.
> >
> >>
> >> >+	install -m 744 vsock_test $(INSTALL_PATH)/vsock/
> >> >+	install -m 744 vsock_perf $(INSTALL_PATH)/vsock/
> >> >+	install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/
> >> >+	install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/
> >>
> >> Also from selftests/Makefile, what about using the ifdef instead of
> >> using $(abspath .) as default place?
> >>
> >> I mean this:
> >>
> >> install: all
> >> ifdef INSTALL_PATH
> >>    ...
> >> else
> >> 	$(error Error: set INSTALL_PATH to use install) endif
> >
> >Is the following looks good to you?
> >
> ># Avoid conflict with INSTALL_PATH set by the main Makefile
> >VSOCK_INSTALL_PATH ?= INSTALL_PATH := $(VSOCK_INSTALL_PATH)
> 
> I'm not a super Makefile expert, but why do we need both
> VSOCK_INSTALL_PATH and INSTALL_PATH?

INSTALL_PATH is exported by kernel root directory makefile.
So to user, we need to avoid export INSTALL_PATH here.
So I just follow selftests/Makefile using KSFT_INSTALL_PATH

Regards,
Peng.

> 
> Stefano
> 
> >
> >install: all
> >ifdef INSTALL_PATH
> >        mkdir -p $(INSTALL_PATH)
> >        install -m 744 vsock_test $(INSTALL_PATH)
> >        install -m 744 vsock_perf $(INSTALL_PATH)
> >        install -m 744 vsock_diag_test $(INSTALL_PATH)
> >        install -m 744 vsock_uring_test $(INSTALL_PATH) else
> >        $(error Error: set INSTALL_PATH to use install) Endif
> >
> >Thanks,
> >Peng.
> >>
> >> Thanks,
> >> Stefano
> >