Re: [Qemu-devel] [PULL v2 10/37] scripts: Add archive-source.sh

[Kamil Rytarowski]

On 09.09.2017 17:07, Fam Zheng wrote:
> On Sat, 09/09 13:07, Peter Maydell wrote:
>> On 9 September 2017 at 06:45, Fam Zheng <address@hidden> wrote:
>>> Signed-off-by: Fam Zheng <address@hidden>
>>> Message-Id: <address@hidden>
>>> ---
>>>  scripts/archive-source.sh | 31 +++++++++++++++++++++++++++++++
>>>  1 file changed, 31 insertions(+)
>>>  create mode 100755 scripts/archive-source.sh
>>>
>>> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
>>> new file mode 100755
>>> index 0000000000..1de369532e
>>> --- /dev/null
>>> +++ b/scripts/archive-source.sh
>>> @@ -0,0 +1,31 @@
>>> +#!/bin/sh
>>> +#
>>> +# Author: Fam Zheng <address@hidden>
>>> +#
>>> +# Create archive of source tree, including submodules
>>> +#
>>> +# This code is licensed under the GPL version 2 or later.  See
>>> +# the COPYING file in the top-level directory.
>>
>> So is this the script that for instance Mike Roth would use
>> to create the release tarballs? If it isn't, should it be?
> 
> I don't know the workflow of release tarballs, there is ./scripts/make-release
> for that. So this one is not.
> 
>> Is it intended for end users to create tarballs with, or
>> is it really just a helper script for the docker stuff?
>> If the latter, it would be helpful to say so. If the former,
>> it could really use more usage information/documentation...
> 
> I'm not sure what end users would use this for, but it should do its work just
> the same.. What kind of usage information do you suggest? More explaination in
> the header, or in the "usage" output when no target is specified, or a "-h"
> option for help?
> 
>>
>>> +
>>> +set -e
>>> +
>>> +if test $# -lt 1; then
>>> +    echo "Usage: $0 <output>"
>>> +    exit 1
>>> +fi
>>> +
>>> +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
>>> +
>>> +if test -n "$submodules"; then
>>> +    {
>>> +        git ls-files
>>> +        for sm in $submodules; do
>>> +            (cd $sm; git ls-files) | sed "s:^:$sm/:"
>>> +        done
>>> +    } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > 
>>> $1.list
>>
>> Supporting '-e something' in a tar -T listfile seems to
>> be GNU tar specific. Do we care?
> 
> The "-e $sm" will go to the grep command line, so it isn't GNU specific, is 
> it?
> 

BSD OSes use GNU grep(1) the latest version GPLv2. There is a work in
progress BSD grep(1), but not turned on by default as it's slower.

The -e option is supported by SmartOS grep(1).

>>
>>> +else
>>> +    git ls-files > $1.list
>>> +fi
>>
>> This will blow up if we ever have a file in the repo
>> that starts with a '-'. Do we care?
> 
> I'm sure such a file will be a trouble, but I'd rather we deal with it when
> there is one: I don't think anyone will think about adding, or welcome such a
> file.
> 
>>
>>> +
>>> +tar -cf $1 -T $1.list
>>> +rm $1.list
>>
>> This is missing a lot of quoting for $1, so it will go wrong
>> if there's a space in that filename argument.
> 
> Yes, good point. I will fix it before sending another pull request along with
> other comments that are just raised.
> 
> Fam
>

signature.asc
Description: OpenPGP digital signature