Re: merging pull requests

From: Kees Cook <keescook@chromium.org>
To: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Cc: tools@linux.kernel.org, users@linux.kernel.org
Subject: Re: merging pull requests
Date: Thu, 30 Sep 2021 16:09:13 -0700	[thread overview]
Message-ID: <202109301559.A9BFB03@keescook> (raw)
In-Reply-To: <20210930200002.67vxbowvegso2zhg@meerkat.local>

On Thu, Sep 30, 2021 at 04:00:02PM -0400, Konstantin Ryabitsev wrote:
> On Thu, Sep 30, 2021 at 10:33:56AM -0700, Kees Cook wrote:
> > Hi!
> > 
> > I'm just realized that all my workflows have been entirely mbox patch
> > sets, and I finally have a pull request to take, and ... I have no idea
> > how to do it "right". :P
> > 
> > I see "b4 pr", but I was hoping for some kind of three-step process:
> > 1) get the code
> > 2) read/verify/test code
> > 3) merge code into main brain
> 
> Glad you're still around and doing well, professor Nightingale. ;)

3 weeks of virtual conferences is wrecking my branch.

> I'm going to cc this to users@, since it's likely to reach more
> maintainers' eyes (and brains) there.

Good idea; thanks!

> > And that it would end up looking like what I see from Linus (i.e. naming
> > branching after the tags sent, etc):
> > 
> > 6e439bbd436e Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
> > a4e6f95a891a Merge tag 'pinctrl-v5.15-2' of git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
> > 62da74a73570 Merge tag 'vfio-v5.15-rc4' of git://github.com/awilliam/linux-vfio
> > e7bd807e8c9e Merge tag 'm68k-for-v5.15-tag3' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k
> > dca50f08a03e Merge tag 'nios2_fixes_for_v5.15_part1' of git://git.kernel.org/pub/scm/linux/kernel/git/dinguyen/linux
> > 
> > It seems like "b4 pr" does part of step 1, but doesn't build a branch
> > from the tag (though one can use "-b").
> > 
> > And for step 3, when I do a "git merge --no-ff $branch", all the tag and
> > origin (e.g. "tag 'pinctrl-v5.15-2' of git://..." is missing).
> 
> It shouldn't be if you leave it in FETCH_HEAD, which is what b4 does by
> default. So, if you do "b4 pr" and then follow it by "git merge", it should
> retain the pull request origin and make it the default subject there.

I guess it depends on the expected order of operations. What do other
maintainers do to process a PR? I would think it would be:

- pull remote branch (to FETCH_HEAD)
- review (in FETCH_HEAD? in a "real" branch?)
- merge into my topic branch (where the commit subject should retain details
  of the pull location, body has notes from the signed tag, etc...)
- review, build, and test
- push to public tree

> I just added some merge message template processing for "b4 shazam", so we can
> probably do the same for "b4 pr" if that makes sense. Should b4 grab the
> contents of the pull request and pre-stuff them into a file you can pass to
> git-merge?

I think so -- that seems to be sort of what Linus does? There seem to be
a few templates (tag, branch, and "patches from Andrew"), e.g.:

Merge tag 'char-misc-5.15-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
Merge branch 'misc.namei' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Merge branch 'akpm' (patches from Andrew)

Though I'd love to know why there's a distinction between "tag" and
"branch" here. Are "branch" pulls not checked for signatures?

-- 
Kees Cook