Hi Jakub,
On Wed, Oct 01, 2025 at 06:30:33PM -0700, Jakub Kicinski wrote:
We get a significant number of conflicts between net and net-next because of selftests Makefile changes. People tend to append new test cases at the end of the Makefile when there's no clear sort order. Sort all networking selftests Makefiles, use the following format:
VAR_NAME := \ entry1 \ entry2 \ entry3 \ # end of VAR_NAME
A potential problem with this format is loss of context with long lists. While I don't think it will cause incorrect conflict resolutions, appending via '+=' may ease reviews of patches:
VAR_NAME := VAR_NAME += entry1 VAR_NAME += entry2 VAR_NAME += entry3
No trailing comment needed this way. Downside is '?=' can't be used.
Some Makefiles are already pretty close to this.
Which is a point to stick with it.
[...]
diff --git a/tools/testing/selftests/drivers/net/netdevsim/Makefile b/tools/testing/selftests/drivers/net/netdevsim/Makefile index 07b7c46d3311..daf51113c827 100644 --- a/tools/testing/selftests/drivers/net/netdevsim/Makefile +++ b/tools/testing/selftests/drivers/net/netdevsim/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ OR MIT -TEST_PROGS = devlink.sh \ +TEST_PROGS := \
Maybe irrelevant, but assignment type changes should be avoided IMO (there are more cases like this one).
- devlink.sh \ devlink_in_netns.sh \ devlink_trap.sh \ ethtool-coalesce.sh \
@@ -17,5 +18,6 @@ TEST_PROGS = devlink.sh \ psample.sh \ tc-mq-visibility.sh \ udp_tunnel_nic.sh \ +# end of TEST_PROGS include ../../../lib.mk
[...]
diff --git a/tools/testing/selftests/drivers/net/virtio_net/Makefile b/tools/testing/selftests/drivers/net/virtio_net/Makefile index 7ec7cd3ab2cc..868ece3fea1f 100644 --- a/tools/testing/selftests/drivers/net/virtio_net/Makefile +++ b/tools/testing/selftests/drivers/net/virtio_net/Makefile @@ -1,15 +1,12 @@ # SPDX-License-Identifier: GPL-2.0+ OR MIT -TEST_PROGS = basic_features.sh \
#+TEST_PROGS = basic_features.sh
-TEST_FILES = \
virtio_net_common.sh \#+TEST_FILES = virtio_net_common.sh
These seem intentional, so change to the syntax as proposed?
[...]
diff --git a/tools/testing/selftests/net/lib/Makefile b/tools/testing/selftests/net/lib/Makefile index 88c4bc461459..ce795bc0a1af 100644 --- a/tools/testing/selftests/net/lib/Makefile +++ b/tools/testing/selftests/net/lib/Makefile @@ -5,12 +5,16 @@ CFLAGS += -I../../../../../usr/include/ $(KHDR_INCLUDES) # Additional include paths needed by kselftest.h CFLAGS += -I../../ -TEST_FILES := ../../../../../Documentation/netlink/specs -TEST_FILES += ../../../../net/ynl +TEST_FILES := \
- ../../../../net/ynl \
- ../../../../../Documentation/netlink/specs \
+# end of TEST_FILES -TEST_GEN_FILES += csum -TEST_GEN_FILES += $(patsubst %.c,%.o,$(wildcard *.bpf.c)) -TEST_GEN_FILES += xdp_helper +TEST_GEN_FILES := \
- $(patsubst %.c,%.o,$(wildcard *.bpf.c)) \
- csum \
- xdp_helper \
+# end of TEST_GEN_FILES
This one is interesting, the old code appended only, new code might overwrite existing content.
Cheers, Phil