Re: [Qemu-devel] [PATCH v11 03/13] scripts: Add archive-source.sh

[Eric Blake]

On 09/21/2017 10:07 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  scripts/archive-source.sh | 51 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100755 scripts/archive-source.sh
> 
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> new file mode 100755
> index 0000000000..8aa363d3a0
> --- /dev/null
> +++ b/scripts/archive-source.sh
> @@ -0,0 +1,51 @@
> +#!/bin/bash
> +#
> +# Author: Fam Zheng <address@hidden>
> +#
> +# Archive source tree, including submodules. This is created for test code to
> +# export the source files, in order to be built in a different enviornment,

s/enviornment/environment/

> +# such as in a docker instance or VM.
> +#
> +# This code is licensed under the GPL version 2 or later.  See
> +# the COPYING file in the top-level directory.
> +
> +error() {
> +    echo "$@" >&2

slightly risky if $@ contains \ - using 'printf %s\\n "$*"' is nicer.
But this particular script can get away with it, as I don't see use of \
(other than if you have a weird $0).

> +
> +if test -n "$submodules"; then
> +    {
> +        git ls-files || error "git ls-files failed"
> +        for sm in $submodules; do
> +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
> +            if test "${PIPESTATUS[0]} ${PIPESTATUS[1]}" != "0 0"; then

Could shorten to "${PIPESTATUS[*]}", as discussed in another mail, but
this works too.

With the typo fixed, and your choice of how to handle my other two nits
(including doing nothing),
Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature