Re: [Qemu-devel] [PATCH] build: add make dist target (v2)

[Eric Blake <eblake@redhat.com>]

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

On 07/17/2012 12:33 PM, Anthony Liguori wrote:
> Let's stop screwing up releases by having a script do the work that Anthony's
> fat fingers can't seem to get right.
> 
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> +++ b/scripts/make-release
> @@ -0,0 +1,24 @@
> +#!/bin/bash -e

Is it worth tightening this up to avoid bashisms like pushd, and use
just POSIX sh?

> +# This work is licensed under the terms of the GNU GPLv2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +src="$1"
> +version="$2"
> +destination=qemu-${version}

Inconsistent quoting.  The fact that you quoted $2 when assigning to
version makes me worry that you are trying to plan for someone calling
this script where $2 contains whitespace, but then destination would
contain whitespace.  This is equivalent with minimal typing (regardless
of whitespace, since variable assignment is not subject to word splitting):

src=$1
version=$2
destination=qemu-$version

or if you want to consistently use full quoting:

src="${1}"
version="${2}"
destination="qemu-${version}"

> +
> +git clone "${src}" ${destination}

But here is a line where it matters if $destination contains whitespace
because $2 contained whitespace.

> +pushd ${destination}
> +git checkout "v${version}"
> +git submodule update --init
> +rm -rf .git roms/*/.git
> +popd

The POSIX spelling to avoid pushd would be:

(
cd $destination
git checkout v$version
git submodule update --init
rm -rf .git roms/*/.git
)

[again, I did minimal typing; you may prefer the "v${version}" style
instead of minimalism]

> +tar cfj ${destination}.tar.bz2 ${destination}

'j' is a GNU tar extension.  Are you okay hard-coding this script to
only run on machines with GNU tar?  Or should you split this into 'tar c
... | bzip2 ...'?

> +rm -rf ${destination}
> 

For the record, I think releases are done so seldom, and on a
controlled-enough environment where extra tools can be relied on, that
this script probably does not have to be super-portable.  Therefore,
since what you have works for your environment, then even though I raked
it over the portability coals above I'm okay if you use it as-is rather
than posting a v3.  Hence, I give my:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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