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.