[PATCH] dim: Consider fix-of-fix for -fixes cherry picking.

[Daniel Vetter]

On Mon, Aug 08, 2022 at 03:00:02PM -0400, Rodrigo Vivi wrote:
> In case a fix was not propagated yet to upstream, the
> original commit sha won't be found hence the fix of this
> fix will be missed during the cherry-pick.
> 
> Since dim cherry-picks reliably adds the -x "(cherry picked
> from" message, let's also grep in the log to see if that
> was the case, before we give up and forget.
> 
> Also, if that's the case, let's then fix the 'Fixes:' tag.
> 
> v2: Actually replaces the 'Fixes:' tag, otherwise dim doesn't
> allow us to push giving the following message:
> "Fixes: SHA1 in not pointing at an ancestor:"
> "dim: ERROR: issues in commits detected, aborting"
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  dim | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/dim b/dim
> index 4b43bf8..33caaf6 100755
> --- a/dim
> +++ b/dim
> @@ -1485,8 +1485,13 @@ function cherry_pick_branch
>  
>  		# Note: Cc: stable will overrule Fixes:
>  		if [[ -n "$have_fixes" && -z "$needed" ]]; then
> -			echo "Fixes a commit not in $branch. OK."
> -			continue
> +
> +			fixes=$(git log -1 --format=format:%h $fixes 2>/dev/null || true)
> +			fix_of_fix=$(git log --grep="cherry picked from commit $fixes" --after=6months --format=format:%h drm-intel-next-fixes -1)
> +			if [[ -z "$fix_of_fix" ]]; then
> +				echo "Fixes a commit not in $branch. OK."
> +				continue
> +			fi

This hunk has my Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

>  		fi
>  
>  		echo "Try to cherry-pick."
> @@ -1495,6 +1500,13 @@ function cherry_pick_branch
>  			echo "FAILED: $(dim_cite $commit)"
>  			(dim_cite $commit) >> $fail_log
>  			git cherry-pick --abort
> +		else
> +			if [[ "$fix_of_fix" ]]; then
> +				git log -1 --pretty=%B | \
> +					sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' | \
> +					sed "s/$fixes/$fix_of_fix/g" | \

Maybe I'm not reading the shell magic here correctly, but I think this
only replaces the sha, not the commit subject we append in ("") after them
usually. So you'll still have some wrong citations in here I think.

Also I'm wondering whether instead it wouldn't be better to patch up dim
to look at the other history when the commit contains a "cherry picked
from $sha1" line unconditionally? Since we have other cases than dim
cherry-pick where cherry-picking happens ...

Cheers, Daniel

> +					git commit --amend -F-
> +			fi
>  		fi
>  	done
>  
> -- 
> 2.37.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch