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

[Rodrigo Vivi]

On Tue, Jan 03, 2023 at 09:53:46PM +0100, Daniel Vetter wrote:
> On Fri, Dec 30, 2022 at 12:03:02PM +0000, Vivi, Rodrigo wrote:
> > On Fri, 2022-12-30 at 06:55 -0500, 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"
> > > 
> > > v3: - Also consider fix of fixes in drm-intel-fixes besides
> > >       the drm-intel-next-fixes.
> > >     - Use intel_remote branches instead of local.
> > >     - And run the check unconditionally as suggested by Daniel.
> > > 
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > >  dim | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/dim b/dim
> > > index b699108..b87b542 100755
> > > --- a/dim
> > > +++ b/dim
> > > @@ -1481,10 +1481,15 @@ function cherry_pick_branch
> > >                         if [[ "$(git merge-base $branch $fixes)" =
> > > "$fixes" ]]; then
> > >                                 needed=1
> > >                         fi
> > > +
> > > +                       fix_of_fix=$(git log --grep="cherry picked
> > > from commit $fixes" --after=6months --format=format:%h
> > > $intel_remote/drm-intel-next-fixes -1)
> > > +                       if [[ -z "$fix_of_fix" ]]; then
> > > +                               fix_of_fix=$(git log --grep="cherry
> > > picked from commit $fixes" --after=6months --format=format:%h
> > > $intel_remote/drm-intel-fixes -1)
> > > +                       fi
> > >                 done
> > >  
> > >                 # Note: Cc: stable will overrule Fixes:
> > > -               if [[ -n "$have_fixes" && -z "$needed" ]]; then
> > > +               if [[ -n "$have_fixes" && -z "$needed" && -z
> > > "$fix_of_fix" ]]; then
> > >                         echo "Fixes a commit not in $branch. OK."
> > >                         continue
> > >                 fi
> > > @@ -1495,6 +1500,14 @@ function cherry_pick_branch
> > >                         echo "FAILED: $(dim_cite $commit)"
> > >                         (dim_cite $commit) >> $fail_log
> > >                         git cherry-pick --abort
> > > +               else
> > > +                       if [[ "$fix_of_fix" ]]; then
> > > +                               short_fixes_hash=$(git log -1 --
> > > format=format:%h $fixes 2>/dev/null || true)
> > > +                               git log -1 --pretty=%B | \
> > > +                                       sed -e :a -e
> > > '/^\n*$/{$d;N;ba' -e '}' | \
> > > +                                       sed
> > > "s/$short_fixes_hash/$fix_of_fix/g" | \
> > 
> > Daniel, I did not ignored your comment. But I tried to replace the
> > whole line but sed with single quote doesn't expand the variable nor
> > dim_cite command and with double quote it fails because the fixes line
> > has parentheses in it. Then bash doesn't accept nested substitution
> > and things started to get really ugly there to replace the whole line
> > that in the end it has absolutely no chance of changing.
> > 
> > Replacing just the hash is safe and the cleanest option imho.
> 
> Occasionally shell scripting is a disappointment :-/
> 
> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Thank you. Pushed.

> > 
> > > +                                       git commit --amend -F-
> > > +                       fi
> > >                 fi
> > >         done
> > >  
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch