On Thu, 26 Apr 2018, Tvrtko Ursulin <[email protected]> wrote:
> From: Tvrtko Ursulin <[email protected]>
>
> For patch authors and committers with multiple email addresses, it is good
> to check all 'From:' lines before deciding to add a Signed-off-by line.
> This prevents adding duplicate S-o-B's in those cases.
>
> Signed-off-by: Tvrtko Ursulin <[email protected]>
> ---
>  dim | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/dim b/dim
> index 091dff8518ed..b587a4d1bccf 100755
> --- a/dim
> +++ b/dim
> @@ -824,6 +824,22 @@ function dim_push
>       dim_push_branch $(git_current_branch) "$@"
>  }
>  
> +function is_own_patch
> +{
> +     patch="$1"
> +     committer_email="$2"
> +
> +     grep "From:" $patch | while read patch_from; do
> +             [[ "$patch_from" == *"$committer_email"* ]] && exit 99
> +     done

So in the case of this patch at hand, the grep will match these two
message header lines:

From: Tvrtko Ursulin <[email protected]>
X-Google-Original-From: Tvrtko Ursulin <[email protected]>

but, due to the base64 Content-Transfer-Encoding added somewhere along
the mail transfer, it won't match the From: line git has added at the
start of the message body. Which gets added because .gitconfig
user.email does not match the patch author. Which is probably what you
intended to search for.

Basically anywhere dim greps or otherwise manipulates the email message
directly, it's a bug waiting to happen. The current

        patch_from=$(grep "From:" "$patch" | head -1)

mostly works because it takes the first match which is in the message
headers and thus unaffected by Content-Transfer-Encoding... but an RFC
compliant message could still have line folding failing the grep:

From:
 Tvrtko Ursulin <[email protected]>

Email is hard.

On a style note, I prefer *not* using the

        foo || bar

or

        foo && bar

style, instead opting for explicit if-then. The notable exception is "||
true" to avoid the set -e kicking in at places.

BR,
Jani.

> +
> +     if [ $? -eq 99 ]; then
> +             return 0
> +     else
> +             return 1
> +     fi
> +}
> +
>  function apply_patch #patch_file
>  {
>       local patch message_id committer_email patch_from sob rv
> @@ -833,10 +849,7 @@ function apply_patch #patch_file
>       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
> +     is_own_patch "$patch" "$committer_email" || sob=-s
>  
>       git am --scissors -3 $sob "$@" $patch

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dim-tools mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dim-tools

Reply via email to