* [Qemu-devel] why restrict pull reqs to signed tags? @ 2016-03-09 10:20 Laszlo Ersek 2016-03-09 11:33 ` Paolo Bonzini 2016-03-09 11:35 ` Peter Maydell 0 siblings, 2 replies; 21+ messages in thread From: Laszlo Ersek @ 2016-03-09 10:20 UTC (permalink / raw) To: Peter Maydell Cc: Jordan Justen (Intel address), David Woodhouse, qemu devel list, Ard Biesheuvel Hi, the question in the subject is not loaded, it is not trying to suggest the opposite. It's a genuine question. If you have a few (tens of) minutes, please read through this sub-thread (or the entire discussion): http://thread.gmane.org/gmane.comp.bios.edk2.devel/8864/focus=8898 See also http://thread.gmane.org/gmane.linux.kernel/1855303/focus=2172858 Thanks Laszlo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 10:20 [Qemu-devel] why restrict pull reqs to signed tags? Laszlo Ersek @ 2016-03-09 11:33 ` Paolo Bonzini 2016-03-09 11:35 ` Peter Maydell 1 sibling, 0 replies; 21+ messages in thread From: Paolo Bonzini @ 2016-03-09 11:33 UTC (permalink / raw) To: Laszlo Ersek, Peter Maydell Cc: Jordan Justen (Intel address), David Woodhouse, qemu devel list, Ard Biesheuvel On 09/03/2016 11:20, Laszlo Ersek wrote: > Hi, > > the question in the subject is not loaded, it is not trying to suggest > the opposite. It's a genuine question. I think it boils down to this comment in the thread: "Signed pulls are about personal trust. If you have signed emails or a signed pull request from someone you trust sufficiently, you don't *need* to look at the content. There is no difference between email and pull requests, in this respect". Paolo > If you have a few (tens of) minutes, please read through this sub-thread > (or the entire discussion): > > http://thread.gmane.org/gmane.comp.bios.edk2.devel/8864/focus=8898 > > See also > > http://thread.gmane.org/gmane.linux.kernel/1855303/focus=2172858 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 10:20 [Qemu-devel] why restrict pull reqs to signed tags? Laszlo Ersek 2016-03-09 11:33 ` Paolo Bonzini @ 2016-03-09 11:35 ` Peter Maydell 2016-03-09 12:13 ` Laszlo Ersek 1 sibling, 1 reply; 21+ messages in thread From: Peter Maydell @ 2016-03-09 11:35 UTC (permalink / raw) To: Laszlo Ersek Cc: Jordan Justen (Intel address), David Woodhouse, qemu devel list, Ard Biesheuvel On 9 March 2016 at 17:20, Laszlo Ersek <lersek@redhat.com> wrote: > the question in the subject is not loaded, it is not trying to suggest > the opposite. It's a genuine question. So, with an initial disclaimer that we have to some extent cargo-culted our process here from the kernel, my view is: * we only take pull requests from known submaintainers (ie I will not take a pull request from an arbitrary person) * I don't do anything with pull requests beyond an automated build test and eyeball of the git log for any obvious howlers * a pull request is therefore equivalent to being able to directly commit to master, and so it's worth using the signed-tag machinery to ensure that we only give those rights to the people (submaintainers) we think we've given them to Conversely, a random set of patches sent to the list is supposed to be reviewed and tested by the submaintainer who applies them to their tree -- that is the gateway at which review happens. thanks -- PMM ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 11:35 ` Peter Maydell @ 2016-03-09 12:13 ` Laszlo Ersek 2016-03-09 12:19 ` Paolo Bonzini 2016-03-09 12:34 ` David Woodhouse 0 siblings, 2 replies; 21+ messages in thread From: Laszlo Ersek @ 2016-03-09 12:13 UTC (permalink / raw) To: Peter Maydell Cc: Jordan Justen (Intel address), Paolo Bonzini, David Woodhouse, qemu devel list, Ard Biesheuvel On 03/09/16 12:35, Peter Maydell wrote: > On 9 March 2016 at 17:20, Laszlo Ersek <lersek@redhat.com> wrote: >> the question in the subject is not loaded, it is not trying to suggest >> the opposite. It's a genuine question. > > So, with an initial disclaimer that we have to some extent cargo-culted > our process here from the kernel, my view is: > > * we only take pull requests from known submaintainers (ie I will > not take a pull request from an arbitrary person) > * I don't do anything with pull requests beyond an automated build > test and eyeball of the git log for any obvious howlers > * a pull request is therefore equivalent to being able to directly > commit to master, and so it's worth using the signed-tag machinery > to ensure that we only give those rights to the people (submaintainers) > we think we've given them to I understand, thank you. Especially your "directly commit to master" analogy is good. Pulling replaces your detailed personal review with the trusted identity of the pull requestor -- you trust that the commits on the requestor's branch are already sufficiently reviewed. http://thread.gmane.org/gmane.linux.kernel/1855303/focus=2172988 > Conversely, a random set of patches sent to the list is supposed > to be reviewed and tested by the submaintainer who applies them to > their tree -- that is the gateway at which review happens. This was my understanding, yes. David is proposing that direct pull requests be allowed on edk2-devel, immediately from contributors, so that the contributor may ask for his/her exact history to be preserved. I'm looking for examples: high profile projects that have adopted such a workflow *all the while* enforcing patch-wise reviews. Thus far I've come up empty. I think the idea we have thus far is: - submitter posts the patches - patches are reviewed on the list - submitter picks up the R-b, A-b, T-b labels - when converged, submitter sends a pull request with the labels applied, with the history he or she likes - maintainer fetches the branch, verifies that the commits indeed match the patches on list; also verifies that the labels have been correctly picked up from the list - maintainer merges the branch locally and pushes the merge commit (and its deps) to upstream master I feel a bit uncertain that we're trailblazing this workflow. It could work I guess. Thank you Laszlo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:13 ` Laszlo Ersek @ 2016-03-09 12:19 ` Paolo Bonzini 2016-03-09 12:31 ` Laszlo Ersek 2016-03-09 12:34 ` David Woodhouse 1 sibling, 1 reply; 21+ messages in thread From: Paolo Bonzini @ 2016-03-09 12:19 UTC (permalink / raw) To: Laszlo Ersek, Peter Maydell Cc: Jordan Justen (Intel address), David Woodhouse, qemu devel list, Ard Biesheuvel On 09/03/2016 13:13, Laszlo Ersek wrote: > David is proposing that direct pull requests be allowed on edk2-devel, > immediately from contributors, so that the contributor may ask for > his/her exact history to be preserved. I'm looking for examples: high > profile projects that have adopted such a workflow *all the while* > enforcing patch-wise reviews. Thus far I've come up empty. Ironically, projects using github pull requests do this. They do code review through the website and merge with a button. The resulting history is non-linear. Paolo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:19 ` Paolo Bonzini @ 2016-03-09 12:31 ` Laszlo Ersek 2016-03-09 12:33 ` Paolo Bonzini 2016-03-09 12:38 ` David Woodhouse 0 siblings, 2 replies; 21+ messages in thread From: Laszlo Ersek @ 2016-03-09 12:31 UTC (permalink / raw) To: Paolo Bonzini, Peter Maydell Cc: Jordan Justen (Intel address), David Woodhouse, qemu devel list, Ard Biesheuvel On 03/09/16 13:19, Paolo Bonzini wrote: > > > On 09/03/2016 13:13, Laszlo Ersek wrote: >> David is proposing that direct pull requests be allowed on edk2-devel, >> immediately from contributors, so that the contributor may ask for >> his/her exact history to be preserved. I'm looking for examples: high >> profile projects that have adopted such a workflow *all the while* >> enforcing patch-wise reviews. Thus far I've come up empty. > > Ironically, projects using github pull requests do this. They do code > review through the website and merge with a button. The resulting > history is non-linear. The website based review is a big minus: - email is more flexible for formulating a careful, detailed review - the review discussion is independently archived, not held hostage in a proprietary system The final result is also inferior I think: - the various feedback tags are not captured in the commit message of each individual patch We've been getting github pull requests for edk2. I'm always in a rush to reject them, lest another maintainer click the button out of oversight. I insist on keeping it all on-list. Thanks Laszlo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:31 ` Laszlo Ersek @ 2016-03-09 12:33 ` Paolo Bonzini 2016-03-09 12:38 ` David Woodhouse 1 sibling, 0 replies; 21+ messages in thread From: Paolo Bonzini @ 2016-03-09 12:33 UTC (permalink / raw) To: Laszlo Ersek, Peter Maydell Cc: Jordan Justen (Intel address), David Woodhouse, qemu devel list, Ard Biesheuvel On 09/03/2016 13:31, Laszlo Ersek wrote: > > Ironically, projects using github pull requests do this. They do code > > review through the website and merge with a button. The resulting > > history is non-linear. > > The website based review is a big minus: > - email is more flexible for formulating a careful, detailed review > - the review discussion is independently archived, not held hostage in > a proprietary system > > The final result is also inferior I think: > - the various feedback tags are not captured in the commit message of > each individual patch > > We've been getting github pull requests for edk2. I'm always in a rush > to reject them, lest another maintainer click the button out of > oversight. I insist on keeping it all on-list. I agree, hence the "Ironically" part. Still, the point remains that github pull requests result in a much more non-linear history than Linux or QEMU. That said, it also doesn't do exactly what David says, because the tags are recorded in the web interface only---not in the commit. Paolo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:31 ` Laszlo Ersek 2016-03-09 12:33 ` Paolo Bonzini @ 2016-03-09 12:38 ` David Woodhouse 2016-03-09 12:40 ` Ard Biesheuvel 1 sibling, 1 reply; 21+ messages in thread From: David Woodhouse @ 2016-03-09 12:38 UTC (permalink / raw) To: Laszlo Ersek, Paolo Bonzini, Peter Maydell Cc: Jordan Justen (Intel address), qemu devel list, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 464 bytes --] On Wed, 2016-03-09 at 13:31 +0100, Laszlo Ersek wrote: > > > The website based review is a big minus: > - email is more flexible for formulating a careful, detailed review Well, assuming people are capable of using email competently. Which isn't necessarily the case in the EDK2 world. But anyway, I don't think anyone is suggesting that we *not* require patches to be posted in email for review. So that's something of a digression., -- dwmw2 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5691 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:38 ` David Woodhouse @ 2016-03-09 12:40 ` Ard Biesheuvel 2016-03-09 12:44 ` Peter Maydell 0 siblings, 1 reply; 21+ messages in thread From: Ard Biesheuvel @ 2016-03-09 12:40 UTC (permalink / raw) To: David Woodhouse Cc: Paolo Bonzini, Laszlo Ersek, qemu devel list, Jordan Justen (Intel address), Peter Maydell On 9 March 2016 at 19:38, David Woodhouse <dwmw2@infradead.org> wrote: > On Wed, 2016-03-09 at 13:31 +0100, Laszlo Ersek wrote: >> >> >> The website based review is a big minus: >> - email is more flexible for formulating a careful, detailed review > > Well, assuming people are capable of using email competently. Which > isn't necessarily the case in the EDK2 world. But anyway, I don't think > anyone is suggesting that we *not* require patches to be posted in > email for review. So that's something of a digression., > Laszlo is going to shoot me for saying this, but as a compromise between the Windows crowd in the Intel firmware team and the more Linux/open source oriented people on the opposite side, using github does not sound like an entirely unreasonable compromise. At least it means no emails anymore with garbled patches and 300 character lines in the commit log ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:40 ` Ard Biesheuvel @ 2016-03-09 12:44 ` Peter Maydell 2016-03-09 13:14 ` Laszlo Ersek 0 siblings, 1 reply; 21+ messages in thread From: Peter Maydell @ 2016-03-09 12:44 UTC (permalink / raw) To: Ard Biesheuvel Cc: Laszlo Ersek, David Woodhouse, qemu devel list, Jordan Justen (Intel address), Paolo Bonzini On 9 March 2016 at 19:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Laszlo is going to shoot me for saying this, but as a compromise > between the Windows crowd in the Intel firmware team and the more > Linux/open source oriented people on the opposite side, using github > does not sound like an entirely unreasonable compromise. At least it > means no emails anymore with garbled patches and 300 character lines > in the commit log By the way, why are you all discussing the EDK2 patch workflow proposals on qemu-devel and not on an EDK mailing list? thanks -- PMM ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:44 ` Peter Maydell @ 2016-03-09 13:14 ` Laszlo Ersek 0 siblings, 0 replies; 21+ messages in thread From: Laszlo Ersek @ 2016-03-09 13:14 UTC (permalink / raw) To: Peter Maydell, Ard Biesheuvel Cc: Paolo Bonzini, David Woodhouse, qemu devel list, Jordan Justen (Intel address) On 03/09/16 13:44, Peter Maydell wrote: > On 9 March 2016 at 19:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> Laszlo is going to shoot me for saying this, but as a compromise >> between the Windows crowd in the Intel firmware team and the more >> Linux/open source oriented people on the opposite side, using github >> does not sound like an entirely unreasonable compromise. At least it >> means no emails anymore with garbled patches and 300 character lines >> in the commit log > > By the way, why are you all discussing the EDK2 patch workflow > proposals on qemu-devel and not on an EDK mailing list? Sorry, that's where we started, and the rest of the discussion should happen there; I agree. The reason for coming here was that we ended up questioning / re-examining the practices we had taken as examples: QEMU, Linux. I needed some info from the source, and I wanted to allow anyone on qemu-devel to chime in. (This is why I didn't ask only you personally, edk2-devel CC'd.) Thanks Laszlo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:13 ` Laszlo Ersek 2016-03-09 12:19 ` Paolo Bonzini @ 2016-03-09 12:34 ` David Woodhouse 2016-03-09 12:42 ` Peter Maydell 1 sibling, 1 reply; 21+ messages in thread From: David Woodhouse @ 2016-03-09 12:34 UTC (permalink / raw) To: Laszlo Ersek, Peter Maydell Cc: Jordan Justen (Intel address), Paolo Bonzini, qemu devel list, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 4282 bytes --] On Wed, 2016-03-09 at 13:13 +0100, Laszlo Ersek wrote: > I understand, thank you. Especially your "directly commit to master" > analogy is good. Pulling replaces your detailed personal review with the > trusted identity of the pull requestor -- you trust that the commits on > the requestor's branch are already sufficiently reviewed. Note that it doesn't *have* to. And again, there's nothing special about email vs. pull requests here. Pater is saying that he *chooses* not to bother reviewing what he pulls in via pull requests, and *that's* why it's equivalent to direct commit access. I could just as well say that *I* choose to hold my nose and look the other way while running 'git am', and thus *patches* would be equivalent to direct commit access. I won't tell Peter that his behaviour is wrong. I'll only say that other projects don't have to do the same thing. And repeat that from the trust point of view, there is *nothing* fundamentally different about patches vs. pull requests. > http://thread.gmane.org/gmane.linux.kernel/1855303/focus=2172988 > > > > > Conversely, a random set of patches sent to the list is supposed > > to be reviewed and tested by the submaintainer who applies them to > > their tree -- that is the gateway at which review happens. > This was my understanding, yes. > > David is proposing that direct pull requests be allowed on edk2-devel, > immediately from contributors, so that the contributor may ask for > his/her exact history to be preserved. I'm looking for examples: high > profile projects that have adopted such a workflow *all the while* > enforcing patch-wise reviews. Thus far I've come up empty. > > I think the idea we have thus far is: > > - submitter posts the patches > - patches are reviewed on the list > - submitter picks up the R-b, A-b, T-b labels (as well as any other feedback, of course) > - when converged, submitter sends a pull request with the labels > applied, with the history he or she likes > - maintainer fetches the branch, verifies that the commits indeed match > the patches on list; also verifies that the labels have been correctly > picked up from the list All of which the maintainer is already expected to do, right? Except instead of 'fetches the branch' the maintainer is currently applying the patches in his/her *own* mailbox, potentially to a current master on which they don't actually work, and then doing the rest of what you said. > - maintainer merges the branch locally and pushes the merge commit (and > its deps) to upstream master Well, the above two steps would be 'pull, then look'. I don't think explicit messing with topic branches and manual merges would be necessary. You do a 'git pull $SUBMITTED_TREE'. If you like what you get, you then just do a 'git push'. If you don't, 'git reset --hard origin' and send an email saying why it was rejected. > I feel a bit uncertain that we're trailblazing this workflow. It could > work I guess. We wouldn't be trailblazing it at all. It's done all the time in Linux and various other projects. It's just that the 'submitter' rôle is split between individual contributors, and subsystem maintainers. > - submitter posts the patches > - patches are reviewed on the list > - submitter picks up the R-b, A-b, T-b labels In fact either the submitter will pick them up when sending a second round of patches (if there are any *other* changes to make), or the subsystem maintainer will pick them up (via patchwork, usually) when applying the patches to the subsystem tree. > - when converged, submitter sends a pull request with the labels applied, with the history he or she likes The subsystem maintainer sends the pull request to Linus. > - maintainer fetches the branch, verifies that the commits indeed match > the patches on list; also verifies that the labels have been correctly > picked up from the list > - maintainer merges the branch locally and pushes the merge commit > (and its deps) to upstream master Linus does a test pull, and either likes what he sees, or doesn't. Or depending on who it comes from and how much he cared about the code being modified, doesn't look too hard. -- dwmw2 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5691 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:34 ` David Woodhouse @ 2016-03-09 12:42 ` Peter Maydell 2016-03-09 13:09 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Peter Maydell @ 2016-03-09 12:42 UTC (permalink / raw) To: David Woodhouse Cc: Jordan Justen (Intel address), Paolo Bonzini, Laszlo Ersek, qemu devel list, Ard Biesheuvel On 9 March 2016 at 19:34, David Woodhouse <dwmw2@infradead.org> wrote: > On Wed, 2016-03-09 at 13:13 +0100, Laszlo Ersek wrote: >> I understand, thank you. Especially your "directly commit to master" >> analogy is good. Pulling replaces your detailed personal review with the >> trusted identity of the pull requestor -- you trust that the commits on >> the requestor's branch are already sufficiently reviewed. > > Note that it doesn't *have* to. And again, there's nothing special > about email vs. pull requests here. > > Pater is saying that he *chooses* not to bother reviewing what he pulls > in via pull requests, and *that's* why it's equivalent to direct commit > access. > > I could just as well say that *I* choose to hold my nose and look the > other way while running 'git am', and thus *patches* would be > equivalent to direct commit access. > > I won't tell Peter that his behaviour is wrong. I'll only say that > other projects don't have to do the same thing. Yes; I'm describing QEMU's workflow, not trying to present it as the One True Way of doing things. I would quibble with the "not to bother" phrasing, though -- if I reviewed everything going into master this would not scale and I would very quickly burn out and go spend my time studying Japanese instead. The pullreqs-from- submaintainers setup is specifically intended to spread that review and test workload out to a wider circle of people (who also have local-subject-area expertise which I don't have). That I don't review the patches flowing into master via the pulls I merge is a feature of this workflow, not a bug. thanks -- PMM ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 12:42 ` Peter Maydell @ 2016-03-09 13:09 ` David Woodhouse 2016-03-09 13:27 ` Peter Maydell 0 siblings, 1 reply; 21+ messages in thread From: David Woodhouse @ 2016-03-09 13:09 UTC (permalink / raw) To: Peter Maydell Cc: Jordan Justen (Intel address), Paolo Bonzini, Laszlo Ersek, qemu devel list, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 998 bytes --] On Wed, 2016-03-09 at 19:42 +0700, Peter Maydell wrote: > > Yes; I'm describing QEMU's workflow, not trying to present it as > the One True Way of doing things. I would quibble with the > "not to bother" phrasing, though -- if I reviewed everything going > into master this would not scale and I would very quickly burn out > and go spend my time studying Japanese instead. The pullreqs-from- > submaintainers setup is specifically intended to spread that > review and test workload out to a wider circle of people (who > also have local-subject-area expertise which I don't have). > That I don't review the patches flowing into master via the > pulls I merge is a feature of this workflow, not a bug. Yeah, but the important criterion is *who* the thing comes from (and again, a signed git tag is just as good as a set of signed emails). It *isn't* about pull vs. email. That's just the transport mechanism. There may be a correlation, but it isn't important. -- dwmw2 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5691 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 13:09 ` David Woodhouse @ 2016-03-09 13:27 ` Peter Maydell 2016-03-09 14:13 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Peter Maydell @ 2016-03-09 13:27 UTC (permalink / raw) To: David Woodhouse Cc: Jordan Justen (Intel address), Paolo Bonzini, Laszlo Ersek, qemu devel list, Ard Biesheuvel On 9 March 2016 at 20:09, David Woodhouse <dwmw2@infradead.org> wrote: > Yeah, but the important criterion is *who* the thing comes from (and > again, a signed git tag is just as good as a set of signed emails). Well, it's also important to me that it's easy to apply stuff and that it comes in a single lump that's large enough that I don't have a lot of overhead in processing it. Sure, you could gpg sign individual patch mails and then check signatures when doing git am, but I don't do that because it would be a complete pain (and I'm not sure git has built-in tooling for doing it the way it does with gpg signed tags and merges). So I definitely would bounce an attempt by a submaintainer to send me stuff as a pile of signed patchmails. > It *isn't* about pull vs. email. That's just the transport mechanism. > There may be a correlation, but it isn't important. Right, but Laszlo didn't ask "why pull requests", he asked "why signed pull requests", to which the answer is "because of the trust implied by the way our workflow uses pullreqs". thanks -- PMM ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 13:27 ` Peter Maydell @ 2016-03-09 14:13 ` David Woodhouse 2016-03-09 14:41 ` Laszlo Ersek 0 siblings, 1 reply; 21+ messages in thread From: David Woodhouse @ 2016-03-09 14:13 UTC (permalink / raw) To: Peter Maydell Cc: Ard Biesheuvel, David Woodhouse, qemu devel list, Jordan Justen (Intel address), Paolo Bonzini, Laszlo Ersek > On 9 March 2016 at 20:09, David Woodhouse <dwmw2@infradead.org> wrote: >> Yeah, but the important criterion is *who* the thing comes from (and >> again, a signed git tag is just as good as a set of signed emails). > > Well, it's also important to me that it's easy to apply stuff > and that it comes in a single lump that's large enough that I > don't have a lot of overhead in processing it. Sure, you could > gpg sign individual patch mails and then check signatures when > doing git am, but I don't do that because it would be a complete > pain (and I'm not sure git has built-in tooling for doing it > the way it does with gpg signed tags and merges). So I definitely > would bounce an attempt by a submaintainer to send me stuff > as a pile of signed patchmails. Sure, pull requests are better than email in fairly much every way :) >> It *isn't* about pull vs. email. That's just the transport mechanism. >> There may be a correlation, but it isn't important. > > Right, but Laszlo didn't ask "why pull requests", he asked > "why signed pull requests", to which the answer is "because > of the trust implied by the way our workflow uses pullreqs". I believe he saw the discussion of *signed* pull requests and was concerned that pull requests were somehow dangerous in a way that required extra validation before you could touch them. When in fact that's not the case at all. The use of signatures permits personal trust to eliminate the additional checking at the time you merge -- but that's a completely orthogonal thing which *can* also apply with emails (although less easily as you observe). The background is that they currently use a workshop which enforces rebasing onto the latest master, thus leading to lost history and the potential for commits which apparently *never* worked, in cases when we *should* have a working commit and a subsequently broken merge. And I'm trying to get them to fix that and use git properly, :) -- dwmw2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 14:13 ` David Woodhouse @ 2016-03-09 14:41 ` Laszlo Ersek 2016-03-10 8:21 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Laszlo Ersek @ 2016-03-09 14:41 UTC (permalink / raw) To: David Woodhouse, Peter Maydell Cc: Jordan Justen (Intel address), Paolo Bonzini, qemu devel list, Ard Biesheuvel On 03/09/16 15:13, David Woodhouse wrote: > The background is that they currently use a workshop which enforces > rebasing onto the latest master, thus leading to lost history This is exactly what the QEMU development model enforces, as I've repeated many times. Maintainers who review and pick up your QEMU patches will freely rebase them, reorder them against other contributors' series they queue until their next pull, and resolve rebase conflicts without asking you if they can. What I may have forgotten to say (but have been reminded of) is that maintainers are personally responsible for *testing* such rebases before they send a PULL with them. Nonetheless, I don't think such testing would make much difference for you, because (a) the history would be changed anyway, which you can't accept, and (b) IIRC I volunteered to test your (great) OpenSSL work in practice even before you did, even without being a CryptoPkg co-maintainer, but that still doesn't seem to give you enough confidence in our workflow. My point is that the workflow we currently use in edk2 is not *inherently* broken. Many other projects, like QEMU, use it. > and the > potential for commits which apparently *never* worked, in cases when we > *should* have a working commit and a subsequently broken merge. And I'm > trying to get them to fix that and use git properly, :) According to your description, QEMU uses git improperly, so does libvirt, and the KVM (kernel) queue too. Anyway, I think we're running in circles. I won't try to block this endeavor in edk2. If (a) merges become generally accepted by the other package maintainers in edk2, and (b) a contributor asks me personally that his branch be pulled rather than rebased, I'll do my best to conform, while ensuring correctness & security. Until (a&&b), I as an edk2 co-maintainer will definitely follow the current rules. I apologize again for the noise on qemu-devel; I'm out. Laszlo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-09 14:41 ` Laszlo Ersek @ 2016-03-10 8:21 ` David Woodhouse 2016-03-10 8:52 ` Markus Armbruster 0 siblings, 1 reply; 21+ messages in thread From: David Woodhouse @ 2016-03-10 8:21 UTC (permalink / raw) To: Laszlo Ersek, Peter Maydell Cc: Jordan Justen (Intel address), Paolo Bonzini, qemu devel list, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 2129 bytes --] On Wed, 2016-03-09 at 15:41 +0100, Laszlo Ersek wrote: > According to your description, QEMU uses git improperly, so does > libvirt, and the KVM (kernel) queue too. Let's see... Imagine I'm working on a new feature, and I submit a carefully tested sequence of commits in a pull request. Someone *rebases* my work onto a different point in history, which introduces a bug. The record now shows that I submitted something *broken*. Like https://github.com/openssl/openssl/commit/963bb621 for example, which is utterly hosed and broke the build for everyone except me. On that occasion, I was able to look back at what I *actually* submitted and point out that it wasn't my fault. But sometimes the breakage is more subtle. You end up looking back a few months later and trying to work out why an esoteric corner case is failing. You might ask me, and I'll say that I *distinctly* remember I thought about it, and that I could have sworn I *had* tested it.... But again the record shows that what got merged has *never* worked. That's no longer just a problem of embarrassment for the submitter, by misrepresenting their work. That's now causing problems for the *project*. Because if history had been represented *correctly*, with a working commit and then a subsequent merge introducing the breakage, then that is a *whole* lot easier to figure out. Preserving accurate history is a large part of the reason we *have* version control systems. And yes, if any of those projects you list are deliberately throwing away history as a matter of course on non-trivial submissions, then I would say that they are using the tools improperly. Of course, for simple patches it often makes no difference (well, apart from the OpenSSL example I gave above). And for larger submissions it doesn't *often* make a difference. But that's not the point. Sure, let people apply their *discretion* about rebasing stuff, if you really must — but a workflow which *enforces* a rebase, and *never* lets you pull a complex submission as it *actually* happened, is quite wrong. -- dwmw2 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5691 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-10 8:21 ` David Woodhouse @ 2016-03-10 8:52 ` Markus Armbruster 2016-03-10 10:34 ` David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Markus Armbruster @ 2016-03-10 8:52 UTC (permalink / raw) To: David Woodhouse Cc: Peter Maydell, Ard Biesheuvel, Jordan Justen (Intel address), qemu devel list, Paolo Bonzini, Laszlo Ersek David Woodhouse <dwmw2@infradead.org> writes: > On Wed, 2016-03-09 at 15:41 +0100, Laszlo Ersek wrote: >> According to your description, QEMU uses git improperly, so does >> libvirt, and the KVM (kernel) queue too. > > Let's see... > > Imagine I'm working on a new feature, and I submit a carefully tested > sequence of commits in a pull request. > > Someone *rebases* my work onto a different point in history, which > introduces a bug. > > The record now shows that I submitted something *broken*. Like > https://github.com/openssl/openssl/commit/963bb621 for example, which > is utterly hosed and broke the build for everyone except me. > > On that occasion, I was able to look back at what I *actually* > submitted and point out that it wasn't my fault. But sometimes the > breakage is more subtle. You end up looking back a few months later and > trying to work out why an esoteric corner case is failing. You might > ask me, and I'll say that I *distinctly* remember I thought about it, > and that I could have sworn I *had* tested it.... > > But again the record shows that what got merged has *never* worked. > That's no longer just a problem of embarrassment for the submitter, by > misrepresenting their work. That's now causing problems for the > *project*. Because if history had been represented *correctly*, with a > working commit and then a subsequent merge introducing the breakage, > then that is a *whole* lot easier to figure out. > > Preserving accurate history is a large part of the reason we *have* > version control systems. And yes, if any of those projects you list are > deliberately throwing away history as a matter of course on non-trivial > submissions, then I would say that they are using the tools improperly. > > Of course, for simple patches it often makes no difference (well, apart > from the OpenSSL example I gave above). And for larger submissions it > doesn't *often* make a difference. But that's not the point. Sure, let > people apply their *discretion* about rebasing stuff, if you really > must — but a workflow which *enforces* a rebase, and *never* lets you > pull a complex submission as it *actually* happened, is quite wrong. Strawman alert: we don't *enforce* rebase. We leave it to the maintainer's discretion. Nothing stops a maintainer (or a chain of them) from accepting pull requests. But for better or worse, most maintainers rebase most of the time. When they do, they add their S-o-B to every patch, which provides a measure of accountability. I acknowledge your points are worth considering, even though you exaggerated for effect :) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-10 8:52 ` Markus Armbruster @ 2016-03-10 10:34 ` David Woodhouse 2016-03-10 12:38 ` Laszlo Ersek 0 siblings, 1 reply; 21+ messages in thread From: David Woodhouse @ 2016-03-10 10:34 UTC (permalink / raw) To: Markus Armbruster Cc: Peter Maydell, Ard Biesheuvel, Jordan Justen (Intel address), qemu devel list, Paolo Bonzini, Laszlo Ersek [-- Attachment #1: Type: text/plain, Size: 524 bytes --] On Thu, 2016-03-10 at 09:52 +0100, Markus Armbruster wrote: > Strawman alert: we don't *enforce* rebase. We leave it to the > maintainer's discretion. Nothing stops a maintainer (or a chain of > them) from accepting pull requests. Which is all I was asking EDK2 to do. They *do* enforce rebase, which is wrong. Laszlo appeared to be saying "but qemu works like this; are they wrong too?". To which the answer is apparently "no, they don't work like this." Thanks for clearing that up. -- dwmw2 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 5691 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] why restrict pull reqs to signed tags? 2016-03-10 10:34 ` David Woodhouse @ 2016-03-10 12:38 ` Laszlo Ersek 0 siblings, 0 replies; 21+ messages in thread From: Laszlo Ersek @ 2016-03-10 12:38 UTC (permalink / raw) To: David Woodhouse, Markus Armbruster Cc: Peter Maydell, Paolo Bonzini, qemu devel list, Jordan Justen (Intel address), Ard Biesheuvel On 03/10/16 11:34, David Woodhouse wrote: > On Thu, 2016-03-10 at 09:52 +0100, Markus Armbruster wrote: >> Strawman alert: we don't *enforce* rebase. We leave it to the >> maintainer's discretion. Nothing stops a maintainer (or a chain of >> them) from accepting pull requests. > > Which is all I was asking EDK2 to do. They *do* enforce rebase, which > is wrong. > > Laszlo appeared to be saying "but qemu works like this; are they wrong > too?". > > To which the answer is apparently "no, they don't work like this." > > Thanks for clearing that up. Markus, thank you for clearing that up. I failed to distinguish "enforcement" from "practice that is applied in 99.999% of the time". David, it doesn't change anything relative to one of my earliest emails: http://thread.gmane.org/gmane.comp.bios.edk2.devel/8864/focus=8889 I personally agreed to your proposal very early (and have repeated that agreement a few times since), dependent on agreement from the other edk2 maintainers too. The linear history requirement is not mine in edk2. I don't enforce it, I comply with it. In my "unkempt" guide, I relay that requirement, don't dictate it. My explanation of it may not have been entirely correct, yes. However, Jordan also told you that it is temporary, while the edk2 people's git expertise matures. If you want to gather feedback on immediately introducing a workflow to edk2 that allows merges, please write a focused group email to the maintainers listed in "Maintainers.txt". Some of them might feel better about discussing this question, and/or feel more closely addressed, if it doesn't happen on the list. Going forward, please refrain from over-using your "cluebat" (e.g., the tons of bold in your email). Discussing workflow is hard enough in its own right, you don't need to make it harder by alienating people. Linus gets away with management by perkele because he's a chief maintainer. In this case, it's you who wants to achieve something, even if you position it as "fixing the workflow for everyone". You are right about merging (and I never denied that), but I do find myself struggling harder and harder to open your next email. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-03-10 12:39 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-09 10:20 [Qemu-devel] why restrict pull reqs to signed tags? Laszlo Ersek 2016-03-09 11:33 ` Paolo Bonzini 2016-03-09 11:35 ` Peter Maydell 2016-03-09 12:13 ` Laszlo Ersek 2016-03-09 12:19 ` Paolo Bonzini 2016-03-09 12:31 ` Laszlo Ersek 2016-03-09 12:33 ` Paolo Bonzini 2016-03-09 12:38 ` David Woodhouse 2016-03-09 12:40 ` Ard Biesheuvel 2016-03-09 12:44 ` Peter Maydell 2016-03-09 13:14 ` Laszlo Ersek 2016-03-09 12:34 ` David Woodhouse 2016-03-09 12:42 ` Peter Maydell 2016-03-09 13:09 ` David Woodhouse 2016-03-09 13:27 ` Peter Maydell 2016-03-09 14:13 ` David Woodhouse 2016-03-09 14:41 ` Laszlo Ersek 2016-03-10 8:21 ` David Woodhouse 2016-03-10 8:52 ` Markus Armbruster 2016-03-10 10:34 ` David Woodhouse 2016-03-10 12:38 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).