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

[Jani Nikula]

On Fri, 18 Aug 2017, Rodrigo Vivi <rodrigo.vivi at gmail.com> wrote:
> 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?

notmuch-emacs lets you pipe the original message as-is.

> 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?!

I tried, it didn'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

It appears to only work if the email is wrapped in an mbox. There is no
reason for a MUA to wrap single messages in an mbox. Running git
mailsplit on a plain message file in maildir results in "corrupt
mailbox" error.

BR,
Jani.

>
> 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

-- 
Jani Nikula, Intel Open Source Technology Center