Re: thoughts on a Merge Request based development workflow

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 15, 2019 at 3:56 AM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Mon, Oct 14, 2019 at 09:08:17PM +0200, Han-Wen Nienhuys wrote:
> > To 1) : Konstantin was worried about performance implication on git
> > notes.  The git-notes command stores data in a single
> > refs/notes/commits branch. Gerrit actually uses notes (the file
> > format) as well, but has a single notes branch per review, so
> > performance here is not a concern when scaling up the number of
> > reviews.
> >
> > To 2) : Google needs special magic sauce, because we service hundreds
> > of teams that work on thousands of repositories. However, here we're
> > talking about just the kernel itself; that is just a single
> > repository, and not an especially large one.  Chromium is our largest
> > repo, and it is about 10x larger than the linux kernel.
>
> I'd be concerned about cgit, because we need to have a separate file
> for the reviews, and you mean a single notes branch per review; a
> single patch series can have dozens of revisions, with potentially
> dozens of people commenting a large number of times, with e-mail
> threads that are hundreds of messages long.  If all of these changes
> are being squeezed into a single notes file, it would be quite large,
> and there would also be a lot of serialization concerns.  If you mean
> that there would be a single git note for each e-mail in a patch
> review thread.... that would seem to be a real potential problem for
> cgit.
>
> Be that as may, that's an optimization problem, and it is solveable,
> in the same way that most things are a Mere Matter of Programming.
> And if you're right, and it's not actually going to be a problem, then
> Huzzah!  But I suspect Konstantin's worries are probably ones we
> should at least pay attention to.
>
> > Gerrit isn't a big favorite of many people, but some of that
> > perception may be outdated. Since 2016, Google has significantly
> > increased its investment in Gerrit. For example, we have rewritten the
> > web UI from scratch, and there have been many performance
> > improvements.
>
> I agree that Gerrit might be a good starting point, having used it to
> review changes for Google's Data Center Kernels, as well as for
> Android and ChromeOS/Cloud Optimized System kernels.  Indeed, if I'm
> forced to use a non-threading mail user agent, it's far superior to
> e-mail reviews.
>
> Even if you have a threading mail agent, if everyone is using it, I'd
> argue that Gerrit is better, because it makes it really easy to look
> at the various versions of the patch series, including "give me the
> diff between the v3 and v7 version of the patch".  Having the
> conversation about a particular hunk of code in-line with the code
> itself is also very helpful.
>
> So let's talk about the sort of features that might need to be added
> to allow Gerrit to work for upstream development.
>
> > Gerrit has a patchset oriented workflow (where changes are amended all
> > the time), which is a good fit to the kernel's development process.
> > Linus doesn't like Change-Id lines, but I think we could adapt Gerrit
> > so it accepts URLs as IDs instead.
>
> Yep, I don't think this is hard.
>
> > There is talk of building a distributed/federated tool, but if there
> > are policies ("Jane Doe is maintainer of the network subsystem, and
> > can merge changes that only touch file in net/ "), then building
> > something decentralized is really hard. You have to build
> > infrastructure where Jane can prove to others who she is (PGP key
> > signing parties?), and some sort of distributed storage of the policy
> > rules.
>
> So requiring centralized authentication is going to be.... hard.
> There will certainly be some operations which will require
> authentication, sure.  But for things like:

What we do on gitlab.freedesktop.org is allow you to log in using
other hubs like github. As long as
patchwork/gerrit/whatever.kernel.org supports OAuth we could add a
simple "log in using kernel.org" button and it'd be a one-click thing.
And everyone can still choose where they want to have their main
account. Imo that's good enough (but I don't have strong opinions
here).

>   * Submitting a patch for review
>   * Making comments on a patch
>
> Adding a formal +1 or +2 vote, or actually approving that the patch be
> merged will obviously require authentication.  But as much as
> possible, a valid e-mail address should be all that's necessary for
> what people currently do using e-mail today.
>
> As far as a federated tool is concerned, I don't think we need to
> encode strict rules, because so long as we have a human (e.g., Linus)
> merging individual subsystem trees, I think we can let maintainers or
> maintainer groups (who, after all, today have absolutely control over
> their git trees) work out those issues amongst themselves, with an
> appeal to Linus to resolve conflicts and to make a final quality
> control check.
>
> Solving the problem of replacing how a maintainer or maintainer group
> reviews patches for their subsystem, and doing the review for patches
> that land in an a particular subsystem's git tree is a much simpler
> problem.  And if we can solve this, I think that's sufficient.
>
> But what this *does* mean is that sometimes patches will be cc'ed to
> multiple mailing lists, we need to map that into the gerrit world of a
> patch being cc'ed to multiple git trees.  The patch series might only
> end up landing in a single git tree, or it might be split up and with
> some commits landing in the ext4.git tree, and some in the btrfs.git
> tree, and some in the xfs.git tree, with some prerequisite patches
> landing on a separate branch of one of these trees, which the
> maintainers will merge into their trees.
>
> Today, this can be easily done by cc'ing the patch to multiple mailing
> lists.  Exactly how this works may get tricky, especially in the
> federated model where (for example) perhaps the btrfs tree might be
> administered by Facebook, while the xfs tree might be administrated by
> Red Hat.  Given that we *also* have to support people who want to keep
> using e-mail during the transition period, it may be that using
> unauthenticated e-mail messages where comments are attached quoted
> patch hunks, perhaps that can be the interchange format between
> different servers that aren't under a common administrative domain.

Last time I looked none of the common web ui tools (gerrit, gitlab,
github) had any reasonable support for topic branches/patch series
that target multiple different branches/repositories. They all assume
that a submission gets merged into one branch only and that's it. You
can of course submit the same stuff for inclusion into multiple
places, but that gives you separate discussion tracking for each one
(at least with the merge/pull request model, maybe gerrit is better
here), which is real bad.

> In *practice* hopefully most of the git/Gerrit trees will be
> administrated by Linux Foundation's kernel.org team.  But I think it's
> important that we support a distributed/federated model, as an
> insurance policy if nothing else.

If we go with any of the tooling changes discussed here I expect it'll
be a per-subsystem choice on what's the preferred thing, e.g. I don't
expect drm to move away from freedesktop.org because we have all our
userspace there. At that level interop with email-based pull requests
should be good enough. Better would be if you could somehow do
"foreign" submissions, which just point at a merge/pull request at the
other submission (maybe just use the git pull line) and internally
track/forward all comments or stuff like CI status. But I'm not aware
of any standard for this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch