Message-ID: <ZZPHQF6wL95oSGzK@yujie-X299>
Date: Tue, 2 Jan 2024 16:20:16 +0800
From: Yujie Liu <yujie.liu@...el.com>
To: Guillaume Nault <gnault@...hat.com>
CC: <netdev@...r.kernel.org>, Hangbin Liu <liuhangbin@...il.com>, Paolo Abeni
	<pabeni@...hat.com>, David Ahern <dsahern@...nel.org>,
	<linux-kselftest@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<lkp@...el.com>, kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH v2 net-next] selftests/net: change shebang to bash to
 support "source"

On Sun, Dec 31, 2023 at 01:17:11PM +0100, Guillaume Nault wrote:
> On Fri, Dec 29, 2023 at 09:19:31PM +0800, Yujie Liu wrote:
> > The patch set [1] added a general lib.sh in net selftests, and converted
> > several test scripts to source the lib.sh.
> > 
> > unicast_extensions.sh (converted in [1]) and pmtu.sh (converted in [2])
> > have a /bin/sh shebang which may point to various shells in different
> > distributions, but "source" is only available in some of them. For
> > example, "source" is a built-it function in bash, but it cannot be
> > used in dash.
> > 
> > Refer to other scripts that were converted together, simply change the
> > shebang to bash to fix the following issues when the default /bin/sh
> > points to other shells.
> 
> Looks like it'd be simpler to just replace the "source" commands with
> "." and leave the shebang as is (unless there are other bash-specific
> constructs in these scripts of course).
> 
> Generally speaking, I think we should avoid madating a specific shell,
> unless that really simplifies the test script (which is not the case
> here).

Hi Guillaume,

Thanks for the comments. Actually I also considered replacing "source"
with "." at first, but finally decided to change the shebang in
consideration of being consistent with other scripts. We can see that
there are 140+ scripts in net selftests that have "source lib.sh" and
"bash" shebang, but none of the selftests has ". lib.sh". If we replace
"source" with "." and keep the "sh" shebang specifically for
unicast_extensions.sh and pmtu.sh, we will get only 2 scripts using
"sh and ." while most other scripts using "bash and source". Maybe it
would be nice to keep the consistency by changing the shebang as well?
What do you think? :)

linux/tools/testing/selftests/net$ grep -rl "source lib.sh" . | xargs grep -F '#!/bin/'
./test_vxlan_under_vrf.sh:#!/bin/bash
./test_vxlan_nolocalbypass.sh:#!/bin/bash
./xfrm_policy.sh:#!/bin/bash
./test_vxlan_mdb.sh:#!/bin/bash
./test_bridge_backup_port.sh:#!/bin/bash
./vrf_route_leaking.sh:#!/bin/bash
./l2tp.sh:#!/bin/bash
./netns-name.sh:#!/bin/bash
./rtnetlink.sh:#!/bin/bash
./ioam6.sh:#!/bin/bash
./drop_monitor_tests.sh:#!/bin/bash
./test_vxlan_vnifiltering.sh:#!/bin/bash
./icmp.sh:#!/bin/bash
./gre_gso.sh:#!/bin/bash
./fib_nexthop_multiprefix.sh:#!/bin/bash
./icmp_redirect.sh:#!/bin/bash
./vrf-xfrm-tests.sh:#!/bin/bash
./vrf_strict_mode_test.sh:#!/bin/bash
./fcnal-test.sh:#!/bin/bash
./stress_reuseport_listen.sh:#!/bin/bash
./srv6_end_dt4_l3vpn_test.sh:#!/bin/bash
./test_bridge_neigh_suppress.sh:#!/bin/bash
./cmsg_ipv6.sh:#!/bin/bash
./arp_ndisc_evict_nocarrier.sh:#!/bin/bash
./fib_rule_tests.sh:#!/bin/bash
./srv6_end_dt6_l3vpn_test.sh:#!/bin/bash
./forwarding/custom_multipath_hash.sh:#!/bin/bash
./forwarding/gre_inner_v4_multipath.sh:#!/bin/bash
./forwarding/tc_tunnel_key.sh:#!/bin/bash
./forwarding/tc_shblocks.sh:#!/bin/bash
./forwarding/router_nh.sh:#!/bin/bash
./forwarding/skbedit_priority.sh:#!/bin/bash
./forwarding/tc_mpls_l2vpn.sh:#!/bin/bash
./forwarding/gre_inner_v6_multipath.sh:#!/bin/bash
./forwarding/vxlan_symmetric.sh:#!/bin/bash
./forwarding/bridge_mdb.sh:#!/bin/bash
./forwarding/no_forwarding.sh:#!/bin/bash
./forwarding/router_bridge_1d.sh:#!/bin/bash
./forwarding/tc_flower_port_range.sh:#!/bin/bash
./forwarding/router_multicast.sh:#!/bin/bash
./forwarding/bridge_locked_port.sh:#!/bin/bash
./forwarding/vxlan_asymmetric_ipv6.sh:#!/bin/bash
./forwarding/dual_vxlan_bridge.sh:#!/bin/bash
./forwarding/bridge_port_isolation.sh:#!/bin/bash
./forwarding/local_termination.sh:#!/bin/bash
./forwarding/ipip_flat_gre_keys.sh:#!/bin/bash
./forwarding/gre_multipath_nh_res.sh:#!/bin/bash
./forwarding/gre_multipath.sh:#!/bin/bash
./forwarding/vxlan_bridge_1d_ipv6.sh:#!/bin/bash
./forwarding/ip6gre_flat_keys.sh:#!/bin/bash
./forwarding/gre_multipath_nh.sh:#!/bin/bash
./forwarding/bridge_mld.sh:#!/bin/bash
./forwarding/ip6gre_inner_v6_multipath.sh:#!/bin/bash
./forwarding/ip6gre_flat_key.sh:#!/bin/bash
./forwarding/vxlan_asymmetric.sh:#!/bin/bash
./forwarding/tc_flower_router.sh:#!/bin/bash
./forwarding/router_bridge_vlan_upper_pvid.sh:#!/bin/bash
./forwarding/mirror_gre_vlan_bridge_1q.sh:#!/bin/bash
./forwarding/q_in_vni_ipv6.sh:#!/bin/bash
./forwarding/mirror_gre_lag_lacp.sh:#!/bin/bash
./forwarding/ip6gre_custom_multipath_hash.sh:#!/bin/bash
./forwarding/vxlan_bridge_1d.sh:#!/bin/bash
./forwarding/ip6gre_hier_key.sh:#!/bin/bash
./forwarding/gre_custom_multipath_hash.sh:#!/bin/bash
./forwarding/ipip_flat_gre_key.sh:#!/bin/bash
./forwarding/mirror_gre_flower.sh:#!/bin/bash
./forwarding/router_bridge.sh:#!/bin/bash
./forwarding/vxlan_symmetric_ipv6.sh:#!/bin/bash
./forwarding/mirror_gre_bridge_1q.sh:#!/bin/bash
./forwarding/router_multipath.sh:#!/bin/bash
./forwarding/tc_vlan_modify.sh:#!/bin/bash
./forwarding/vxlan_bridge_1q.sh:#!/bin/bash
./forwarding/bridge_mdb_port_down.sh:#!/bin/bash
./forwarding/tc_flower.sh:#!/bin/bash
./forwarding/tc_flower_cfm.sh:#!/bin/bash
./forwarding/mirror_gre_neigh.sh:#!/bin/bash
./forwarding/ethtool_rmon.sh:#!/bin/bash
./forwarding/hw_stats_l3_gre.sh:#!/bin/bash
./forwarding/router.sh:#!/bin/bash
./forwarding/ipip_hier_gre_key.sh:#!/bin/bash
./forwarding/tc_police.sh:#!/bin/bash
./forwarding/pedit_ip.sh:#!/bin/bash
./forwarding/ip6_forward_instats_vrf.sh:#!/bin/bash
./forwarding/router_mpath_nh_res.sh:#!/bin/bash
./forwarding/mirror_gre_changes.sh:#!/bin/bash
./forwarding/hw_stats_l3.sh:#!/bin/bash
./forwarding/ipip_hier_gre.sh:#!/bin/bash
./forwarding/q_in_vni.sh:#!/bin/bash
./forwarding/ip6gre_flat.sh:#!/bin/bash
./forwarding/router_bridge_vlan_upper.sh:#!/bin/bash
./forwarding/bridge_igmp.sh:#!/bin/bash
./forwarding/mirror_gre_nh.sh:#!/bin/bash
./forwarding/bridge_mdb_host.sh:#!/bin/bash
./forwarding/ipip_hier_gre_keys.sh:#!/bin/bash
./forwarding/pedit_dsfield.sh:#!/bin/bash
./forwarding/bridge_vlan_mcast.sh:#!/bin/bash
./forwarding/mirror_gre_bridge_1d_vlan.sh:#!/bin/bash
./forwarding/router_bridge_1d_lag.sh:#!/bin/bash
./forwarding/router_bridge_pvid_vlan_upper.sh:#!/bin/bash
./forwarding/mirror_gre_bound.sh:#!/bin/bash
./forwarding/ip6gre_hier.sh:#!/bin/bash
./forwarding/ip6gre_hier_keys.sh:#!/bin/bash
./forwarding/ethtool_extended_state.sh:#!/bin/bash
./forwarding/router_mpath_nh.sh:#!/bin/bash
./forwarding/tc_flower_l2_miss.sh:#!/bin/bash
./forwarding/bridge_vlan_unaware.sh:#!/bin/bash
./forwarding/router_broadcast.sh:#!/bin/bash
./forwarding/bridge_fdb_learning_limit.sh:#!/bin/bash
./forwarding/ipip_lib.sh:#!/bin/bash
./forwarding/ip6gre_inner_v4_multipath.sh:#!/bin/bash
./forwarding/router_vid_1.sh:#!/bin/bash
./forwarding/mirror_gre.sh:#!/bin/bash
./forwarding/router_bridge_vlan.sh:#!/bin/bash
./forwarding/bridge_vlan_aware.sh:#!/bin/bash
./forwarding/ethtool.sh:#!/bin/bash
./forwarding/loopback.sh:#!/bin/bash
./forwarding/bridge_sticky_fdb.sh:#!/bin/bash
./forwarding/bridge_mdb_max.sh:#!/bin/bash
./forwarding/pedit_l4port.sh:#!/bin/bash
./forwarding/tc_actions.sh:#!/bin/bash
./forwarding/mirror_vlan.sh:#!/bin/bash
./forwarding/sch_red.sh:#!/bin/bash
./forwarding/ipip_flat_gre.sh:#!/bin/bash
./forwarding/mirror_gre_bridge_1d.sh:#!/bin/bash
./forwarding/lib.sh:#!/bin/bash
./forwarding/mirror_gre_vlan.sh:#!/bin/bash
./forwarding/mirror_gre_bridge_1q_lag.sh:#!/bin/bash
./forwarding/ethtool_mm.sh:#!/bin/bash
./forwarding/vxlan_bridge_1q_ipv6.sh:#!/bin/bash
./forwarding/tc_chains.sh:#!/bin/bash
./forwarding/ip6gre_lib.sh:#!/bin/bash
./fib_nexthop_nongw.sh:#!/bin/bash
./srv6_end_dt46_l3vpn_test.sh:#!/bin/bash
./cmsg_so_mark.sh:#!/bin/bash
./sctp_vrf.sh:#!/bin/bash
./fdb_flush.sh:#!/bin/bash
./ndisc_unsolicited_na_test.sh:#!/bin/bash
./traceroute.sh:#!/bin/bash
./fib-onlink-tests.sh:#!/bin/bash
./fib_tests.sh:#!/bin/bash
./cmsg_time.sh:#!/bin/bash
./arp_ndisc_untracked_subnets.sh:#!/bin/bash
./fib_nexthops.sh:#!/bin/bash

linux/tools/testing/selftests/net$ grep -rF ". lib.sh"
<-- nothing

Thanks,
Yujie

Powered by blists - more mailing lists