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

[Sakari Ailus <sakari.ailus@linux.intel.com>]

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.

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