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

[Jani Nikula <jani.nikula@intel.com>]

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