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.
# selftests: net: unicast_extensions.sh
# ./unicast_extensions.sh: 31: source: not found
# ###########################################################################
# Unicast address extensions tests (behavior of reserved IPv4 addresses)
# ###########################################################################
# TEST: assign and ping within 240/4 (1 of 2) (is allowed) [FAIL]
# TEST: assign and ping within 240/4 (2 of 2) (is allowed) [FAIL]
# TEST: assign and ping within 0/8 (1 of 2) (is allowed) [FAIL]
# TEST: assign and ping within 0/8 (2 of 2) (is allowed) [FAIL]
# TEST: assign and ping inside 255.255/16 (is allowed) [FAIL]
# TEST: assign and ping inside 255.255.255/24 (is allowed) [FAIL]
# TEST: route between 240.5.6/24 and 255.1.2/24 (is allowed) [FAIL]
# TEST: route between 0.200/16 and 245.99/16 (is allowed) [FAIL]
# TEST: assign and ping lowest address (/24) [FAIL]
# TEST: assign and ping lowest address (/26) [FAIL]
# TEST: routing using lowest address [FAIL]
# TEST: assigning 0.0.0.0 (is forbidden) [ OK ]
# TEST: assigning 255.255.255.255 (is forbidden) [ OK ]
# TEST: assign and ping inside 127/8 (is forbidden) [ OK ]
# TEST: assign and ping class D address (is forbidden) [ OK ]
# TEST: routing using class D (is forbidden) [ OK ]
# TEST: routing using 127/8 (is forbidden) [ OK ]
not ok 51 selftests: net: unicast_extensions.sh # exit=1
v1 -> v2:
- Fix pmtu.sh which has the same issue as unicast_extensions.sh,
suggested by Hangbin
- Change the style of the "source" line to be consistent with other
tests, suggested by Hangbin
Link: https://lore.kernel.org/all/[email protected]/ [1]
Link: https://lore.kernel.org/all/[email protected]/ [2]
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Yujie Liu <[email protected]>
---
tools/testing/selftests/net/pmtu.sh | 4 ++--
tools/testing/selftests/net/unicast_extensions.sh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 175d3d1d773b..f10879788f61 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
#
# Check that route PMTU values match expectations, and that initial device MTU
@@ -198,7 +198,7 @@
# - pmtu_ipv6_route_change
# Same as above but with IPv6
-source ./lib.sh
+source lib.sh
PAUSE_ON_FAIL=no
VERBOSE=0
diff --git a/tools/testing/selftests/net/unicast_extensions.sh b/tools/testing/selftests/net/unicast_extensions.sh
index b7a2cb9e7477..f52aa5f7da52 100755
--- a/tools/testing/selftests/net/unicast_extensions.sh
+++ b/tools/testing/selftests/net/unicast_extensions.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
#
# By Seth Schoen (c) 2021, for the IPv4 Unicast Extensions Project
@@ -28,7 +28,7 @@
# These tests provide an easy way to flip the expected result of any
# of these behaviors for testing kernel patches that change them.
-source ./lib.sh
+source lib.sh
# nettest can be run from PATH or from same directory as this selftest
if ! which nettest >/dev/null; then
base-commit: cd4d7263d58ab98fd4dee876776e4da6c328faa3
--
2.34.1
On 12/29/23 14:19, 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.
>
(snip)
>
> Link: https://lore.kernel.org/all/[email protected]/ [1]
> Link: https://lore.kernel.org/all/[email protected]/ [2]
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Yujie Liu <[email protected]>
I would recommend use of shellcheck in the future, it will catch this
particular bug, with following warning:
SC3046: In POSIX sh, 'source' in place of '.' is undefined.
Being specific, and requiring bash looks fine for me.
Reviewed-by: Przemek Kitszel <[email protected]>
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.
>
> # selftests: net: unicast_extensions.sh
> # ./unicast_extensions.sh: 31: source: not found
> # ###########################################################################
> # Unicast address extensions tests (behavior of reserved IPv4 addresses)
> # ###########################################################################
> # TEST: assign and ping within 240/4 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 240/4 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255/16 (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255.255/24 (is allowed) [FAIL]
> # TEST: route between 240.5.6/24 and 255.1.2/24 (is allowed) [FAIL]
> # TEST: route between 0.200/16 and 245.99/16 (is allowed) [FAIL]
> # TEST: assign and ping lowest address (/24) [FAIL]
> # TEST: assign and ping lowest address (/26) [FAIL]
> # TEST: routing using lowest address [FAIL]
> # TEST: assigning 0.0.0.0 (is forbidden) [ OK ]
> # TEST: assigning 255.255.255.255 (is forbidden) [ OK ]
> # TEST: assign and ping inside 127/8 (is forbidden) [ OK ]
> # TEST: assign and ping class D address (is forbidden) [ OK ]
> # TEST: routing using class D (is forbidden) [ OK ]
> # TEST: routing using 127/8 (is forbidden) [ OK ]
> not ok 51 selftests: net: unicast_extensions.sh # exit=1
>
> v1 -> v2:
> - Fix pmtu.sh which has the same issue as unicast_extensions.sh,
> suggested by Hangbin
> - Change the style of the "source" line to be consistent with other
> tests, suggested by Hangbin
>
> Link: https://lore.kernel.org/all/[email protected]/ [1]
> Link: https://lore.kernel.org/all/[email protected]/ [2]
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Yujie Liu <[email protected]>
> ---
> tools/testing/selftests/net/pmtu.sh | 4 ++--
> tools/testing/selftests/net/unicast_extensions.sh | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 175d3d1d773b..f10879788f61 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # Check that route PMTU values match expectations, and that initial device MTU
> @@ -198,7 +198,7 @@
> # - pmtu_ipv6_route_change
> # Same as above but with IPv6
>
> -source ./lib.sh
> +source lib.sh
>
> PAUSE_ON_FAIL=no
> VERBOSE=0
> diff --git a/tools/testing/selftests/net/unicast_extensions.sh b/tools/testing/selftests/net/unicast_extensions.sh
> index b7a2cb9e7477..f52aa5f7da52 100755
> --- a/tools/testing/selftests/net/unicast_extensions.sh
> +++ b/tools/testing/selftests/net/unicast_extensions.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # By Seth Schoen (c) 2021, for the IPv4 Unicast Extensions Project
> @@ -28,7 +28,7 @@
> # These tests provide an easy way to flip the expected result of any
> # of these behaviors for testing kernel patches that change them.
>
> -source ./lib.sh
> +source lib.sh
>
> # nettest can be run from PATH or from same directory as this selftest
> if ! which nettest >/dev/null; then
>
> base-commit: cd4d7263d58ab98fd4dee876776e4da6c328faa3
> --
> 2.34.1
>
Reviewed-by: Hangbin Liu <[email protected]>
On 12/29/23 6:19 PM, 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.
>
> # selftests: net: unicast_extensions.sh
> # ./unicast_extensions.sh: 31: source: not found
> # ###########################################################################
> # Unicast address extensions tests (behavior of reserved IPv4 addresses)
> # ###########################################################################
> # TEST: assign and ping within 240/4 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 240/4 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255/16 (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255.255/24 (is allowed) [FAIL]
> # TEST: route between 240.5.6/24 and 255.1.2/24 (is allowed) [FAIL]
> # TEST: route between 0.200/16 and 245.99/16 (is allowed) [FAIL]
> # TEST: assign and ping lowest address (/24) [FAIL]
> # TEST: assign and ping lowest address (/26) [FAIL]
> # TEST: routing using lowest address [FAIL]
> # TEST: assigning 0.0.0.0 (is forbidden) [ OK ]
> # TEST: assigning 255.255.255.255 (is forbidden) [ OK ]
> # TEST: assign and ping inside 127/8 (is forbidden) [ OK ]
> # TEST: assign and ping class D address (is forbidden) [ OK ]
> # TEST: routing using class D (is forbidden) [ OK ]
> # TEST: routing using 127/8 (is forbidden) [ OK ]
> not ok 51 selftests: net: unicast_extensions.sh # exit=1
>
> v1 -> v2:
> - Fix pmtu.sh which has the same issue as unicast_extensions.sh,
> suggested by Hangbin
> - Change the style of the "source" line to be consistent with other
> tests, suggested by Hangbin
>
> Link: https://lore.kernel.org/all/[email protected]/ [1]
> Link: https://lore.kernel.org/all/[email protected]/ [2]
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Yujie Liu <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>
> ---
> tools/testing/selftests/net/pmtu.sh | 4 ++--
> tools/testing/selftests/net/unicast_extensions.sh | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 175d3d1d773b..f10879788f61 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # Check that route PMTU values match expectations, and that initial device MTU
> @@ -198,7 +198,7 @@
> # - pmtu_ipv6_route_change
> # Same as above but with IPv6
>
> -source ./lib.sh
> +source lib.sh
>
> PAUSE_ON_FAIL=no
> VERBOSE=0
> diff --git a/tools/testing/selftests/net/unicast_extensions.sh b/tools/testing/selftests/net/unicast_extensions.sh
> index b7a2cb9e7477..f52aa5f7da52 100755
> --- a/tools/testing/selftests/net/unicast_extensions.sh
> +++ b/tools/testing/selftests/net/unicast_extensions.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # By Seth Schoen (c) 2021, for the IPv4 Unicast Extensions Project
> @@ -28,7 +28,7 @@
> # These tests provide an easy way to flip the expected result of any
> # of these behaviors for testing kernel patches that change them.
>
> -source ./lib.sh
> +source lib.sh
>
> # nettest can be run from PATH or from same directory as this selftest
> if ! which nettest >/dev/null; then
>
> base-commit: cd4d7263d58ab98fd4dee876776e4da6c328faa3
--
BR,
Muhammad Usama Anjum
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).
> # selftests: net: unicast_extensions.sh
> # ./unicast_extensions.sh: 31: source: not found
> # ###########################################################################
> # Unicast address extensions tests (behavior of reserved IPv4 addresses)
> # ###########################################################################
> # TEST: assign and ping within 240/4 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 240/4 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255/16 (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255.255/24 (is allowed) [FAIL]
> # TEST: route between 240.5.6/24 and 255.1.2/24 (is allowed) [FAIL]
> # TEST: route between 0.200/16 and 245.99/16 (is allowed) [FAIL]
> # TEST: assign and ping lowest address (/24) [FAIL]
> # TEST: assign and ping lowest address (/26) [FAIL]
> # TEST: routing using lowest address [FAIL]
> # TEST: assigning 0.0.0.0 (is forbidden) [ OK ]
> # TEST: assigning 255.255.255.255 (is forbidden) [ OK ]
> # TEST: assign and ping inside 127/8 (is forbidden) [ OK ]
> # TEST: assign and ping class D address (is forbidden) [ OK ]
> # TEST: routing using class D (is forbidden) [ OK ]
> # TEST: routing using 127/8 (is forbidden) [ OK ]
> not ok 51 selftests: net: unicast_extensions.sh # exit=1
>
> v1 -> v2:
> - Fix pmtu.sh which has the same issue as unicast_extensions.sh,
> suggested by Hangbin
> - Change the style of the "source" line to be consistent with other
> tests, suggested by Hangbin
>
> Link: https://lore.kernel.org/all/[email protected]/ [1]
> Link: https://lore.kernel.org/all/[email protected]/ [2]
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Yujie Liu <[email protected]>
> ---
> tools/testing/selftests/net/pmtu.sh | 4 ++--
> tools/testing/selftests/net/unicast_extensions.sh | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 175d3d1d773b..f10879788f61 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # Check that route PMTU values match expectations, and that initial device MTU
> @@ -198,7 +198,7 @@
> # - pmtu_ipv6_route_change
> # Same as above but with IPv6
>
> -source ./lib.sh
> +source lib.sh
>
> PAUSE_ON_FAIL=no
> VERBOSE=0
> diff --git a/tools/testing/selftests/net/unicast_extensions.sh b/tools/testing/selftests/net/unicast_extensions.sh
> index b7a2cb9e7477..f52aa5f7da52 100755
> --- a/tools/testing/selftests/net/unicast_extensions.sh
> +++ b/tools/testing/selftests/net/unicast_extensions.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> #
> # By Seth Schoen (c) 2021, for the IPv4 Unicast Extensions Project
> @@ -28,7 +28,7 @@
> # These tests provide an easy way to flip the expected result of any
> # of these behaviors for testing kernel patches that change them.
>
> -source ./lib.sh
> +source lib.sh
>
> # nettest can be run from PATH or from same directory as this selftest
> if ! which nettest >/dev/null; then
>
> base-commit: cd4d7263d58ab98fd4dee876776e4da6c328faa3
> --
> 2.34.1
>
>
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.
>
> # selftests: net: unicast_extensions.sh
> # ./unicast_extensions.sh: 31: source: not found
> # ###########################################################################
> # Unicast address extensions tests (behavior of reserved IPv4 addresses)
> # ###########################################################################
> # TEST: assign and ping within 240/4 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 240/4 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (1 of 2) (is allowed) [FAIL]
> # TEST: assign and ping within 0/8 (2 of 2) (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255/16 (is allowed) [FAIL]
> # TEST: assign and ping inside 255.255.255/24 (is allowed) [FAIL]
> # TEST: route between 240.5.6/24 and 255.1.2/24 (is allowed) [FAIL]
> # TEST: route between 0.200/16 and 245.99/16 (is allowed) [FAIL]
> # TEST: assign and ping lowest address (/24) [FAIL]
> # TEST: assign and ping lowest address (/26) [FAIL]
> # TEST: routing using lowest address [FAIL]
> # TEST: assigning 0.0.0.0 (is forbidden) [ OK ]
> # TEST: assigning 255.255.255.255 (is forbidden) [ OK ]
> # TEST: assign and ping inside 127/8 (is forbidden) [ OK ]
> # TEST: assign and ping class D address (is forbidden) [ OK ]
> # TEST: routing using class D (is forbidden) [ OK ]
> # TEST: routing using 127/8 (is forbidden) [ OK ]
> not ok 51 selftests: net: unicast_extensions.sh # exit=1
>
> v1 -> v2:
> - Fix pmtu.sh which has the same issue as unicast_extensions.sh,
> suggested by Hangbin
> - Change the style of the "source" line to be consistent with other
> tests, suggested by Hangbin
>
> Link: https://lore.kernel.org/all/[email protected]/ [1]
> Link: https://lore.kernel.org/all/[email protected]/ [2]
Also, please add the missing Fixes tags.
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Yujie Liu <[email protected]>
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. As this is related with a large patch series from
Hangbin, and other scripts use "source" during the conversion, so we may
need some input from Hangbin.
Hi Hangbin,
Could you please share your comments on this? Would you like to replace
"source" with "." for these two specific scripts as Guillaume suggested,
or change the shebang from "sh" to "bash"?
Best Regards,
Yujie
On Tue, Jan 02, 2024 at 01:51:56PM +0800, Yujie Liu wrote:
> > 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. As this is related with a large patch series from
> Hangbin, and other scripts use "source" during the conversion, so we may
> need some input from Hangbin.
>
> Hi Hangbin,
>
> Could you please share your comments on this? Would you like to replace
> "source" with "." for these two specific scripts as Guillaume suggested,
> or change the shebang from "sh" to "bash"?
Both works for me :)
Thanks
Hangbin
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
On Tue, Jan 02, 2024 at 04:20:16PM +0800, Yujie Liu wrote:
> 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? :)
The use of "source" instead of "." is clearly an overlook. Consistency
is desirable only when it brings better quality code.
And it should be easy enough to convert the remaining "source lib.sh"
in a followup patch to make the other shell scripts consistent.
> 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
>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:
On Fri, 29 Dec 2023 21:19:31 +0800 you 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.
>
> [...]
Here is the summary with links:
- [v2,net-next] selftests/net: change shebang to bash to support "source"
https://git.kernel.org/netdev/net-next/c/05d92cb0e919
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html