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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]

On 09/19/2017 02:27 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  scripts/archive-source.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 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..e155d980ee
> --- /dev/null
> +++ b/scripts/archive-source.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh

Needs to be /bin/bash...

> +        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]} -ne 0 -o $? -ne 0; then
> +                error "git ls-files in submodule $sm failed"

...because $PIPESTATUS is a useful bashism not present in dash, where
the alternative is MUCH more verbose and painful to write in portable shell.

With just that fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>

If desired, one other change (not mandatory, but if you want to do it,
I'll want to re-review):

> +
> +tar -cf "$1" -T "$1".list || error "failed to create tar file"

If we exit here, $1.list is not removed...

> +rm "$1".list

Should we make removal of the list be controlled by a cleanup trap, to
always happen even if something else fails in the interim?  Or is
leaving it around on failure useful for debugging?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 615 bytes --]