bash: substitute legacy `` with modern $()

openvpn is also built on many non bash systems. what about them?

24 авг. 2017 г. 18:34 пользователь "Antonio Quartulli" <[email protected]....> написал:

The backquotes for command substitution in bash are considered old-style in favour of the more modern $() [1]. Substitute them.

[1]https://www.gnu.org/software/bash/manual/html_node/Command-Substitution. html#Command-Substitution

Signed-off-by Antonio Quartulli <[email protected]...>

---

note: I did not really know how to test this patch.

 dev-tools/gen-release-tarballs.sh | 10 +++++-----
 tests/t_cltsrv.sh                 |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

dev-tools/gen-release- renamed from dev-tools/gen-release-tarballs.sh +0 -0

tarballs.sh
index f9c620e3..550e5cd2 100755

dev-tools/gen-release-tarballs.sh +5 -7

@@ -49,7 +49,7 @@ if [ $? -ne 0 ]; then
 fi
 
 # Extract the git URL
-giturl="`git remote get-url $arg_remote_name 2>/dev/null`"
+giturl="$(git remote get-url $arg_remote_name 2>/dev/null)"
 if [ $? -ne 0 ]; then
     echo "** ERROR ** Invalid git remote name: $arg_remote_name"
     exit 2

@@ -71,7 +71,7 @@ get_filename()
 {
     local wildcard="$1"
 
-    res="`find . -maxdepth 1 -type f -name \"$wildcard\" | head -n1 | cut
-d/ -f2-`"
+    res="$(find . -maxdepth 1 -type f -name \"$wildcard\" | head -n1 | cut
-d/ -f2-)"
     if [ $? -ne 0 ]; then
         echo "-- 'find' failed."
         exit 5

@@ -88,7 +88,7 @@ copy_files()
     local fileext="$1"
     local dest="$2"
 
-    file="`get_filename openvpn-*.*.*.$fileext`"
+    file="$(get_filename openvpn-*.*.*.$fileext)"
     if [ -z "$file" ]; then
         echo "** ERROR Failed to find source file"
         exit 5

@@ -106,7 +106,7 @@ sign_file()
     local signkey="$1"
     local srchfile="$2"
     local signtype="$3"
-    local file="`get_filename $srchfile`"
+    local file="$(get_filename $srchfile)"
 
     echo "-- Signing $file ..."
     case "$signtype" in

@@ -169,7 +169,7 @@ fi
 #
 
 # Clone the remote repository
-workdir="`mktemp -d -p /var/tmp openvpn-build-release-XXXXXX`"
+workdir="$(mktemp -d -p /var/tmp openvpn-build-release-XXXXXX)"
 cd $workdir
 echo "-- Working directory: $workdir"
 echo "-- git clone $giturl"

tests/t_cltsrv.sh +3 -3

index 752251e4..1ab3db3e 100755
@@ -25,14 +25,14 @@ top_builddir="${top_builddir:-..}"
 trap "rm -f log.$$ log.$$.signal ; trap 0 ; exit 77" 1 2 15
 trap "rm -f log.$$ log.$$.signal ; exit 1" 0 3
 addopts=
-case `uname -s` in
+case $(uname -s) in
     FreeBSD)
     # FreeBSD jails map the outgoing IP to the jail IP - we need to
     # allow the real IP unless we want the test to run forever.
-    if test "`sysctl 2>/dev/null -n security.jail.jailed`" = 1 \
+    if test "$(sysctl 2>/dev/null -n security.jail.jailed)" = 1 \
     || ps -ostate= -p $$ | grep -q J; then
        addopts="--float"
-       if test "x`ifconfig | grep inet`" = x ; then
+       if test "x$(ifconfig | grep inet)" = x ; then
            echo "###"
            echo "### To run the test in a FreeBSD jail, you MUST add an IP

alias for the jail's IP."

            echo "###"

--
2.14.1

------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

dev-tools/gen-release-tarballs.sh is only for devs, while tests/t_cltsrv.sh is for running some tests, but I am not sure sure how the latter would interact with non-linux systems.

Maybe Gert knows(?)

Quoted 98 lines

openvpn is also built on many non bash systems. what about them?

24 авг. 2017 г. 18:34 пользователь "Antonio Quartulli" <[email protected]....> написал:

The backquotes for command substitution in bash are considered old-style in favour of the more modern $() [1]. Substitute them.

[1]https://www.gnu.org/software/bash/manual/html_node/Command-Substitution. html#Command-Substitution

Signed-off-by Antonio Quartulli <[email protected]...>

---

note: I did not really know how to test this patch.

 dev-tools/gen-release-tarballs.sh | 10 +++++-----
 tests/t_cltsrv.sh                 |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

dev-tools/gen-release- renamed from dev-tools/gen-release-tarballs.sh +0 -0

tarballs.sh
index f9c620e3..550e5cd2 100755

dev-tools/gen-release-tarballs.sh +5 -7

@@ -49,7 +49,7 @@ if [ $? -ne 0 ]; then
 fi
 
 # Extract the git URL
-giturl="`git remote get-url $arg_remote_name 2>/dev/null`"
+giturl="$(git remote get-url $arg_remote_name 2>/dev/null)"
 if [ $? -ne 0 ]; then
     echo "** ERROR ** Invalid git remote name: $arg_remote_name"
     exit 2

@@ -71,7 +71,7 @@ get_filename()
 {
     local wildcard="$1"
 
-    res="`find . -maxdepth 1 -type f -name \"$wildcard\" | head -n1 | cut
-d/ -f2-`"
+    res="$(find . -maxdepth 1 -type f -name \"$wildcard\" | head -n1 | cut
-d/ -f2-)"
     if [ $? -ne 0 ]; then
         echo "-- 'find' failed."
         exit 5

@@ -88,7 +88,7 @@ copy_files()
     local fileext="$1"
     local dest="$2"
 
-    file="`get_filename openvpn-*.*.*.$fileext`"
+    file="$(get_filename openvpn-*.*.*.$fileext)"
     if [ -z "$file" ]; then
         echo "** ERROR Failed to find source file"
         exit 5

@@ -106,7 +106,7 @@ sign_file()
     local signkey="$1"
     local srchfile="$2"
     local signtype="$3"
-    local file="`get_filename $srchfile`"
+    local file="$(get_filename $srchfile)"
 
     echo "-- Signing $file ..."
     case "$signtype" in

@@ -169,7 +169,7 @@ fi
 #
 
 # Clone the remote repository
-workdir="`mktemp -d -p /var/tmp openvpn-build-release-XXXXXX`"
+workdir="$(mktemp -d -p /var/tmp openvpn-build-release-XXXXXX)"
 cd $workdir
 echo "-- Working directory: $workdir"
 echo "-- git clone $giturl"

tests/t_cltsrv.sh +3 -3

index 752251e4..1ab3db3e 100755
@@ -25,14 +25,14 @@ top_builddir="${top_builddir:-..}"
 trap "rm -f log.$$ log.$$.signal ; trap 0 ; exit 77" 1 2 15
 trap "rm -f log.$$ log.$$.signal ; exit 1" 0 3
 addopts=
-case `uname -s` in
+case $(uname -s) in
     FreeBSD)
     # FreeBSD jails map the outgoing IP to the jail IP - we need to
     # allow the real IP unless we want the test to run forever.
-    if test "`sysctl 2>/dev/null -n security.jail.jailed`" = 1 \
+    if test "$(sysctl 2>/dev/null -n security.jail.jailed)" = 1 \
     || ps -ostate= -p $$ | grep -q J; then
        addopts="--float"
-       if test "x`ifconfig | grep inet`" = x ; then
+       if test "x$(ifconfig | grep inet)" = x ; then
            echo "###"
            echo "### To run the test in a FreeBSD jail, you MUST add an IP

alias for the jail's IP."

            echo "###"

--
2.14.1

Quoted 5 lines · Antonio Quartulli

dev-tools/gen-release-tarballs.sh is only for devs, while tests/t_cltsrv.sh is for running some tests, but I am not sure sure how the latter would interact with non-linux systems.

Maybe Gert knows(?)

Tried running them through ksh or dash? Those are the most feature restrictive shells I can think of right now. Dash is supposed to be the most POSIX compliant shell, iirc.

The backquotes for command substitution in bash are considered old-style in favour of the more modern $() [1]. Substitute them.

[1]https://www.gnu.org/software/bash/manual/html_node/Command-Substitution.html#Command-Substitution

The backquotes for command substitution in bash are considered old-style in favour of the more modern $() [1]. Substitute them.

[1]https://www.gnu.org/software/bash/manual/html_node/Command-Substitution.html#Command-Substitution

Signed-off-by Antonio Quartulli <[email protected]...>

---

v2: added overlooked occurrence

note: I did not really know how to test this patch.

(gen-release-tarballs.sh only needs to work on FreeBSD and Linux, and FreeBSD's /bin/sh is sufficiently modern so so it's likely to work
- but the test scripts need to run robustly everywhere a user builds,
so never assume "because bash says so!" is a way anywhere but into worlds of pain. And yes, we've been there before :-) )

Quoted 5 lines · Gert Doering

(gen-release-tarballs.sh only needs to work on FreeBSD and Linux, and FreeBSD's /bin/sh is sufficiently modern so so it's likely to work
- but the test scripts need to run robustly everywhere a user builds,
so never assume "because bash says so!" is a way anywhere but into worlds of pain. And yes, we've been there before :-) )

And to avoid style confusions ... as long as we can avoid having "one style" on the test scripts and "another style" on the dev-tools scripts, that will be easier to review and maintain.

We can have more slack in dev-tools, but if we deviate, then we need to properly document it so we won't forget why.

Quoted 10 lines · David Sommerseth

Quoted 5 lines · Gert Doering

(gen-release-tarballs.sh only needs to work on FreeBSD and Linux, and FreeBSD's /bin/sh is sufficiently modern so so it's likely to work
- but the test scripts need to run robustly everywhere a user builds,
so never assume "because bash says so!" is a way anywhere but into worlds of pain. And yes, we've been there before :-) )

And to avoid style confusions ... as long as we can avoid having "one style" on the test scripts and "another style" on the dev-tools scripts, that will be easier to review and maintain.

Quoted 2 lines

We can have more slack in dev-tools, but if we deviate, then we need to properly document it so we won't forget why.

I also think it is a bad idea to have different styles. If we have to agree on something, it should be valid for every file of that type.

Let's postpone this for when we'll have tested various platforms.

my biggest complaint about the so-called modern $() notation is that it clashes/confuses the heck out of me when using proper variable usage, e.g. I find
output=${SOME_COMMAND} ${SOME_ARGS}
much better readable than
output=$(${SOME_COMMAND} ${SOME_ARGS})

especially since you are allowed to **NEST** $() strings, IIRC. Things like

   output=$(${cmd} $(${cmd2}))

work, and in my view, thank god that you cannot do that using backtics ;)