[Intel-gfx] [PATCH] dim: Properly handle series on apply_branch

[Rodrigo Vivi]

On Tue, Aug 22, 2017 at 12:22 AM, Jani Nikula <jani.nikula at intel.com> wrote:
> On Mon, 21 Aug 2017, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
>> So far we could use *dim* to apply a whole series
>> in a mbox, but only the very last patch was receiving
>> all the checks and patchwork link.
>>
>> So this patch remove this limitation by using git mailsplit
>> to split the mbox and than use git am and checks individually
>> on each patch.
>>
>> v2: a. Don't loop with `ls $dir` nor use ls. Shellcheck recommends
>>        globs instead. Reference: SC2045
>>     c. Split the apply patch in a separated function as suggested
>>        by Jani.
>>     b. Use -b on git mailsplit so it will automatically it is not
>>        an mbox file and parse it assuming a single mail message.
>>        This fixes the issue Jani notice with input directly from
>>        MUA: "corrupt mailbox".
>>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> ---
>>  dim | 65 ++++++++++++++++++++++++++++++++++++++---------------------------
>>  1 file changed, 38 insertions(+), 27 deletions(-)
>>
>> diff --git a/dim b/dim
>> index 11aa675cc3bc..866563624eb5 100755
>> --- a/dim
>> +++ b/dim
>> @@ -756,49 +756,60 @@ function dim_push
>>       dim_push_branch $(git_current_branch) "$@"
>>  }
>>
>> +function apply_patch #patch_file
>> +{
>> +     local patch message_id committer_email patch_from sob rv
>> +
>> +     patch="$1"
>
> shift here
>
>> +     message_id=$(message_get_id $patch)
>> +     committer_email=$(git_committer_email)
>> +
>> +     patch_from=$(grep "From:" "$patch" | head -1)
>> +     if [[ "$patch_from" != *"$committer_email"* ]] ; then
>> +             sob=-s
>> +     fi
>> +
>> +     git am --scissors -3 $sob "$@" $patch
>
> The "$@" no longer gets passed all the way here.
>
>> +
>> +     if [ -n "$message_id" ]; then
>> +             dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
>> +     else
>> +             echoerr "WARNING: No message-id found in the patch file."
>> +             rv=1
>> +     fi
>> +
>> +     if ! checkpatch_commit HEAD; then
>> +             rv=1
>> +     fi
>> +     if ! check_maintainer $branch HEAD; then
>> +             rv=1
>> +     fi
>> +
>> +     eval $DRY $DIM_POST_APPLY_ACTION
>
> return $rv.
>
>> +}
>> +
>>  # ensure we're on branch $1, and apply patches. the rest of the arguments are
>>  # passed to git am.
>>  dim_alias_ab=apply-branch
>>  dim_alias_sob=apply-branch
>>  function dim_apply_branch
>>  {
>> -     local branch file message_id committer_email patch_from sob rv
>> +     local branch file
>>
>>       branch=${1:?$usage}
>>       shift
>>       file=$(mktemp)
>> +     dir=$(mktemp -d)
>>
>>       assert_branch $branch
>>       assert_repo_clean
>>
>>       cat > $file
>> +     git mailsplit -b -o$dir $file > /dev/null
>
> Nitpick, git mailsplit could consume the file directly from stdin, so we
> no longer have a need for the temp $file.

I had considered and tried this here, but didn't worked as I expected...
maybe in a separate patch later after playing a bit and test more?

>
>>
>> -     message_id=$(message_get_id $file)
>> -
>> -     committer_email=$(git_committer_email)
>> -
>> -     patch_from=$(grep "From:" "$file" | head -1)
>> -     if [[ "$patch_from" != *"$committer_email"* ]] ; then
>> -             sob=-s
>> -     fi
>> -
>> -     git am --scissors -3 $sob "$@" $file
>> -
>> -     if [ -n "$message_id" ]; then
>> -             dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
>> -     else
>> -             echoerr "WARNING: No message-id found in the patch file."
>> -             rv=1
>> -     fi
>> -
>> -     if ! checkpatch_commit HEAD; then
>> -             rv=1
>> -     fi
>> -     if ! check_maintainer $branch HEAD; then
>> -             rv=1
>> -     fi
>> -
>> -     eval $DRY $DIM_POST_APPLY_ACTION
>> +     for patch in $dir/*; do
>> +             apply_patch $patch
>
> Need to pass "$@" to apply_patch.
>
> Need to handle the return value from apply_patch, and I presume we don't
> want checkpatch warnings in a single patch to stop applying the rest of
> the mbox (set -e would abort on errors otherwise). But we want to return
> non-zero exit status in that case.
>
> BR,
> Jani.
>
>
>> +     done
>>
>>       return $rv
>>  }
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br