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

[Eric Blake]

On 09/17/2017 09:43 PM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  scripts/archive-source.sh | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 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..8b373e3090
> --- /dev/null
> +++ b/scripts/archive-source.sh
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +#
> +# 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,
> +# 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.
> +
> +set -e

Do we want to rely on 'set -e'?  The alternative would be:

> +
> +if test $# -lt 1; then
> +    echo "Usage: $0 <output tarball>"
> +    exit 1
> +fi
> +
> +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> +

...Check here that the $() ran (rather than git being missing)

> +if test -n "$submodules"; then
> +    {
> +        git ls-files

...and that this ran

> +        for sm in $submodules; do
> +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
> +        done
> +    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > 
> "$1".list

...and the grep

> +else
> +    git ls-files > "$1".list

...and the git ls-files

> +fi
> +
> +tar -cf "$1" -T "$1".list

...and the tar

> +rm "$1".list
> 

I'm not opposed to the patch going in as-is; but I do like to flag
reliance on 'set -e' as it tends to NOT do what you think it does, and
ends up getting in the way more than it helps.

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

signature.asc
Description: OpenPGP digital signature