Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[George Dunlap <george.dunlap@eu.citrix.com>]

On 07/16/2014 04:06 PM, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
>> At the moment, making a release tarball is an annoyingly manual
>> process that involves running "git archive" into a temporary directory.
> ...
>> +src-tarball: subtree-update
>> +	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
> Does this absolutely have to depend on subtree-update ?  Perhaps we
> need a new "make the subtrees exist" target ?

I'll add a "subtree-find" command (perhaps renaming the other to 
"subtree-force-update"; then as per IanC's suggestion, I'll make this 
depend on subtree-force-update unless some sensible parameter is set.

>
>> diff --git a/tools/misc/mktarball b/tools/misc/mktarball
> ...
>> +function finish {
>> +    [[ -n "$tdir" ]] && rm -rf $tdir
>> +}
> The use of the POSIX syntax
>     finish() {
> seems much more prevalent in-tree, than the use of `function'.

Ack.

>
>> +trap finish EXIT
> The EXIT trap handler should probably `set +e'.

Ack.

>
>> +function git_archive_into {
>> +    mkdir "$2"
>> +
>> +    git --git-dir="$1"/.git \
>> +	archive --format=tar HEAD | \
>> +	tar Cxf "$2" -
>> +}
>> +
>> +if [[ -z "$1" || -z "$2" ]] ; then
>> +  echo "usage: $0 path-to-XEN_ROOT xen-version"
>> +  exit 1
>> +fi
>> +
>> +xen_root="$1"
>> +desc="$2"
>> +
>> +mkdir -p $xen_root/dist/
>> +
>> +tdir="$(mktemp -d $xen_root/dist/xen.XXXXXXXX)"
> Why not use a fixed filename ?  This script isn't safe for concurrent
> invocation anyway, because the output filename is (mostly) fixed.

I'm not so much worried about concurrent invocation per se, but about 
dealing with stuff possibly left over from a previous run.  If we use a 
fixed path, we have to always rm -rf the path before starting, which 
seems unnecessarily risky to me.  (Or we could not rm -rf the path, 
which is even worse from a consistency point of view.)  Creating a new 
temporary directory just seems like the cleanest, safest thing to do.  
It's not like it's that expensive.

(Re concurrent invocation: The output filename may collide, but at least 
you'll get a consistent snapshot of that one particular tree. What I'd 
be more worried about is racing with make subdir-force-update and 
getting inconsisent subtree archives.)

>
> If you do that you can do away with the trap handler entirely.  You
> should probably make git_archive_into delete the destination
> directory.

I'm not sure what the fixed filename has to do with cleanup -- I would 
want to remove the temporary directory on a failure regardless.

>
>> +GZIP=-9v tar cz -f $xen_root/dist/xen-$desc.tar.gz -C $tdir xen-$desc
>> +
>> +echo "Source tarball in $xen_root/dist/xen-$desc.tar.gz"
>> \ No newline at end of file
> No newline at end of file.

Ack.

  -George