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

[Fam Zheng]

On Thu, 09/21 10:22, Eric Blake wrote:
> 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>

Fixing all of them and adding your r-b line.

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

Fam