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

[Rodrigo Vivi]

On Fri, Aug 18, 2017 at 12:07 AM, Jani Nikula <jani.nikula at intel.com> wrote:
> On Thu, 17 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.
>>
>> 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 | 45 +++++++++++++++++++++++++--------------------
>>  1 file changed, 25 insertions(+), 20 deletions(-)
>>
>> diff --git a/dim b/dim
>> index 11aa675cc3bc..5457006631d6 100755
>> --- a/dim
>> +++ b/dim
>> @@ -767,38 +767,43 @@ function dim_apply_branch
>>       branch=${1:?$usage}
>>       shift
>>       file=$(mktemp)
>> +     dir=$(mktemp -d)
>>
>>       assert_branch $branch
>>       assert_repo_clean
>>
>> +     committer_email=$(git_committer_email)
>> +
>>       cat > $file
>> +     git mailsplit -o$dir $file > /dev/null
>
> Oh, git mailsplit must have been introduced after I looked into this the
> last time. Nice.
>
> The downside is that this doesn't work on patches piped from the MUA,
> because git mailsplit expects an mbox. Fail.

How do you guys send directly from MUA?
When I was sending from mutt I was saving in an mbox anyway, but
nowadays I prefer to get from patchwork that gives exactly the mbox
that was validated on CI.
(Reason why I tweaked my dimrc to download directly mbox from a link
and apply to dinq...

Anyway are you sure this wouldn't work?!
git mailsplit works with a single patch or single email anyway and I
put in a way that I keep the cat > $file

so I don't see reasons why that shouldn't work..

maybe the missing \r and something that
-keep-cr could help with?

>
> I notice that you indent most of the function in the for loop, perhaps
> that's an indication you should turn this one into a single patch
> helper, and do the mailsplit in a new dim_apply_branch that only does
> that?

good idea... at least I don't mess with current people using directly
from mail clients without mbox intermediate...

>
>>
>> -     message_id=$(message_get_id $file)
>> +     for patch in `ls $dir`; do
>
> Please use $(...) instead of `...`. Didn't test, but I think shellcheck
> would complain.
>
> I think you should do $(find $dir | sort) instead of ls.

yep, make check complained now that I have a version of shellcheck that works ;)

>
> Otherwise seems like this should work.
>
> BR,
> Jani.
>
>>
>> -     committer_email=$(git_committer_email)
>> +             message_id=$(message_get_id $dir/$patch)
>>
>> -     patch_from=$(grep "From:" "$file" | head -1)
>> -     if [[ "$patch_from" != *"$committer_email"* ]] ; then
>> -             sob=-s
>> -     fi
>> +             patch_from=$(grep "From:" "$dir/$patch" | head -1)
>> +             if [[ "$patch_from" != *"$committer_email"* ]] ; then
>> +                     sob=-s
>> +             fi
>>
>> -     git am --scissors -3 $sob "$@" $file
>> +             git am --scissors -3 $sob "$@" $dir/$patch
>>
>> -     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 [ -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
>> +             if ! checkpatch_commit HEAD; then
>> +                     rv=1
>> +             fi
>> +             if ! check_maintainer $branch HEAD; then
>> +                     rv=1
>> +             fi
>>
>> -     eval $DRY $DIM_POST_APPLY_ACTION
>> +             eval $DRY $DIM_POST_APPLY_ACTION
>> +     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