| Message ID | 20170920032555.18911-4-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests: Add VM based build tests (for non-x86_64 and/or non-Linux) | expand |
On 09/19/2017 10:25 PM, Fam Zheng wrote: > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > scripts/archive-source.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100755 scripts/archive-source.sh > > + > +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]} -ne 0 -o $? -ne 0; then This relies on 'test ... -o ...' which is non-portable. It "works" because there is no possible ambiguity in the contents of $PIPESTATUS that could cause a different parse of the test arguments, but I tend to discourage any use of -a/-o inside test on principle. Sadly, writing: if test ${PIPESTATUS[0]} -ne 0 || test $? -ne 0 has a flaw that $? is no longer what you want, at which point you would have to introduce a temporary variable. But we're using bash, so you can instead write this as: if test "${PIPESTATUS[@]}" != "0 0"; then > + error "git ls-files in submodule $sm failed" > + fi > + done > + } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > "$1".list > +else > + git ls-files > "$1".list > +fi At this point, $1.list has been created, even if commands failed... > + > +if test $? -ne 0; then > + error "failed to generate list file" > +fi ...but this exits without cleanup. If we really want it cleaned no matter what, it's probably better to do: trap "status=$?; rm -f "$1".list; exit \$status" 0 1 2 3 15 earlier than anything that can create the file. > + > +tar -cf "$1" -T "$1".list > +status=$? > +rm "$1".list > +if test $statue -ne 0; then Umm, $statue is not the same as $status. > + error "failed to create tar file" > +fi > +exit 0 >
On Wed, 09/20 08:20, Eric Blake wrote: > On 09/19/2017 10:25 PM, Fam Zheng wrote: > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > scripts/archive-source.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100755 scripts/archive-source.sh > > > > > + > > +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]} -ne 0 -o $? -ne 0; then > > This relies on 'test ... -o ...' which is non-portable. It "works" > because there is no possible ambiguity in the contents of $PIPESTATUS > that could cause a different parse of the test arguments, but I tend to > discourage any use of -a/-o inside test on principle. Sadly, writing: > > if test ${PIPESTATUS[0]} -ne 0 || test $? -ne 0 > > has a flaw that $? is no longer what you want, at which point you would > have to introduce a temporary variable. But we're using bash, so you > can instead write this as: > > if test "${PIPESTATUS[@]}" != "0 0"; then Okay. > > > + error "git ls-files in submodule $sm failed" > > + fi > > + done > > + } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > "$1".list > > +else > > + git ls-files > "$1".list > > +fi > > At this point, $1.list has been created, even if commands failed... > > > + > > +if test $? -ne 0; then > > + error "failed to generate list file" > > +fi > > ...but this exits without cleanup. If we really want it cleaned no > matter what, it's probably better to do: > > trap "status=$?; rm -f "$1".list; exit \$status" 0 1 2 3 15 Sounds good, will do. > > earlier than anything that can create the file. > > > + > > +tar -cf "$1" -T "$1".list > > +status=$? > > +rm "$1".list > > +if test $statue -ne 0; then > > Umm, $statue is not the same as $status. Oops, will fix. Fam
On Wed, 09/20 08:20, Eric Blake wrote: > On 09/19/2017 10:25 PM, Fam Zheng wrote: > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > scripts/archive-source.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100755 scripts/archive-source.sh > > > > > + > > +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]} -ne 0 -o $? -ne 0; then > > This relies on 'test ... -o ...' which is non-portable. It "works" > because there is no possible ambiguity in the contents of $PIPESTATUS > that could cause a different parse of the test arguments, but I tend to > discourage any use of -a/-o inside test on principle. Sadly, writing: > > if test ${PIPESTATUS[0]} -ne 0 || test $? -ne 0 > > has a flaw that $? is no longer what you want, at which point you would > have to introduce a temporary variable. But we're using bash, so you > can instead write this as: > > if test "${PIPESTATUS[@]}" != "0 0"; then Hmm, with exactly this line here I get something like: ./scripts/archive-source.sh: line 36: test: too many arguments But with if test "${PIPESTATUS[0]} ${PIPESTATUS[1]}" != "0 0"; then it seems to work fine. What is the magic here? Fam
On 09/20/2017 07:45 PM, Fam Zheng wrote: >> >> has a flaw that $? is no longer what you want, at which point you would >> have to introduce a temporary variable. But we're using bash, so you >> can instead write this as: >> >> if test "${PIPESTATUS[@]}" != "0 0"; then > > Hmm, with exactly this line here I get something like: > > ./scripts/archive-source.sh: line 36: test: too many arguments D'oh - [@] causes word splitting (same as "$@"); we want [*] instead: if test "${PIPESTATUS[*]}" != "0 0"; > > But with > > if test "${PIPESTATUS[0]} ${PIPESTATUS[1]}" != "0 0"; then or that's a manual way of spelling the auto-array-concatenation that you get with [*]. > > it seems to work fine. What is the magic here? > > Fam >
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh new file mode 100755 index 0000000000..81e559cb8a --- /dev/null +++ b/scripts/archive-source.sh @@ -0,0 +1,51 @@ +#!/bin/bash +# +# Author: Fam Zheng <famz@redhat.com> +# +# 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. + +error() { + echo "$@" >&2 + exit 1 +} + +if test $# -lt 1; then + error "Usage: $0 <output tarball>" +fi + +submodules=$(git submodule foreach --recursive --quiet 'echo $name') + +if test $? -ne 0; then + error "git submodule command failed" +fi + +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]} -ne 0 -o $? -ne 0; then + error "git ls-files in submodule $sm failed" + fi + done + } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > "$1".list +else + git ls-files > "$1".list +fi + +if test $? -ne 0; then + error "failed to generate list file" +fi + +tar -cf "$1" -T "$1".list +status=$? +rm "$1".list +if test $statue -ne 0; then + error "failed to create tar file" +fi +exit 0
Signed-off-by: Fam Zheng <famz@redhat.com> --- scripts/archive-source.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100755 scripts/archive-source.sh