Re: [ANN] Media Summit September 16th: Final Agenda (v7)

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thu, Sep 26, 2024 at 10:35:34AM +0000, Sakari Ailus wrote:
> 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@linux.intel.com> 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.

We have an explicit check in the CI to ensure that both the author and the
committer have a SoB line. It's not base on checkpatch.pl. Ricardo,
could you confirm this ?

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

That's not much different than someone impersonating me or you when
sending pull requests. Signing tags helps, but if someone steals
credentials, it's also game over. With the new workflow the malicious
changes will need to pass CI, so there may be more scrutiny :-)

> Reminding of
> <URL:https://github.com/lfit/itpol/blob/master/linux-workstation-security.md>
> in the instructions might not be a bad idea.

Agreed.

-- 
Regards,

Laurent Pinchart