Re: [Qemu-devel] [PATCH v3] scripts: use git archive in archive-source

[Eric Blake <eblake@redhat.com>]

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

On 1/31/19 7:00 AM, Gerd Hoffmann wrote:
> Use git archive to create tarballs of qemu and submodules instead of
> cloning the repository and the submodules.  This is a order of magnitude
> faster because it doesn't fetch the submodules from the internet each
> time the script runs.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  scripts/archive-source.sh | 63 ++++++++++++++++++++---------------------------
>  1 file changed, 27 insertions(+), 36 deletions(-)
> 
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> index 6eed2a29bd..38d53986d7 100755
> --- a/scripts/archive-source.sh
> +++ b/scripts/archive-source.sh
> @@ -19,8 +19,8 @@ if test $# -lt 1; then
>  fi
>  
>  tar_file=$(realpath "$1")
> -list_file="${tar_file}.list"
> -vroot_dir="${tar_file}.vroot"
> +sub_file=$(mktemp "${tar_file%.tar}.sub.XXXXXXXX.tar")
> +sub_tdir=$(mktemp -d "${tar_file%.tar}.sub.XXXXXXXX")

mktemp is not specified by POSIX; and FreeBSD man pages for mktemp
suggest that if you don't use XXXXXX as the suffix that you are not
guaranteed correct behavior.  Are you sure this is portable enough?  Do
you need both a temp file and dir, or can you create the file name of
your choice inside a temp dir, where only the dir has to have a
randomized name?

>  
>  # We want a predictable list of submodules for builds, that is
>  # independent of what the developer currently has initialized
> @@ -28,7 +28,7 @@ vroot_dir="${tar_file}.vroot"
>  # different to the host OS.
>  submodules="dtc ui/keycodemapdb tests/fp/berkeley-softfloat-3 tests/fp/berkeley-testfloat-3"
>  
> -trap "status=$?; rm -rf \"$list_file\" \"$vroot_dir\"; exit \$status" 0 1 2 3 15
> +trap "status=$?; rm -rf \"$sub_file\" \"$sub_tdir\" ; exit \$status" 0 1 2 3 15
>  
>  if git diff-index --quiet HEAD -- &>/dev/null
>  then
> @@ -36,38 +36,29 @@ then
>  else
>      HEAD=$(git stash create)
>  fi
> -git clone --shared . "$vroot_dir"
> -test $? -ne 0 && error "failed to clone into '$vroot_dir'"
> -
> -cd "$vroot_dir"
> -test $? -ne 0 && error "failed to change into '$vroot_dir'"
> -
> -git checkout $HEAD
> -test $? -ne 0 && error "failed to checkout $HEAD revision"
> -
> +git archive --format tar $HEAD > "$tar_file"
> +test $? -ne 0 && error "failed to archive qemu"
>  for sm in $submodules; do
> -    git submodule update --init $sm
> -    test $? -ne 0 && error "failed to init submodule $sm"
> +	status="$(git submodule status "$sm")"
> +	smhash="${status# }"
> +	smhash="${smhash#+}"
> +	smhash="${smhash#-}"

These three lines can be consolidated into one:
smhash=${status#[ +-]}

> +	smhash="${smhash%% *}"
> +	smdir="$sm"
> +	case "$status" in
> +	    -*)
> +		smdir="$sub_tdir/$sm"
> +		smurl="$(git config -f .gitmodules submodule.${sm}.url)"
> +		echo "NOTICE: using temporary clone for submodule $sm"
> +		git clone "$smurl" "$smdir"
> +		test $? -ne 0 && error "failed to clone submodule $sm"

I know we don't want to affect the developer's normal checkout, but is
it worth storing the temporary clone in a specifically-named
subdirectory of their checkout instead of in a randomly-generated mktemp
transient location, so that we can reuse results from a previous
archive-source run?

> +		;;
> +	    +*)
> +		echo "WARNING: submodule $sm is out of sync"
> +		;;
> +	esac
> +	(cd $smdir; git archive --format tar --prefix "$sm/" $smhash) > "$sub_file"
> +	test $? -ne 0 && error "failed to archive submodule $sm ($smhash)"
> +	tar --concatenate --file "$tar_file" "$sub_file"
> +	test $? -ne 0 && error "failed append submodule $sm to $tar_file"
>  done

Overall, though, the idea seems reasonable.

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


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