intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [maintainer-tools PATCH] dim: Properly handle series on apply_branch
@ 2017-08-17 18:09 Rodrigo Vivi
  2017-08-18  7:07 ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-08-17 18:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter, Rodrigo Vivi

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@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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
 
-	message_id=$(message_get_id $file)
+	for patch in `ls $dir`; do
 
-	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
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-17 18:09 [maintainer-tools PATCH] dim: Properly handle series on apply_branch Rodrigo Vivi
@ 2017-08-18  7:07 ` Jani Nikula
  2017-08-19  0:05   ` Rodrigo Vivi
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2017-08-18  7:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi

On Thu, 17 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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.

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?

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

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-18  7:07 ` Jani Nikula
@ 2017-08-19  0:05   ` Rodrigo Vivi
  2017-08-21  8:01     ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-08-19  0:05 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi

On Fri, Aug 18, 2017 at 12:07 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 17 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-19  0:05   ` Rodrigo Vivi
@ 2017-08-21  8:01     ` Jani Nikula
  2017-08-21 22:10       ` [PATCH] " Rodrigo Vivi
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2017-08-21  8:01 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi

On Fri, 18 Aug 2017, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> On Fri, Aug 18, 2017 at 12:07 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Thu, 17 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dim: Properly handle series on apply_branch
  2017-08-21  8:01     ` Jani Nikula
@ 2017-08-21 22:10       ` Rodrigo Vivi
  2017-08-22  7:22         ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-08-21 22:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter, Rodrigo Vivi

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@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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"
+	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
+
+	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
+}
+
 # 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
 
-	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
+	done
 
 	return $rv
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] dim: Properly handle series on apply_branch
  2017-08-21 22:10       ` [PATCH] " Rodrigo Vivi
@ 2017-08-22  7:22         ` Jani Nikula
  2017-08-22 16:57           ` [maintainer-tools PATCH] " Rodrigo Vivi
  2017-08-22 16:58           ` [PATCH] " Rodrigo Vivi
  0 siblings, 2 replies; 12+ messages in thread
From: Jani Nikula @ 2017-08-22  7:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi

On Mon, 21 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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.

>  
> -	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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-22  7:22         ` Jani Nikula
@ 2017-08-22 16:57           ` Rodrigo Vivi
  2017-08-23  8:54             ` Jani Nikula
  2017-08-22 16:58           ` [PATCH] " Rodrigo Vivi
  1 sibling, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-08-22 16:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter, Rodrigo Vivi

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".
v3: Pass $@ to apply_patch function and properly handle the shift.
    Handle returns. If any patch in the series had some kind of goof
    return 1.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 dim | 52 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/dim b/dim
index 11aa675cc3bc..80ad23a1cd31 100755
--- a/dim
+++ b/dim
@@ -756,33 +756,21 @@ function dim_push
 	dim_push_branch $(git_current_branch) "$@"
 }
 
-# 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
+function apply_patch #patch_file
 {
-	local branch file message_id committer_email patch_from sob rv
+	local patch message_id committer_email patch_from sob rv
 
-	branch=${1:?$usage}
+	patch="$1"
 	shift
-	file=$(mktemp)
-
-	assert_branch $branch
-	assert_repo_clean
-
-	cat > $file
-
-	message_id=$(message_get_id $file)
-
+	message_id=$(message_get_id $patch)
 	committer_email=$(git_committer_email)
 
-	patch_from=$(grep "From:" "$file" | head -1)
+	patch_from=$(grep "From:" "$patch" | head -1)
 	if [[ "$patch_from" != *"$committer_email"* ]] ; then
 		sob=-s
 	fi
 
-	git am --scissors -3 $sob "$@" $file
+	git am --scissors -3 $sob "$@" $patch
 
 	if [ -n "$message_id" ]; then
 		dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
@@ -799,6 +787,34 @@ function dim_apply_branch
 	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 rv
+
+	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
+
+	for patch in $dir/*; do
+		apply_patch $patch $@
+		if [[ $? -eq "1" ]] ; then
+			rv=1
+		fi
+	done
 
 	return $rv
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] dim: Properly handle series on apply_branch
  2017-08-22  7:22         ` Jani Nikula
  2017-08-22 16:57           ` [maintainer-tools PATCH] " Rodrigo Vivi
@ 2017-08-22 16:58           ` Rodrigo Vivi
  1 sibling, 0 replies; 12+ messages in thread
From: Rodrigo Vivi @ 2017-08-22 16:58 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx, Rodrigo Vivi

On Tue, Aug 22, 2017 at 12:22 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 21 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-22 16:57           ` [maintainer-tools PATCH] " Rodrigo Vivi
@ 2017-08-23  8:54             ` Jani Nikula
  2017-08-23  8:58               ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2017-08-23  8:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi

On Tue, 22 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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".
> v3: Pass $@ to apply_patch function and properly handle the shift.
>     Handle returns. If any patch in the series had some kind of goof
>     return 1.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  dim | 52 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/dim b/dim
> index 11aa675cc3bc..80ad23a1cd31 100755
> --- a/dim
> +++ b/dim
> @@ -756,33 +756,21 @@ function dim_push
>  	dim_push_branch $(git_current_branch) "$@"
>  }
>  
> -# 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
> +function apply_patch #patch_file
>  {
> -	local branch file message_id committer_email patch_from sob rv
> +	local patch message_id committer_email patch_from sob rv
>  
> -	branch=${1:?$usage}
> +	patch="$1"
>  	shift
> -	file=$(mktemp)
> -
> -	assert_branch $branch
> -	assert_repo_clean
> -
> -	cat > $file
> -
> -	message_id=$(message_get_id $file)
> -
> +	message_id=$(message_get_id $patch)
>  	committer_email=$(git_committer_email)
>  
> -	patch_from=$(grep "From:" "$file" | head -1)
> +	patch_from=$(grep "From:" "$patch" | head -1)
>  	if [[ "$patch_from" != *"$committer_email"* ]] ; then
>  		sob=-s
>  	fi
>  
> -	git am --scissors -3 $sob "$@" $file
> +	git am --scissors -3 $sob "$@" $patch
>  
>  	if [ -n "$message_id" ]; then
>  		dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
> @@ -799,6 +787,34 @@ function dim_apply_branch
>  	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 rv
> +
> +	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
> +
> +	for patch in $dir/*; do
> +		apply_patch $patch $@

Should be quoted "$@".

> +		if [[ $? -eq "1" ]] ; then

This if will never be executed if apply_patch exit code is != 0 because
of set -e. I gave you the example how to call apply_patch! ;)

> +			rv=1
> +		fi
> +	done

Hmm, could do rm -rf $(file) $(dir) here.

>  
>  	return $rv
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-23  8:54             ` Jani Nikula
@ 2017-08-23  8:58               ` Jani Nikula
  2017-08-23 17:29                 ` Rodrigo Vivi
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2017-08-23  8:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi

On Wed, 23 Aug 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> I gave you the example how to call apply_patch! ;)

Huh, I *thought* I did, maybe I didn't. Sorry.

	if ! apply_patch $patch "$@"; then
		rv=1
	fi

Similar to the checkpatch_commit call in dim_apply_branch.

BR
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-23  8:58               ` Jani Nikula
@ 2017-08-23 17:29                 ` Rodrigo Vivi
  2017-08-24  7:58                   ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-08-23 17:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter, Rodrigo Vivi

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".
v3: Pass $@ to apply_patch function and properly handle the shift.
    Handle returns. If any patch in the series had some kind of goof
    return 1.
v4: a. Fix "@"
    b. handle in a way that rv gets propagated with set -e
    c. Delete temporary files and directory.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 dim | 52 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/dim b/dim
index 11aa675cc3bc..46bd66f3cd3d 100755
--- a/dim
+++ b/dim
@@ -756,33 +756,21 @@ function dim_push
 	dim_push_branch $(git_current_branch) "$@"
 }
 
-# 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
+function apply_patch #patch_file
 {
-	local branch file message_id committer_email patch_from sob rv
+	local patch message_id committer_email patch_from sob rv
 
-	branch=${1:?$usage}
+	patch="$1"
 	shift
-	file=$(mktemp)
-
-	assert_branch $branch
-	assert_repo_clean
-
-	cat > $file
-
-	message_id=$(message_get_id $file)
-
+	message_id=$(message_get_id $patch)
 	committer_email=$(git_committer_email)
 
-	patch_from=$(grep "From:" "$file" | head -1)
+	patch_from=$(grep "From:" "$patch" | head -1)
 	if [[ "$patch_from" != *"$committer_email"* ]] ; then
 		sob=-s
 	fi
 
-	git am --scissors -3 $sob "$@" $file
+	git am --scissors -3 $sob "$@" $patch
 
 	if [ -n "$message_id" ]; then
 		dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
@@ -799,6 +787,34 @@ function dim_apply_branch
 	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 rv
+
+	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
+
+	for patch in $dir/*; do
+		if ! apply_patch $patch "$@"; then
+			rv=1
+		fi
+	done
+	rm -rf $file $dir
 
 	return $rv
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [maintainer-tools PATCH] dim: Properly handle series on apply_branch
  2017-08-23 17:29                 ` Rodrigo Vivi
@ 2017-08-24  7:58                   ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2017-08-24  7:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi

On Wed, 23 Aug 2017, Rodrigo Vivi <rodrigo.vivi@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".
> v3: Pass $@ to apply_patch function and properly handle the shift.
>     Handle returns. If any patch in the series had some kind of goof
>     return 1.
> v4: a. Fix "@"
>     b. handle in a way that rv gets propagated with set -e
>     c. Delete temporary files and directory.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

LGTM, and seems to work in my workflow.

BR,
Jani.

> ---
>  dim | 52 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/dim b/dim
> index 11aa675cc3bc..46bd66f3cd3d 100755
> --- a/dim
> +++ b/dim
> @@ -756,33 +756,21 @@ function dim_push
>  	dim_push_branch $(git_current_branch) "$@"
>  }
>  
> -# 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
> +function apply_patch #patch_file
>  {
> -	local branch file message_id committer_email patch_from sob rv
> +	local patch message_id committer_email patch_from sob rv
>  
> -	branch=${1:?$usage}
> +	patch="$1"
>  	shift
> -	file=$(mktemp)
> -
> -	assert_branch $branch
> -	assert_repo_clean
> -
> -	cat > $file
> -
> -	message_id=$(message_get_id $file)
> -
> +	message_id=$(message_get_id $patch)
>  	committer_email=$(git_committer_email)
>  
> -	patch_from=$(grep "From:" "$file" | head -1)
> +	patch_from=$(grep "From:" "$patch" | head -1)
>  	if [[ "$patch_from" != *"$committer_email"* ]] ; then
>  		sob=-s
>  	fi
>  
> -	git am --scissors -3 $sob "$@" $file
> +	git am --scissors -3 $sob "$@" $patch
>  
>  	if [ -n "$message_id" ]; then
>  		dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
> @@ -799,6 +787,34 @@ function dim_apply_branch
>  	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 rv
> +
> +	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
> +
> +	for patch in $dir/*; do
> +		if ! apply_patch $patch "$@"; then
> +			rv=1
> +		fi
> +	done
> +	rm -rf $file $dir
>  
>  	return $rv
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-08-24  7:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-17 18:09 [maintainer-tools PATCH] dim: Properly handle series on apply_branch Rodrigo Vivi
2017-08-18  7:07 ` Jani Nikula
2017-08-19  0:05   ` Rodrigo Vivi
2017-08-21  8:01     ` Jani Nikula
2017-08-21 22:10       ` [PATCH] " Rodrigo Vivi
2017-08-22  7:22         ` Jani Nikula
2017-08-22 16:57           ` [maintainer-tools PATCH] " Rodrigo Vivi
2017-08-23  8:54             ` Jani Nikula
2017-08-23  8:58               ` Jani Nikula
2017-08-23 17:29                 ` Rodrigo Vivi
2017-08-24  7:58                   ` Jani Nikula
2017-08-22 16:58           ` [PATCH] " Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).