From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.9156.1624461425427783655 for ; Wed, 23 Jun 2021 08:17:05 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Wbspxbr5; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624461424; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=FzckP9CSuAP6zK9GE7rW4GIICrHS1u9/tx3G6IuGMu4=; b=Wbspxbr5b7pB4m53LO9zVDaNZoq120sEXiXBTza/QkacaxZiiF1RlkiaOQ7UjFaP+kWAHf XEcOPhpWEcTUVQhdzXjLo2plzmUUqrrwlu4ZJqfq+4+r6JPUa89jVqoBXqlOr8V0gn1SoZ Is7oEw05TSlbRMd2tGAp8Cn1M45RdEU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-436-LH_IgzQJOSOzmhfNPGMKog-1; Wed, 23 Jun 2021 11:17:01 -0400 X-MC-Unique: LH_IgzQJOSOzmhfNPGMKog-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 36181809DDD; Wed, 23 Jun 2021 15:16:45 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-200.ams2.redhat.com [10.36.112.200]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 330D926FDD; Wed, 23 Jun 2021 15:16:41 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants To: "Kinney, Michael D" , "devel@edk2.groups.io" , "spbrogan@outlook.com" , "ardb@kernel.org" Cc: Peter Grehan , Ard Biesheuvel , "Justen, Jordan L" , Sean Brogan , Rebecca Cran References: <20210612204340.52290-1-rebecca@bsdio.com> <8539c5bc-0092-0c48-7555-76dafcfe18ba@redhat.com> From: "Laszlo Ersek" Message-ID: <6e8381ff-d7bc-655e-4794-3ffa0b548ac3@redhat.com> Date: Wed, 23 Jun 2021 17:16:40 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Groupsio-MsgNum: 76997 Content-Type: multipart/mixed; boundary="------------F596473B18230618A268543B" Content-Language: en-US --------------F596473B18230618A268543B Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 06/22/21 17:38, Kinney, Michael D wrote: > Hi Laszlo, > > I am trying the following configuration that is very conservative: > > actions: > queue: > method: rebase > rebase_fallback: none > name: default > > The auto rebase only attempts a strict rebase. If that attempt at a > strict rebase fails then it will show that there is a conflict that > the developer must take care of. > > I believe any combination of 2 PRs that have overlapping diff stat > should fail a strict rebase. The following link describes the method > and rebase_fallback settings in the queue command. > > https://docs.mergify.io/actions/queue/#id2 > > I would be more concerned if we used a method of merge or a > rebase_fallback of merge. > > Are there examples you can think of where the diff stat overlap and > the strict rebase will succeed? I've read the strict rebase definition and the above link in the mergify documentation, but I'm none the wiser. Consider the following test case (with master @ 7471751a4d81): git checkout -b b1 master git am b1.patch # attached git checkout -b b2 master git am b2.patch # attached git branch b2-rebase b2 git rebase b1 b2-rebase Locally, this produces the following message for me: > First, rewinding head to replay your work on top of it... > Applying: world > Using index info to reconstruct a base tree... > M ReadMe.rst > Falling back to patching base and 3-way merge... > Auto-merging ReadMe.rst The rebase succeeds and produces the expected result, but that result is *exactly* what a human should review. I don't know if mergify catches the above. While the rebase succeeds locally, it should not succeed in mergify. Using the "git rebase -i" (interactive) command, which uses a different rebase backend (based on git-cherry-pick, not on git-am), and specifying a single "pick" command, the rebase still succeeds; this time without producing any diagnostic messages even. So from an auto-rebase perspective, it's even less desirable. Thanks Laszlo > > Another option to consider is to define an additional 'auto-rebase' label that is > off by default to enable the auto rebase feature. By default the PR must be synced > with head when submitted. Only if a maintainer sets the 'auto-rebase' label will > an auto-rebase be attempted. > > I also want to make it easy for non-maintainers to submit PRs and get CI test results. > So auto rebase may be useful for that use case. Perhaps the 'auto-rebase' label > can be considered when the 'push' label is also set. > > Thanks, > > Mike > >> -----Original Message----- >> From: Laszlo Ersek >> Sent: Tuesday, June 22, 2021 8:17 AM >> To: Kinney, Michael D ; devel@edk2.groups.io; spbrogan@outlook.com; ardb@kernel.org >> Cc: Peter Grehan ; Ard Biesheuvel ; Justen, Jordan L >> ; Sean Brogan ; Rebecca Cran >> Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants >> >> On 06/17/21 23:53, Kinney, Michael D wrote: >>> Hi Sean, >>> >>> Mergify had added a queue feature to handle the rebases automatically and make sure >>> CI passes in the order that the PRs are being applied to the base branch. >> >> I'm opposed to *unconditional* auto-rebase. >> >> On one hand, it is indeed unreasonable to require a human to manually >> rebase a "ShellPkg/Application/AcpiViewApp" series just because a series >> for "SecurityPkg/FvReportPei" was merged a bit earlier. In other words, >> merge requests for unrelated modules should not block each other. >> >> On the other hand, auto-rebase is a bad idea if both series modify at >> least one module in common (especially if both series modify at least >> one *file* in common). In case there is a contextual conflict, even if >> the conflict can be auto-resolved, and even if that resolution >> *compiles*, it has to be reviewed by a human first. >> >> I regularly use the git-range-diff command for this. >> >> At Red Hat we've seen obscure bugs due to silent mis-merges (not in edk2 >> -- in different packages); such issues are difficult to debug. >> >> Bisectability helps for sure, but only if the community treats >> bisectability with high priority in the first place. (That is, if every >> contributor builds their patch set at every stage, before submitting it >> for review.) >> >> Can we restrict the auto-rebase feature to such merge requests whose >> cumulative diffstats do not intersect? >> >> Thanks, >> Laszlo --------------F596473B18230618A268543B Content-Type: text/x-patch; name="b1.patch" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="b1.patch" RnJvbSAwNDk3MGNhZjk4ZjhkZWVlY2VjMmQwYmM4ODA4Y2Q4YmQzNGNjNTMwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBMYXN6bG8gRXJzZWsgPGxlcnNla0ByZWRoYXQuY29tPgpEYXRl OiBXZWQsIDIzIEp1biAyMDIxIDE2OjE5OjI1ICswMjAwClN1YmplY3Q6IFtQQVRDSF0gaGVsbG8K Ci0tLQogUmVhZE1lLnJzdCB8IDEgKwogMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspCgpk aWZmIC0tZ2l0IGEvUmVhZE1lLnJzdCBiL1JlYWRNZS5yc3QKaW5kZXggOGY1ZGIxMTI4MWJmLi4z MGY4OTBlZjUzMmUgMTAwNjQ0Ci0tLSBhL1JlYWRNZS5yc3QKKysrIGIvUmVhZE1lLnJzdApAQCAt Myw2ICszLDcgQEAgRURLIElJIFByb2plY3QKID09PT09PT09PT09PT09DQogDQogQSBtb2Rlcm4s IGZlYXR1cmUtcmljaCwgY3Jvc3MtcGxhdGZvcm0gZmlybXdhcmUgZGV2ZWxvcG1lbnQNCisgIEhF TExPDQogZW52aXJvbm1lbnQgZm9yIHRoZSBVRUZJIGFuZCBQSSBzcGVjaWZpY2F0aW9ucyBmcm9t IHd3dy51ZWZpLm9yZy4NCiANCiBDb3JlIENJIEJ1aWxkIFN0YXR1cw0KLS0gCjIuMTkuMS4zLmcz MDI0N2FhNWQyMDEKCg== --------------F596473B18230618A268543B Content-Type: text/x-patch; name="b2.patch" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="b2.patch" RnJvbSA5MjVlOTNhZjc2YWYyNzUyNjAxMWFkMzM4NzVmYTI2Y2M0Y2RjNmMwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBMYXN6bG8gRXJzZWsgPGxlcnNla0ByZWRoYXQuY29tPgpEYXRl OiBXZWQsIDIzIEp1biAyMDIxIDE2OjE5OjUxICswMjAwClN1YmplY3Q6IFtQQVRDSF0gd29ybGQK Ci0tLQogUmVhZE1lLnJzdCB8IDEgKwogMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspCgpk aWZmIC0tZ2l0IGEvUmVhZE1lLnJzdCBiL1JlYWRNZS5yc3QKaW5kZXggOGY1ZGIxMTI4MWJmLi5j ZmMzNjRkYjc0MDYgMTAwNjQ0Ci0tLSBhL1JlYWRNZS5yc3QKKysrIGIvUmVhZE1lLnJzdApAQCAt NCw2ICs0LDcgQEAgRURLIElJIFByb2plY3QKIA0KIEEgbW9kZXJuLCBmZWF0dXJlLXJpY2gsIGNy b3NzLXBsYXRmb3JtIGZpcm13YXJlIGRldmVsb3BtZW50DQogZW52aXJvbm1lbnQgZm9yIHRoZSBV RUZJIGFuZCBQSSBzcGVjaWZpY2F0aW9ucyBmcm9tIHd3dy51ZWZpLm9yZy4NCisgIFdPUkxEDQog DQogQ29yZSBDSSBCdWlsZCBTdGF0dXMNCiAtLS0tLS0tLS0tLS0tLS0tLS0tLQ0KLS0gCjIuMTku MS4zLmczMDI0N2FhNWQyMDEKCg== --------------F596473B18230618A268543B--