[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZY3MRl5Jtb08YotB@d3>
Date: Thu, 28 Dec 2023 14:28:06 -0500
From: Benjamin Poirier <bpoirier@...dia.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
Petr Machata <petrm@...dia.com>, Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [RFC PATCH net-next 05/10] selftests: Introduce Makefile
variable to list shared bash scripts
On 2023-12-27 21:47 +0200, Vladimir Oltean wrote:
> On Wed, Dec 27, 2023 at 09:43:56PM +0200, Vladimir Oltean wrote:
> > On Fri, Dec 22, 2023 at 08:58:31AM -0500, Benjamin Poirier wrote:
> > > diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> > > index ab376b316c36..8b79843ca514 100644
> > > --- a/Documentation/dev-tools/kselftest.rst
> > > +++ b/Documentation/dev-tools/kselftest.rst
> > > @@ -255,9 +255,15 @@ Contributing new tests (details)
> > >
> > > TEST_PROGS_EXTENDED, TEST_GEN_PROGS_EXTENDED mean it is the
> > > executable which is not tested by default.
> > > +
> > > TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
> > > test.
> > >
> > > + TEST_INCLUDES lists files which are not in the current directory or one of
> > > + its descendants but which should be included when exporting or installing
> > > + the tests. The files are listed with a path relative to
> > > + tools/testing/selftests/.
> > > +
> > > * First use the headers inside the kernel source and/or git repo, and then the
> > > system headers. Headers for the kernel release as opposed to headers
> > > installed by the distro on the system should be the primary focus to be able
> >
> > I've never had to touch this infrastructure, but the fact that TEST_INCLUDES
> > is relative to tools/testing/selftests/ when all other TEST_* variables
> > are relative to $PWD seems ... inconsistent?
I agree with your point about consistency. I have changed TEST_INCLUDES
to take paths relative to PWD. The implementation is more complicated
since the paths have to be converted to the old values anyways for
`rsync -R` but it works. I pasted the overall diff below and it'll be
part of the next version of the series.
>
> To solve the inconsistency, can it be used like this everywhere?
>
> TEST_INCLUDES := \
> $(SRC_PATH)/net/lib.sh
After the changes, it's possible to list files using SRC_PATH but I
didn't do it. Since the point is to make TEST_INCLUDES be more like
TEST_PROGS, TEST_FILES, ..., I used relative paths.
For example in net/forwarding/Makefile:
TEST_INCLUDES := \
../lib.sh
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 8b79843ca514..470cc7913647 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -259,10 +259,14 @@ Contributing new tests (details)
TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
test.
- TEST_INCLUDES lists files which are not in the current directory or one of
- its descendants but which should be included when exporting or installing
- the tests. The files are listed with a path relative to
- tools/testing/selftests/.
+ TEST_INCLUDES is similar to TEST_FILES, it lists files which should be
+ included when exporting or installing the tests, with the following
+ differences:
+ * symlinks to files in other directories are preserved
+ * the part of paths below tools/testing/selftests/ is preserved when copying
+ the files to the output directory
+ TEST_INCLUDES is meant to list dependencies located in other directories of
+ the selftests hierarchy.
* First use the headers inside the kernel source and/or git repo, and then the
system headers. Headers for the kernel release as opposed to headers
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 3f494a31b479..c289505245f5 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -188,7 +188,7 @@ run_tests: all
@for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET run_tests \
- SRC_PATH=$(shell pwd) \
+ SRC_PATH=$(shell readlink -e $$(pwd)) \
OBJ_PATH=$(BUILD) \
O=$(abs_objtree); \
done;
@@ -242,7 +242,7 @@ ifdef INSTALL_PATH
BUILD_TARGET=$$BUILD/$$TARGET; \
$(MAKE) install OUTPUT=$$BUILD_TARGET -C $$TARGET \
INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \
- SRC_PATH=$(shell pwd) \
+ SRC_PATH=$(shell readlink -e $$(pwd)) \
OBJ_PATH=$(INSTALL_PATH) \
O=$(abs_objtree) \
$(if $(FORCE_TARGETS),|| exit); \
diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index a61fe339b9be..03a089165d3f 100644
--- a/tools/testing/selftests/drivers/net/bonding/Makefile
+++ b/tools/testing/selftests/drivers/net/bonding/Makefile
@@ -18,7 +18,7 @@ TEST_FILES := \
bond_topo_3d1c.sh
TEST_INCLUDES := \
- net/forwarding/lib.sh \
- net/lib.sh
+ ../../../net/forwarding/lib.sh \
+ ../../../net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/dsa/Makefile b/tools/testing/selftests/drivers/net/dsa/Makefile
index 8259eac80c3b..cd6817fe5be6 100644
--- a/tools/testing/selftests/drivers/net/dsa/Makefile
+++ b/tools/testing/selftests/drivers/net/dsa/Makefile
@@ -16,17 +16,17 @@ TEST_FILES := \
forwarding.config
TEST_INCLUDES := \
- net/forwarding/bridge_locked_port.sh \
- net/forwarding/bridge_mdb.sh \
- net/forwarding/bridge_mld.sh \
- net/forwarding/bridge_vlan_aware.sh \
- net/forwarding/bridge_vlan_mcast.sh \
- net/forwarding/bridge_vlan_unaware.sh \
- net/forwarding/lib.sh \
- net/forwarding/local_termination.sh \
- net/forwarding/no_forwarding.sh \
- net/forwarding/tc_actions.sh \
- net/forwarding/tc_common.sh \
- net/lib.sh
+ ../../../net/forwarding/bridge_locked_port.sh \
+ ../../../net/forwarding/bridge_mdb.sh \
+ ../../../net/forwarding/bridge_mld.sh \
+ ../../../net/forwarding/bridge_vlan_aware.sh \
+ ../../../net/forwarding/bridge_vlan_mcast.sh \
+ ../../../net/forwarding/bridge_vlan_unaware.sh \
+ ../../../net/forwarding/lib.sh \
+ ../../../net/forwarding/local_termination.sh \
+ ../../../net/forwarding/no_forwarding.sh \
+ ../../../net/forwarding/tc_actions.sh \
+ ../../../net/forwarding/tc_common.sh \
+ ../../../net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/team/Makefile b/tools/testing/selftests/drivers/net/team/Makefile
index 8a9846b5a209..2d5a76d99181 100644
--- a/tools/testing/selftests/drivers/net/team/Makefile
+++ b/tools/testing/selftests/drivers/net/team/Makefile
@@ -4,8 +4,8 @@
TEST_PROGS := dev_addr_lists.sh
TEST_INCLUDES := \
- drivers/net/bonding/lag_lib.sh \
- net/forwarding/lib.sh \
- net/lib.sh
+ ../bonding/lag_lib.sh \
+ ../../../net/forwarding/lib.sh \
+ ../../../net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 2b6c2be4f356..087fee22dd53 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -69,14 +69,29 @@ define RUN_TESTS
run_many $(1)
endef
+define INSTALL_INCLUDES
+ $(if $(TEST_INCLUDES), \
+ relative_files=""; \
+ for entry in $(TEST_INCLUDES); do \
+ entry_dir=$$(readlink -e "$$(dirname "$$entry")"); \
+ entry_name=$$(basename "$$entry"); \
+ relative_dir=$${entry_dir#"$$SRC_PATH"/}; \
+ if [ "$$relative_dir" = "$$entry_dir" ]; then \
+ echo "Error: TEST_INCLUDES entry \"$$entry\" not located inside selftests directory ($$SRC_PATH)" >&2; \
+ exit 1; \
+ fi; \
+ relative_files="$$relative_files $$relative_dir/$$entry_name"; \
+ done; \
+ cd $(SRC_PATH) && rsync -aR $$relative_files $(OBJ_PATH)/ \
+ )
+endef
+
run_tests: all
ifdef building_out_of_srctree
@if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \
rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \
fi
- $(if $(TEST_INCLUDES), \
- cd $(SRC_PATH) && rsync -aR $(TEST_INCLUDES) $(OBJ_PATH)/ \
- )
+ @$(INSTALL_INCLUDES)
@if [ "X$(TEST_PROGS)" != "X" ]; then \
$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \
$(addprefix $(OUTPUT)/,$(TEST_PROGS))) ; \
@@ -106,9 +121,7 @@ endef
install: all
ifdef INSTALL_PATH
$(INSTALL_RULE)
- $(if $(TEST_INCLUDES), \
- cd $(SRC_PATH) && rsync -aR $(TEST_INCLUDES) $(OBJ_PATH)/ \
- )
+ $(INSTALL_INCLUDES)
else
$(error Error: set INSTALL_PATH to use install)
endif
diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index 5b55c0467eed..1fba2717738d 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -130,6 +130,6 @@ TEST_PROGS_EXTENDED := devlink_lib.sh \
tc_common.sh
TEST_INCLUDES := \
- net/lib.sh
+ ../lib.sh
include ../../lib.mk
Powered by blists - more mailing lists