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

[Vivi, Rodrigo]

On Wed, 2022-10-05 at 14:12 +0200, Daniel Vetter wrote:
> 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.

you are right. I'm only changing the sha, but I'm considering that this
is the only thing we are changing anyway when cherry-picking to the
fixes branches.

I have never seen any case where we changed the subject while porting.
But you are saying that we should be save then sorry?

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

By unconditionally you mean out of the 
if [[ -n "$have_fixes" && -z "$needed" ]]; then
?

I don't believe this is needed because even if it is a cherry-picked
case, if it was found is because it was already propagated properly
and contains the right shas

Thanks,
Rodrigo.

> 
> Cheers, Daniel
> 
> > +                                       git commit --amend -F-
> > +                       fi
> >                 fi
> >         done
> >  
> > -- 
> > 2.37.1
> > 
>