Re: [PATCH 1/2] copy-firmware: make script smarter about parameters

[Mario Limonciello <superm1@kernel.org>]

On 3/17/2025 14:16, Timur Tabi wrote:
> Several improvements to copy-firmware.sh that make it more friendly
> when passed unknown or not exactly correct command-line parameters.
> 
> 1) Add a usage() function to show the command-line options.
> 2) Print that usage on all errors.
> 3) Don't fail with a weird error if there's a space between -j and
>     the number.
> 4) Add support for the -h or --help options.
> 5) Ignore any command-line unsupported parameters that start with
>     a dash.  This is necessary because otherwise the script will
>     assume the option is actually a destination directory, and then
>     the "test" command will get confused.  Drawback is that we don't
>     support any more destination directories that start with a dash,
>     but no one does that.
> 
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>   copy-firmware.sh | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)

/Conceptually/ I'm aligned on everything in this patch.

A few things.

1) One patch per change please.
2) It appears that the second patch in this this series fails CI.  You 
should install pre-commit or run `make check` to check before submitting 
in the future.

Here are the details of the failure.

https://gitlab.com/kernel-firmware/linux-firmware/-/merge_requests/486

BTW - In general it would be better to submit a MR so that you could 
just force push a fix.

> 
> diff --git a/copy-firmware.sh b/copy-firmware.sh
> index dd4b9b6f..cd5a6893 100755
> --- a/copy-firmware.sh
> +++ b/copy-firmware.sh
> @@ -11,8 +11,13 @@ compext=
>   destdir=
>   num_jobs=1
>   
> +usage() {
> +    echo "Usage: $0 [-v] [-jN] [--xz|--zstd] <destination directory>"
> +}
> +
>   err() {
>       printf "ERROR: %s\n" "$*"
> +    usage
>       exit 1
>   }
>   
> @@ -39,6 +44,7 @@ while test $# -gt 0; do
>   
>           -j*)
>               num_jobs=$(echo "$1" | sed 's/-j//')
> +            num_jobs=${num_jobs:-1}
>               if [ "$num_jobs" -gt 1 ] && ! has_gnu_parallel; then
>                       err "the GNU parallel command is required to use -j"
>               fi
> @@ -66,6 +72,18 @@ while test $# -gt 0; do
>               shift
>               ;;
>   
> +        -h|--help)
> +            usage
> +            exit 1
> +            ;;
> +
> +        -*)
> +            # Ignore anything else that begins with - because that confuses
> +            # the "test" command below
> +            warn "ignoring option $1"
> +            shift
> +            ;;
> +
>           *)
>               if test -n "$destdir"; then
>                   err "unknown command-line options: $*"