Re: [Qemu-ppc] [PATCH v6 3/8] scripts: Submit changes while updating linux headers

[Peter Maydell]

On 24 August 2015 at 13:03, Gavin Shan <address@hidden> wrote:
> This submits changes with formatted commit log while updating Linux
> headers using scripts/update-linux-headers.sh.
>
> Signed-off-by: Gavin Shan <address@hidden>

Thanks for writing a patch for this.

> ---
>  scripts/update-linux-headers.sh | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 18daabe..451b739 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -63,6 +63,25 @@ cp_virtio() {
>      fi
>  }
>
> +submit_change() {
> +    from=$1
> +    to=$2
> +    if ! [ -e $to/include/qemu-common.h ]; then

An error message about why we're bailing out might be nice.
Also, it would be better to tell the user the output directory
isn't valid before we spend all the time building the
kernel headers, rather than afterwards...

> +        exit 3
> +    fi
> +
> +    cd $from
> +    version=$(make -s kernelversion)
> +    subject="Sync Linux headers from kernel $version"
> +    message=$(git log --oneline -1)
> +    cd -

I think 'cd -' is a bashism. Better to use
   version=$(make -C $from -s kernelversion)
   message = $(cd $from && git log ...)
etc.

> +    cd $to
> +    name=$(git config --get user.name)
> +    email=$(git config --get user.email)
> +    git commit -a -m "$subject" -m "$message" -m "Signed-off-by: $name 
> <$email>"

Is git commit's --signoff option not present on all the git
versions we care about?


The commit message could be made a bit more verbose. I'd suggest
something like:

   Synchronize Linux headers from kernel $version

   Synchronize the Linux headers from kernel version $version
   (commit $commithash).

   This commit was created automatically by update-linux-headers.sh.

thanks
-- PMM