Hi Laurent, On Thu, Sep 26, 2024 at 01:24:48PM +0300, Laurent Pinchart wrote: > On Thu, Sep 26, 2024 at 12:19:14PM +0200, Mauro Carvalho Chehab wrote: > > Em Thu, 26 Sep 2024 09:30:34 +0000 > > Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu: > > > > > > >>> Can you refresh my memory which processes need more work? > > > > >> > > > > >> I have the same doubt. As discussed during the summit, Hans and I had some > > > > >> discussions yesterday, to address a few details. For both of us the process > > > > >> sounds well defined. > > > > >> > > > > >> From my personal notes, this will be the new process: > > > > >> > > > > >> - committers will merge patches at media-committers.git tree at fdo, > > > > >> provided that they'll follow the rules defined on a committers agreement > > > > >> and (partially?) enforced by media-ci checks; > > > > >> - core committers follow the same rules, with a broader privilege of > > > > >> changing kernel API/ABI; > > > > >> - committers will ensure that patchwork will reflect the review process of > > > > >> the patches; > > > > >> - maintainers will double-check if everything is ok and, if ok, merge the > > > > >> changes at linuxtv.org. We intend to rename the tree there to "media.git", > > > > >> being the main tree to be used for development; > > > > >> - pull requests will keep using the same process as currently. The only > > > > >> change is that the media-stage.git tree will be renamed to "media.git"; > > > > >> - maintainers will periodically send patches upstream. > > > > >> > > > > >> The media-commiters.git tree at fdo might be rebased if needed; the > > > > >> media.git tree at linuxtv.org is stable. A large effort will be taken to > > > > >> avoid rebasing it. > > > > >> > > > > >> We may need some helper scripts and/or use pwclient to keep patchwork > > > > >> updated after committers reviews. > > > > > > > > > > What will happen if we update the status of patches in patchwork when > > > > > merging them to the fdo tree, and the tree is later rebased to drop > > > > > commits ? Will the person rebasing handle updating patchwork to move the > > > > > patches back from accepted to a different status ? > > > > > > > > That should be the responsibility of the person doing the rebase. I think > > > > that's what is done today as well in the rare cases we rebase. > > > > > > Sounds reasonable. I'd also like to avoid rebases as much as possible. > > > > > > Do we have a list of cases where a rebase would be needed? A license issue > > > or a missing Sob line, perhaps? > > > > No, and I don't think we can write a rule to cover such cases. The only rule > > is that it is up to maintainers to decide to do a rebase or not, and this > > will be done case by case. > > > > With regards to the cases you mentioned, it is almost surely that license > > issues will call for a rebase. The same may apply up to some point for > > missing/refused SoBs from authors, co-developers and from the committers. > > > > Yet, I would expect that a more common case is if someone touches the code > > and another committer/developer/author nacks with such changes. > > > > So, for instance, suppose you maintain driver A. Some other committer > > may end merging a patch for driver A without your ack. Depending on the > > patch that could be OK (trivial/doc changes, bugs with bug fixes that > > are available for some time, etc). > > > > Yet, even if the committer did an honest handling of the patch, you may > > still disagree or want some changes at the original patch. On such cases, > > the maintainers may decide to drop the changes and do a normal review > > process. They may otherwise request a patch on the top of the applied > > one to address the pointed issues. > > Let's do a revert in that case, and keep rebases for cases where having > content in the git history causes issues other than bisection problems. I'd very much prefer this as well: revert or fix, if at all reasonable, instead of rebasing should be a rule. > > I'd argue that even a missing SoB may not be a cause for rebase if it's > an accident, but that's not worth debating because CI will make sure > this never happens. Does it? checkpatch.pl checks should just be warnings. And that should probably stay. Sob: and From: being different isn't necessarily that far-fetched as having an address in .mailmap may change From: field but not Sob:, resulting in a checkpatch.pl warning. I wonder if checkpatch.pl should know about .mailmap actually, currently it doesn't. I could send a patch. > > > There is also worse case scenarios, like a committer violating the > > committer's agreement. > > I'm fine with rebases if someone gets rogue and merges malicious code, > or commits with insults in the commit message. I don't foresee that > happening regularly, if ever. I'm more concerned of a malicious actor getting access to the committer's credentials rather than the committer him-/herself going crazy. And if this happens, changes are it won't be noticed immediately. Reminding of <URL:https://github.com/lfit/itpol/blob/master/linux-workstation-security.md> in the instructions might not be a bad idea. -- Kind regards, Sakari Ailus
