On Thu, 2 Oct 2025 21:23:57 +0200 Phil Sutter wrote: > 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
I think this should work, FWIW. The validation script only wants the subsequent lines to belong to the same VAR_NAME, so we can't mix. But we can break the chain and start with VAR_NAME += to make the name re-appear. > 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). Doesn't seem to matter right now. We can adjust the validation script to reject := if we find a reason. > > + 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? I think we should support single line variables. The formatting here used spaces so I had to change it.
