Re: [PATCH] dim: Properly handle series on apply_branch

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

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