On Wed, 2016-03-09 at 00:05 +0100, Laszlo Ersek wrote: > (1) The submitter is himself/herself responsible for picking up review > tags, and then for posting a final (fully reviewed) PULL that can be > merged without *any* kind of rebase by the pulling maintainer. > > Corollary: since the first submission never has any reviews, the first > submission should never be a PULL.
If submissions *require* review, the first submission isn't intended to be merged anyway. So it's meaningless to say *either* that the first submission is to be merged by applying patches, *or* that it's to be merged as a pull. This is fairly much a non-sequitur. > It seems to me that the extra trust for a pull request is needed because > it's possible to post patches to the list (for review) that *differ* > from the commits that *actually* lead up to the hash that is presented > in the pull request. An attacker can prepare two patch sets, an innocent > and a malicious one; post one series for review, but include the final > commit hash of the other series in the pull request. > > Whereas when the maintainer applies patches from the list (i.e., at that > point: from his own mailbox) directly, then the maintainer can be > (reasonably) sure that those are exactly the patches the community > reviewed (not counting mailbox break-ins etc). Nonsense. If I go through seven rounds of posting patches to the list, and I gather a series of acks from the first six but you apply the seventh, you have *no* good reason to believe that my final submission doesn't contain the trojan horse. You have to *look* before you apply it. And it's *just* the same if my final submission is actually in the form of a pull request. Sure, the signed tag means you know it comes from *me*, which can be achieved by signed email too (like this one). But you still have to trust *me* just the same. Or actually do a final review on the code you're actually *merging*. Whether it be in email patches or in a pull request, that's just the same. Really, from the trust point of view there is *not* a difference between pull requests and patches. Look at what you merge, or trust the submitter. It's as simple as that. > ... So, I dunno what to do about this. I trust you, but I wouldn't want > to open up the possibility of *any* maintainer pulling from *any* > contributor. > > * How about an alternative. > > (a) Contributors are encouraged to name the fork-off point (commit hash) > of their topic branch, from which they post their patches. > > (b) For a large series, the maintainer is expected to apply the patches > on a local, temporary branch, forked off of exactly the named commit of > the master branch. Review tags are also added on this branch. > > (c) Maintainer merges topic into master locally. > > (d) Maintainer pushes the merge commit to his or her personal repo. > > (e) Submitter fetches maintainer's personal repo, and confirms (in > email, stating the hash of the merge commit) that it's fine. > > (f) Maintainer pushes the merge commit (and its dependencies of course) > to upstream master. > > Now, steps (d) and (e) add a round-trip that can make the merge commit > prepared in step (c) obsolete, by the time the confirmation arrives. So > perhaps we can drop (d) and (e). > > The point is, your history would be precisely reconstructed, and you > could get to say the final OK. No, this is pointless complexity. Again, just *look* at what you merge. Whether it means a proper read through the seventh round of patches, or actually looking at the contents of a pull request, there's *no* difference. > * There is another alternative. You should become an official > co-maintainer of CryptoPkg (using your Intel email address), and then > you could push your merges directly, after review. As I said, I trust > you, but I don't trust a setup where any maintainer can pull from any > contributor. That's a band-aid for the one issue. I'm trying to make things better for *everyone*. -- dwmw2
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
