Re: [MAINTAINERS SUMMIT] [4/4] Discuss how to better prevent backports of commits that turn out to cause regressions

[Lee Jones <lee@kernel.org>]

On Fri, 14 Jun 2024, Lee Jones wrote:

> On Thu, 13 Jun 2024, Konstantin Ryabitsev wrote:
> 
> > On Thu, Jun 13, 2024 at 11:59:17AM GMT, Jan Kara wrote:
> > > > * One cause of regressions that happen in stable trees (and not in
> > > > mainline) I've seen quite a few times are backports of commits with
> > > > Fixes: tags that were part of a patch-series and depend on earlier
> > > > patches from the series. The stable-team afaics has no easy way to spot
> > > > this, as there is no way to check "was this change part of a series".
> > > > Sometimes I wonder if a dedicated tag linking to the submission of a
> > > > patch could help -- and is something quite a few maintainers already
> > > > really want and add using a "Link" tag despite Linus dislike for that
> > > > (IIRC).
> > > 
> > > FWIW I (and a few other maintainers) use 'Message-Id' tag to link to
> > > submission. This is still easily convertible to lore link and unlike 'Link'
> > > tag it is clear what this tag is about and that it is not just a link to
> > > related discussion or something like that. AFAIK this also addresses Linus'
> > > dislike because what he was complaining about is that 'Link' should be
> > > linking to some useful context for the changelog, not just patch
> > > submission.
> > 
> > I am strongly in favour of that from the tooling perspective. Linus suggested
> > that we can always trace the original patch submission from the commit by
> > using the patch-id, but that doesn't work reliably. I mused on that here:
> > 
> > https://lore.kernel.org/git/20240605-hilarious-dramatic-mushroom-7fd941@lemur/
> > 
> > The gist is that we cannot reliably match the patch-id of the original
> > submission from the git commit, because there are multiple ways to generate
> > the same patch, such as changing the diff algorithm (myers vs. minimal vs.
> > histogram), or changing the number of context lines. If the original author
> > generated their patch with --histogram, but we try to find it by generating
> > the same patch using the default myers algorithm, we may not find it.
> > 
> > The "Message-Id" trailer is already documented in git:
> > https://www.git-scm.com/docs/git-am#Documentation/git-am.txt---message-id
> > 
> > I suggest we move away from the practice of using Link: trailers to indicate
> > the patch provenance to using Message-Id: trailers for the same purpose. This
> > solves multiple problems:
> > 
> > 1. disambiguates Link: trailers so they point to relevant online discussions
> > 2. allows tooling like b4, patchwork, etc, to reliably match commits to
> >    submissions for the purposes of better code review automation
> > 3. allows stable and similar projects to better track series grouping for
> >    commits
> 
> Sounds good to me.
> 
> So `b4 am -l` should be replaced with `b4 am ?`.

Actually it looks as though I have an avenue for this already:

  print_green "\nFetching patch(es)"
  b4 am -3 -slt ${PATCHES} -o - ${id} > ${MBOX}

  print_green "\nRunning through checkpatch.pl"
  cat ${MBOX} | formail -ds ./scripts/checkpatch.pl || true
  print_blue "\nCheck the results (hit return to continue or Ctrl+c to exit)"
  read -u 1

  print_green "\nApplying patch(es)"
  cat ${MBOX} | git am -3 --reject --message-id              #### <--- HERE
  if [ ${?} != 0 ]; then
      print_red "\nFailed to apply patches (fix and either hit return to continue or Ctrl+c to exit)"
      read -u 1
  fi

Might be nicer to build it right into `b4` though.

-- 
Lee Jones [李琼斯]