[Buildroot] [PATCH v2 2/3] board/qemu/start-qemu.sh.in: rework argument handling

[Buildroot] [PATCH v2 2/3] board/qemu/start-qemu.sh.in: rework argument handling

[James Knight]

Provides the ability to forward command line options directly to QEMU.
When invoking `start-qemu.sh`, users can forward arguments by adding a
double dash (`--`) into the argument set, and any trailing arguments
will be forwarded into QEMU. For example, `start-qemu.sh -- --help`.

The original implementation supported a "serial-only" command line
argument to help run in a non-graphical mode for some use cases. These
changes try to promote a newly added `--serial-only` argument to drive
this mode; that being said, a `serial-only` argument will still be
accepted for backwards compatibility.

Since this script is primarily for CI-related testing, a warning is
generated for users if any unsupported options are provided.

Signed-off-by: James Knight <james.d.knight@live.com>
---
Changes v1 -> v2:
  - From the original patch, this second patch now only contains changes
    related to argument processing.
  - Add unsupported warning with custom options (suggested by Romain)
  - Ensure sh compliant in the argument parsing (while condition).
  - Properly handle legacy `serial-only` check in argument processing by
    not cycling just on dash-prefixed arguments.
---
 board/qemu/start-qemu.sh.in | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/board/qemu/start-qemu.sh.in b/board/qemu/start-qemu.sh.in
index c2d77734c7a6b318a5f7adedfd9b0b5875e84f59..dd387507f8867778ec73f49d769679dcd0956fd4 100644
--- a/board/qemu/start-qemu.sh.in
+++ b/board/qemu/start-qemu.sh.in
@@ -3,12 +3,25 @@
 BINARIES_DIR="${0%/*}/"
 cd ${BINARIES_DIR}
 
-if [ "${1}" = "serial-only" ]; then
+mode_serial=0
+while [ "$1" ]; do
+    case "$1" in
+    --serial-only|serial-only) mode_serial=1; shift;;
+    --) shift; break;;
+    *) echo "unknown option: $1" 1>&2; exit 1;;
+    esac
+done
+
+if [ $mode_serial -eq 1 ]; then
     EXTRA_ARGS='VAR_SERIAL_ARGS'
 else
     EXTRA_ARGS='VAR_DEFAULT_ARGS'
 fi
 
+if [ "$*" ]; then
+    echo "(warning) unsupported options: $*"
+fi
+
 export PATH="${HOST_DIR}/bin:${PATH}"
-exec VAR_QEMU_CMD_LINE ${EXTRA_ARGS}
+exec VAR_QEMU_CMD_LINE ${EXTRA_ARGS} "$@"
 )
-- 
2.39.1.windows.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 2/3] board/qemu/start-qemu.sh.in: rework argument handling

[Yann E. MORIN]

James, All,

On 2023-04-07 01:21 -0400, James Knight spake thusly:
> Provides the ability to forward command line options directly to QEMU.
> When invoking `start-qemu.sh`, users can forward arguments by adding a
> double dash (`--`) into the argument set, and any trailing arguments
> will be forwarded into QEMU. For example, `start-qemu.sh -- --help`.
> 
> The original implementation supported a "serial-only" command line
> argument to help run in a non-graphical mode for some use cases. These
> changes try to promote a newly added `--serial-only` argument to drive
> this mode; that being said, a `serial-only` argument will still be
> accepted for backwards compatibility.
> 
> Since this script is primarily for CI-related testing, a warning is
> generated for users if any unsupported options are provided.
> 
> Signed-off-by: James Knight <james.d.knight@live.com>
> ---
> Changes v1 -> v2:
>   - From the original patch, this second patch now only contains changes
>     related to argument processing.
>   - Add unsupported warning with custom options (suggested by Romain)
>   - Ensure sh compliant in the argument parsing (while condition).
>   - Properly handle legacy `serial-only` check in argument processing by
>     not cycling just on dash-prefixed arguments.
> ---
>  board/qemu/start-qemu.sh.in | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/board/qemu/start-qemu.sh.in b/board/qemu/start-qemu.sh.in
> index c2d77734c7a6b318a5f7adedfd9b0b5875e84f59..dd387507f8867778ec73f49d769679dcd0956fd4 100644
> --- a/board/qemu/start-qemu.sh.in
> +++ b/board/qemu/start-qemu.sh.in
> @@ -3,12 +3,25 @@
>  BINARIES_DIR="${0%/*}/"
>  cd ${BINARIES_DIR}
>  
> -if [ "${1}" = "serial-only" ]; then
> +mode_serial=0
> +while [ "$1" ]; do
> +    case "$1" in
> +    --serial-only|serial-only) mode_serial=1; shift;;

I like to use false/true for booleans, first because this is exactly
what mode_serial is, and second because we can directly call them to
test truthness...

> +    --) shift; break;;
> +    *) echo "unknown option: $1" 1>&2; exit 1;;
> +    esac
> +done
> +
> +if [ $mode_serial -eq 1 ]; then

... here, as:

    if ${mode_serial}; then
        ...

>      EXTRA_ARGS='VAR_SERIAL_ARGS'
>  else
>      EXTRA_ARGS='VAR_DEFAULT_ARGS'
>  fi
>  
> +if [ "$*" ]; then
> +    echo "(warning) unsupported options: $*"
> +fi

This warning is not going to be usefull, is it? If the user does pass
extra variables for qemu, $* will not be empty, and this is expected.

OTOH, if there were options the script did not recognise for itself, it
would have already errored out becasue of 'exit 1' earlier.

So, I dropped the warning.

Applied to master, thanks.

Regards,
Yann E. MORIN.

>  export PATH="${HOST_DIR}/bin:${PATH}"
> -exec VAR_QEMU_CMD_LINE ${EXTRA_ARGS}
> +exec VAR_QEMU_CMD_LINE ${EXTRA_ARGS} "$@"
>  )
> -- 
> 2.39.1.windows.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 3/3] board/qemu/start-qemu.sh.in: support launching with host system's qemu

[James Knight]

Provides the ability to use a host system's QEMU. While a Buildroot
generated QEMU should work for most cases, a developer may wish to use
the system's QEMU for options which may not have been configured in the
Buildroot's QEMU build (e.g. configuring a different display mode).

Signed-off-by: James Knight <james.d.knight@live.com>
---
Changes v1 -> v2:
  - From the original patch, this third patch now only contains changes
    related to permitting a system QEMU usage.
  - Change from `--host` to `--use-system-qemu` (suggested by Romain).
---
 board/qemu/start-qemu.sh.in | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/board/qemu/start-qemu.sh.in b/board/qemu/start-qemu.sh.in
index dd387507f8867778ec73f49d769679dcd0956fd4..51f15b3ecdfdc8151caeae90582ad1142d122e69 100644
--- a/board/qemu/start-qemu.sh.in
+++ b/board/qemu/start-qemu.sh.in
@@ -4,9 +4,11 @@ BINARIES_DIR="${0%/*}/"
 cd ${BINARIES_DIR}
 
 mode_serial=0
+mode_sys_qemu=0
 while [ "$1" ]; do
     case "$1" in
     --serial-only|serial-only) mode_serial=1; shift;;
+    --use-system-qemu) mode_sys_qemu=1; shift;;
     --) shift; break;;
     *) echo "unknown option: $1" 1>&2; exit 1;;
     esac
@@ -22,6 +24,9 @@ if [ "$*" ]; then
     echo "(warning) unsupported options: $*"
 fi
 
-export PATH="${HOST_DIR}/bin:${PATH}"
+if [ $mode_sys_qemu -ne 1 ]; then
+    export PATH="${HOST_DIR}/bin:${PATH}"
+fi
+
 exec VAR_QEMU_CMD_LINE ${EXTRA_ARGS} "$@"
 )
-- 
2.39.1.windows.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 3/3] board/qemu/start-qemu.sh.in: support launching with host system's qemu

[Yann E. MORIN]

James, All,

On 2023-04-07 01:21 -0400, James Knight spake thusly:
> Provides the ability to use a host system's QEMU. While a Buildroot
> generated QEMU should work for most cases, a developer may wish to use
> the system's QEMU for options which may not have been configured in the
> Buildroot's QEMU build (e.g. configuring a different display mode).
> 
> Signed-off-by: James Knight <james.d.knight@live.com>
> ---
> Changes v1 -> v2:
>   - From the original patch, this third patch now only contains changes
>     related to permitting a system QEMU usage.
>   - Change from `--host` to `--use-system-qemu` (suggested by Romain).
> ---
>  board/qemu/start-qemu.sh.in | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/board/qemu/start-qemu.sh.in b/board/qemu/start-qemu.sh.in
> index dd387507f8867778ec73f49d769679dcd0956fd4..51f15b3ecdfdc8151caeae90582ad1142d122e69 100644
> --- a/board/qemu/start-qemu.sh.in
> +++ b/board/qemu/start-qemu.sh.in
> @@ -4,9 +4,11 @@ BINARIES_DIR="${0%/*}/"
>  cd ${BINARIES_DIR}
>  
>  mode_serial=0
> +mode_sys_qemu=0
>  while [ "$1" ]; do
>      case "$1" in
>      --serial-only|serial-only) mode_serial=1; shift;;
> +    --use-system-qemu) mode_sys_qemu=1; shift;;

I also swotched to false/true here...

>      --) shift; break;;
>      *) echo "unknown option: $1" 1>&2; exit 1;;
>      esac
> @@ -22,6 +24,9 @@ if [ "$*" ]; then
>      echo "(warning) unsupported options: $*"
>  fi
>  
> -export PATH="${HOST_DIR}/bin:${PATH}"
> +if [ $mode_sys_qemu -ne 1 ]; then

... and here.

Applied to master, thanks.

Regards,
Yann E. MORIN.

> +    export PATH="${HOST_DIR}/bin:${PATH}"
> +fi
> +
>  exec VAR_QEMU_CMD_LINE ${EXTRA_ARGS} "$@"
>  )
> -- 
> 2.39.1.windows.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot