[Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shell scripting (use $() instead of ``)

[Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shell scripting (use $() instead of ``)

[Mao Zhongyi]

Various shell files contain a mix between obsolete `` and
modern $(); use of `` is only required when using /bin/sh
on Solaris. It would be nice to convert to using $()
everywhere, or at least in all bash scripts, as well as in
all scripts that are known to not be run on Solaris.

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 scripts/archive-source.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 4e63774f9a..62bd22578b 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -18,7 +18,7 @@ if test $# -lt 1; then
     error "Usage: $0 <output tarball>"
 fi
 
-tar_file=`realpath "$1"`
+tar_file=$(realpath "$1")
 list_file="${tar_file}.list"
 vroot_dir="${tar_file}.vroot"
 
@@ -34,7 +34,7 @@ if git diff-index --quiet HEAD -- &>/dev/null
 then
     HEAD=HEAD
 else
-    HEAD=`git stash create`
+    HEAD=$(git stash create)
 fi
 git clone --shared . "$vroot_dir"
 test $? -ne 0 && error "failed to clone into '$vroot_dir'"
-- 
2.17.1

[Qemu-devel] [PATCH 2/3] git-submodule.sh: Modern shell scripting (use $() instead of ``)

[Mao Zhongyi]

Various shell files contain a mix between obsolete `` and
modern $(); use of `` is only required when using /bin/sh
on Solaris. It would be nice to convert to using $()
everywhere, or at least in all bash scripts, as well as in
all scripts that are known to not be run on Solaris.

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 scripts/git-submodule.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index 807ca0b4f8..d452149c1a 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -59,7 +59,7 @@ status)
     fi
 
     test -f "$substat" || exit 1
-    CURSTATUS=`$GIT submodule status $modules`
+    CURSTATUS=$($GIT submodule status $modules)
     OLDSTATUS=`cat $substat`
     test "$CURSTATUS" = "$OLDSTATUS"
     exit $?
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/3] git-submodule.sh: Modern shell scripting (use $() instead of ``)

[Thomas Huth]

On 2018-10-15 08:51, Mao Zhongyi wrote:
> Various shell files contain a mix between obsolete `` and
> modern $(); use of `` is only required when using /bin/sh
> on Solaris. It would be nice to convert to using $()
> everywhere, or at least in all bash scripts, as well as in
> all scripts that are known to not be run on Solaris.
> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>  scripts/git-submodule.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> index 807ca0b4f8..d452149c1a 100755
> --- a/scripts/git-submodule.sh
> +++ b/scripts/git-submodule.sh
> @@ -59,7 +59,7 @@ status)
>      fi
>  
>      test -f "$substat" || exit 1
> -    CURSTATUS=`$GIT submodule status $modules`
> +    CURSTATUS=$($GIT submodule status $modules)
>      OLDSTATUS=`cat $substat`
>      test "$CURSTATUS" = "$OLDSTATUS"
>      exit $?
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH 3/3] show-fixed-bugs.sh: Modern shell scripting (use $() instead of ``)

[Mao Zhongyi]

Various shell files contain a mix between obsolete `` and
modern $(); use of `` is only required when using /bin/sh
on Solaris. It would be nice to convert to using $()
everywhere, or at least in all bash scripts, as well as in
all scripts that are known to not be run on Solaris.

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 scripts/show-fixed-bugs.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/show-fixed-bugs.sh b/scripts/show-fixed-bugs.sh
index 36f306898f..a095a4d6ba 100755
--- a/scripts/show-fixed-bugs.sh
+++ b/scripts/show-fixed-bugs.sh
@@ -23,10 +23,10 @@ while getopts "s:e:cbh" opt; do
 done
 
 if [ "x$start" = "x" ]; then
-    start=`git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1`
+    start=$(git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1)
 fi
 if [ "x$end" = "x" ]; then
-    end=`git tag -l  'v[0-9]*\.[0-9]*\.0' | tail -n 1`
+    end=$(git tag -l  'v[0-9]*\.[0-9]*\.0' | tail -n 1)
 fi
 
 if [ "x$start" = "x" ] || [ "x$end" = "x" ]; then
@@ -38,9 +38,9 @@ fi
 echo "Searching git log for bugs in the range $start..$end"
 
 urlstr='https://bugs.launchpad.net/\(bugs\|qemu/+bug\)/'
-bug_urls=`git log $start..$end \
+bug_urls=$(git log $start..$end \
   | sed -n '\,'"$urlstr"', s,\(.*\)\('"$urlstr"'\)\([0-9]*\).*,\2\4,p' \
-  | sort -u`
+  | sort -u)
 
 echo Found bug URLs:
 for i in $bug_urls ; do echo " $i" ; done
@@ -68,7 +68,7 @@ elif [ "x$show_in_browser" = "x1" ]; then
         bugbrowser=xdg-open
     elif command -v gnome-open >/dev/null 2>&1; then
         bugbrowser=gnome-open
-    elif [ "`uname`" = "Darwin" ]; then
+    elif [ "$(uname)" = "Darwin" ]; then
         bugbrowser=open
     elif command -v sensible-browser >/dev/null 2>&1; then
         bugbrowser=sensible-browser
-- 
2.17.1

Re: [Qemu-devel] [PATCH 3/3] show-fixed-bugs.sh: Modern shell scripting (use $() instead of ``)

[Thomas Huth]

On 2018-10-15 08:51, Mao Zhongyi wrote:
> Various shell files contain a mix between obsolete `` and
> modern $(); use of `` is only required when using /bin/sh
> on Solaris. It would be nice to convert to using $()
> everywhere, or at least in all bash scripts, as well as in
> all scripts that are known to not be run on Solaris.
> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>  scripts/show-fixed-bugs.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/show-fixed-bugs.sh b/scripts/show-fixed-bugs.sh
> index 36f306898f..a095a4d6ba 100755
> --- a/scripts/show-fixed-bugs.sh
> +++ b/scripts/show-fixed-bugs.sh
> @@ -23,10 +23,10 @@ while getopts "s:e:cbh" opt; do
>  done
>  
>  if [ "x$start" = "x" ]; then
> -    start=`git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1`
> +    start=$(git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1)
>  fi
>  if [ "x$end" = "x" ]; then
> -    end=`git tag -l  'v[0-9]*\.[0-9]*\.0' | tail -n 1`
> +    end=$(git tag -l  'v[0-9]*\.[0-9]*\.0' | tail -n 1)
>  fi
>  
>  if [ "x$start" = "x" ] || [ "x$end" = "x" ]; then
> @@ -38,9 +38,9 @@ fi
>  echo "Searching git log for bugs in the range $start..$end"
>  
>  urlstr='https://bugs.launchpad.net/\(bugs\|qemu/+bug\)/'
> -bug_urls=`git log $start..$end \
> +bug_urls=$(git log $start..$end \
>    | sed -n '\,'"$urlstr"', s,\(.*\)\('"$urlstr"'\)\([0-9]*\).*,\2\4,p' \
> -  | sort -u`
> +  | sort -u)
>  
>  echo Found bug URLs:
>  for i in $bug_urls ; do echo " $i" ; done
> @@ -68,7 +68,7 @@ elif [ "x$show_in_browser" = "x1" ]; then
>          bugbrowser=xdg-open
>      elif command -v gnome-open >/dev/null 2>&1; then
>          bugbrowser=gnome-open
> -    elif [ "`uname`" = "Darwin" ]; then
> +    elif [ "$(uname)" = "Darwin" ]; then
>          bugbrowser=open
>      elif command -v sensible-browser >/dev/null 2>&1; then
>          bugbrowser=sensible-browser
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shell scripting (use $() instead of ``)

[Thomas Huth]

On 2018-10-15 08:51, Mao Zhongyi wrote:
> Various shell files contain a mix between obsolete `` and
> modern $(); use of `` is only required when using /bin/sh
> on Solaris. It would be nice to convert to using $()
> everywhere, or at least in all bash scripts, as well as in
> all scripts that are known to not be run on Solaris.

FWIW, I think we do not have to worry about Solaris' /bin/sh here
anymore. Somebody tried to compile on Solaris a couple of weeks ago, and
found out that you need a proper POSIX-compliant shell for compiling
QEMU, so /bin/sh can not be used here anymore anyway.

> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>  scripts/archive-source.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> index 4e63774f9a..62bd22578b 100755
> --- a/scripts/archive-source.sh
> +++ b/scripts/archive-source.sh
> @@ -18,7 +18,7 @@ if test $# -lt 1; then
>      error "Usage: $0 <output tarball>"
>  fi
>  
> -tar_file=`realpath "$1"`
> +tar_file=$(realpath "$1")
>  list_file="${tar_file}.list"
>  vroot_dir="${tar_file}.vroot"
>  
> @@ -34,7 +34,7 @@ if git diff-index --quiet HEAD -- &>/dev/null
>  then
>      HEAD=HEAD
>  else
> -    HEAD=`git stash create`
> +    HEAD=$(git stash create)
>  fi
>  git clone --shared . "$vroot_dir"
>  test $? -ne 0 && error "failed to clone into '$vroot_dir'"
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shellscripting (use $() instead of ``)

[maozy]

On 10/15/18 3:07 PM, Thomas Huth wrote:
> On 2018-10-15 08:51, Mao Zhongyi wrote:
>> Various shell files contain a mix between obsolete `` and
>> modern $(); use of `` is only required when using /bin/sh
>> on Solaris. It would be nice to convert to using $()
>> everywhere, or at least in all bash scripts, as well as in
>> all scripts that are known to not be run on Solaris.
> 
> FWIW, I think we do not have to worry about Solaris' /bin/sh here
> anymore. Somebody tried to compile on Solaris a couple of weeks ago, and
> found out that you need a proper POSIX-compliant shell for compiling
> QEMU, so /bin/sh can not be used here anymore anyway.

Thanks for your quick review and clarification. I got it.
In addition, I fount that I didn't  completely replace `` in
git-submodule.sh, so I plan to resend it later.

Thanks,
Mao

> 
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>>   scripts/archive-source.sh | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
>> index 4e63774f9a..62bd22578b 100755
>> --- a/scripts/archive-source.sh
>> +++ b/scripts/archive-source.sh
>> @@ -18,7 +18,7 @@ if test $# -lt 1; then
>>       error "Usage: $0 <output tarball>"
>>   fi
>>   
>> -tar_file=`realpath "$1"`
>> +tar_file=$(realpath "$1")
>>   list_file="${tar_file}.list"
>>   vroot_dir="${tar_file}.vroot"
>>   
>> @@ -34,7 +34,7 @@ if git diff-index --quiet HEAD -- &>/dev/null
>>   then
>>       HEAD=HEAD
>>   else
>> -    HEAD=`git stash create`
>> +    HEAD=$(git stash create)
>>   fi
>>   git clone --shared . "$vroot_dir"
>>   test $? -ne 0 && error "failed to clone into '$vroot_dir'"
>>
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Re: [Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shell scripting (use $() instead of ``)

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 2018-10-15 08:51, Mao Zhongyi wrote:
>> Various shell files contain a mix between obsolete `` and
>> modern $(); use of `` is only required when using /bin/sh
>> on Solaris. It would be nice to convert to using $()
>> everywhere, or at least in all bash scripts, as well as in
>> all scripts that are known to not be run on Solaris.
>
> FWIW, I think we do not have to worry about Solaris' /bin/sh here
> anymore. Somebody tried to compile on Solaris a couple of weeks ago, and
> found out that you need a proper POSIX-compliant shell for compiling
> QEMU, so /bin/sh can not be used here anymore anyway.

Try

    SHELL=/usr/xpg4/bin/sh PATH=/usr/xpg6/bin:/usr/xpg4/bin:$PATH

to make Solaris behave.  See also
Message-ID: <878t58s2kb.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg02641.html

Re: [Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shell scripting (use $() instead of ``)

[Eric Blake]

On 10/15/18 1:51 AM, Mao Zhongyi wrote:
> Various shell files contain a mix between obsolete `` and
> modern $(); use of `` is only required when using /bin/sh
> on Solaris. It would be nice to convert to using $()
> everywhere, or at least in all bash scripts, as well as in
> all scripts that are known to not be run on Solaris.

Did you have a particular program you used to find these, or just grep?

Also, when sending a series, don't forget a 0/3 cover letter.

> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>   scripts/archive-source.sh | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/3] archive-source.sh: Modern shellscripting (use $() instead of ``)

[maozy]

Hi, Eric

On 10/15/18 11:44 PM, Eric Blake wrote:
> On 10/15/18 1:51 AM, Mao Zhongyi wrote:
>> Various shell files contain a mix between obsolete `` and
>> modern $(); use of `` is only required when using /bin/sh
>> on Solaris. It would be nice to convert to using $()
>> everywhere, or at least in all bash scripts, as well as in
>> all scripts that are known to not be run on Solaris.
> 
> Did you have a particular program you used to find these, or just grep?

Just grep in scripts dir, I also entered the file to make sure I didn't 
missing anything. In addition, I will send a separated patch to replace 
all obsolete `` in the source tree.

> 
> Also, when sending a series, don't forget a 0/3 cover letter.
>

OK, I see. :)
  Thanks,
Mao

>>
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>>   scripts/archive-source.sh | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>