Re: AAARRRGGGHHH!!!! (was Re: [PATCH 6.12.y] xe/oa: Fix query mode of operation for OAR/OAC)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 13, 2025 at 04:48:17PM -0500, Sasha Levin wrote:
> On Mon, Jan 13, 2025 at 10:44:41AM +1000, Dave Airlie wrote:
> > Pretty sure we've explained how a few times now, not sure we can do much more.
> > 
> > If you see a commit with a cherry-pick link in it and don't have any
> > sight on that commit in Linus's tree, ignore the cherry-pick link in
> > it, assume it's a future placeholder for that commit id. You could if
> > you wanted to store that info somewhere, but there shouldn't be a
> > need.
> > 
> > When the initial commit enters during the next merge window, you look
> > for that subject or commit id in the stable tree already, if it
> > exists, dump the latest Linus patch on the floor, it's already in
> > stable your job is done.
> 
> We can't rely too heavily on the subject line. Consider the following two
> very different commits that have the same subject line:
> 
> 	3119668c0e0a ("drm/amd/display: avoid disable otg when dig was disabled")
> 	218784049f4b ("drm/amd/display: avoid disable otg when dig was disabled")
> 
> Now, if a new commit lands and it has the following "Fixes:" tag:
> 
> 	Fixes: abcdef12345 ("drm/amd/display: avoid disable otg when dig was disabled")

This is why we're asking people to include the cherry-picked from line, so
you're scripts can handle this automatically.

Because then you get two cherry-picked from lines in your stable commits:
- one from the drm cherry-pick
- one from the stable cherry-pick

And instead of only checking the stable cherry-pick line you just check
both if you want an answer to the "do I have this one already?" question.
There's two cases:

- You have a backport candidate, but want to check if you have it already.
  When that happens with the 2nd commit your scripts will try to apply
  that patch (because it doesn't match the cherry-picked from you've added
  yourself), which will fail and result in an angry mail to dri-devel.

  But if you instead check against both your and the drm cherry-pick
  lines, you'd know that you have this patch already and can drop it
  automatically.

- You get a Fixes: line like above, and want to know whether you need that
  patch. You already have to consult all the stable cherry-pick lines to
  make sure (because stable doesn't have that sha1 if the broken commit
  was itself cherry-picked). If you instead check against both the drm and
  stable cherry-pick lines then the tooling will do the right job.

Which is why Dave&me want these cherry-pick lines, but Alex has removed
them again because of the last round of shouting about this. Because
without cherry-pick lines you're down to guessing by title, which goes
wrong.

So the only thing that's needed in the tooling is that instead of only
looking at your own cherry-pick lines in stable commits to figure out
whether you need a backport or have it already, or whether you need that
bugfix or don't have the broken commit, is to look at all cherry-pick
lines. And ask Alex to again add them.

> Does it refer to one of the older commits? Or a new commit that will
> show up during the merge window?
> 
> Or... What happens if a new commit with the very same subject line shows
> up, and it has a cherry-pick link that points to a completely different
> commit that is not in the tree yet? :)
> 
> But just in general, there are so many odd combinations of commits where
> trying to follow the suggestion you've made will just break...
> 
> Something like these two identical commits which are not tagged for stable:
> 
> 	21afc872fbc2 ("drm/amd/display: Add monitor patch for specific eDP")
> 	3d71a8726e05 ("drm/amd/display: Add monitor patch for specific eDP")
 
Yeah sometimes people forget to add cc: stable. It happens. I don't think
anything else is going on here.

> And the following two identical ones which are tagged for stable:
> 
> 	b7cdccc6a849 ("drm/amd/display: Add monitor patch for specific eDP")
> 	04a59c547575 ("drm/amd/display: Add monitor patch for specific eDP")

Yeah this is just standard bugfix cherry-picking, except because you
shouted about the cherry-pick lines last time around they're now gone, so
you have no idea what's going on here.

Imo we should add the cherry-pick lines back and then this would be all
clear.
-Sima
-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux