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
