[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] build: add make dist target (v2)
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] build: add make dist target (v2) |
Date: |
Tue, 17 Jul 2012 12:50:01 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 |
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 <address@hidden>
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
> +++ 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 <address@hidden>
--
Eric Blake address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature