From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.15565.1607095341354634045 for ; Fri, 04 Dec 2020 07:22:21 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=KoZzdmyi; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607095340; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=u7uV5sRisNUL616ebTRIpo1VDn0SgewO6GY7QZ6+rnI=; b=KoZzdmyikggXUn/5t7NV91nDEWMOsoRY/EN0cZJyRh5TwsKwf+1SoLs6G5tnSFzlJc1bUt e/BDY5NdEkz+DgSu7Pdvm31uWFwGBuUsnnrh/ZQ6Vhg/I8ra0Gph4CQD4ZETPb8MknZYXe 6Ikc+Vybs6/woXWlrdyVOmKaisHwHSU= 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-293-21QpxAX9NkyUuHsBhsrpNw-1; Fri, 04 Dec 2020 10:22:14 -0500 X-MC-Unique: 21QpxAX9NkyUuHsBhsrpNw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9ADAAD1664; Fri, 4 Dec 2020 15:22:12 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-12.ams2.redhat.com [10.36.113.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 918E55D9CA; Fri, 4 Dec 2020 15:22:10 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 1/2] OvmfPkg: start using the ECC plugin exception list To: Sean Brogan , devel@edk2.groups.io Cc: Ard Biesheuvel , James Bottomley , Jordan Justen , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Tom Lendacky , "Leif Lindholm (Nuvia address)" References: <20201204032116.31321-1-lersek@redhat.com> <20201204032116.31321-2-lersek@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 4 Dec 2020 16:22:09 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 12/04/20 05:05, Sean Brogan wrote: > 3 things > > 1. I would support disabling any ci plugins that can be destructive to > your local environment.  I think EccCheck needs to be re-evaluated > regarding how it gets the change set.  What it does now is not a pattern > I have encouraged and although i understand the desire (for server ci) i > think it has caused more trouble than it is worth. Thanks. To be honest, I called ECC "fascist", when I went over the report it generated. It's insane. It makes me want to quit programming. And I'm the reviewer guy that forces contributors to conform to the coding style in minute detail. ECC is unbearable. > > 2. Not sure if license check has same problem or not. My understanding is that, for a while now, it has been technically impossible to contribute code under any other license than BSD-2-Clause-Patent. I think Leif had some serious thoughts that the limitation wasn't only technical but even legal. I forget the details. > > > > On 12/3/2020 7:21 PM, Laszlo Ersek wrote: >> In the recent past, ECC has wreaked havoc at least twice: >> >> - rejected Tom Lendacky's perfectly valid and clean code: >> >>    https://bugzilla.tianocore.org/show_bug.cgi?id=3008#c5 >>    https://edk2.groups.io/g/devel/message/67097 >> >> - rejected James Bottomley's series for bogus reasons: >> >>    https://bugzilla.tianocore.org/show_bug.cgi?id=3077#c4 >>    https://edk2.groups.io/g/devel/message/68302 >> >> There isn't capacity to improve ECC: >> >> - Liming filed an ECC bug about the first case noted above, but it has >>    received no feedback in 25 days (as of this writing): >> >>    https://bugzilla.tianocore.org/show_bug.cgi?id=3060 >> >> And running CI or ECC on developer machines is difficult: >> >> - I had to set up a separate VM for it, >> >> - Shenglei gave Windows-based usage instructions only, >> >>    https://edk2.groups.io/g/devel/message/61966 >> >> - experimenting with a CI outside of a VM is somewhat risky: >> >>    https://github.com/tianocore/edk2-pytool-extensions/issues/231 > > 3. Running CI locally should not be "somewhat risky".  More work needs > to be done to identify the root cause of the above behavior but my guess > is that it has to do with EccCheck and nothing to do with > pytool-extensions. Sorry, I guess I mixed up my references a little bit. I consider running binaries downloaded from the internet risky (except from the official repos of my Linux distro(s)). But that's indeed a different topic and I shouldn't have generalized. Sorry about that. Thanks for commenting! Laszlo > > >> >> ECC should be considered an experimental tool; I agreed to its enablement >> in CI specifically because we were offered an exception list: >> >>    https://edk2.groups.io/g/devel/message/60961 >>    >> https://www.redhat.com/archives/edk2-devel-archive/2020-June/msg00473.html >> >> >> The github.com-based pull request process is already very inefficient for >> both contributors and maintainers; it's time to put the ECC exception >> list >> to use. >> >> Now, I've tried adding those error codes that were reported against >> James's series (after evaluating each entry in the report at >> ), >> >> namely 4001, 4002, 5007, 8001, 8004, 8005. However, to my horror, >> "EccCheck.ExceptionList" only supports the following format: >> >>    "", "", "", "", ... >> >> where each pair binds a particular error ID to a particular "offending" >> identifier, such as C variable name. There is no wildcard support, so >> it's >> impossible to disable entire classes of ECC reports. >> >> Therefore, I have to use "EccCheck.IgnoreFiles". It also doesn't support >> the "." subdirectory or the "*" wildcard. But, with some sweat, I can >> still use it to disable ECC for all of OvmfPkg. >> >> Cc: Ard Biesheuvel >> Cc: James Bottomley >> Cc: Jordan Justen >> Cc: Philippe Mathieu-Daudé >> Cc: Tom Lendacky >> Signed-off-by: Laszlo Ersek >> --- >>   OvmfPkg/OvmfPkg.ci.yaml | 49 ++++++++++++++++++++ >>   1 file changed, 49 insertions(+) >> >> diff --git a/OvmfPkg/OvmfPkg.ci.yaml b/OvmfPkg/OvmfPkg.ci.yaml >> index 3128aefe9ed1..68d2de704d19 100644 >> --- a/OvmfPkg/OvmfPkg.ci.yaml >> +++ b/OvmfPkg/OvmfPkg.ci.yaml >> @@ -22,6 +22,55 @@ >>           ], >>           ## Both file path and directory path are accepted. >>           "IgnoreFiles": [ >> +            "8254TimerDxe", >> +            "8259InterruptControllerDxe", >> +            "AcpiPlatformDxe", >> +            "AcpiTables", >> +            "AmdSev", >> +            "AmdSevDxe", >> +            "Bhyve", >> +            "CompatImageLoaderDxe", >> +            "CpuHotplugSmm", >> +            "CpuS3DataDxe", >> +            "Csm", >> +            "EmuVariableFvbRuntimeDxe", >> +            "EnrollDefaultKeys", >> +            "Include", >> +            "IncompatiblePciDeviceSupportDxe", >> +            "IoMmuDxe", >> +            "Library", >> +            "LinuxInitrdDynamicShellCommand", >> +            "LsiScsiDxe", >> +            "MptScsiDxe", >> +            "OvmfXenElfHeaderGenerator.c", >> +            "PciHotPlugInitDxe", >> +            "PlatformDxe", >> +            "PlatformPei", >> +            "PvScsiDxe", >> +            "QemuFlashFvbServicesRuntimeDxe", >> +            "QemuKernelLoaderFsDxe", >> +            "QemuRamfbDxe", >> +            "QemuVideoDxe", >> +            "SataControllerDxe", >> +            "Sec", >> +            "SioBusDxe", >> +            "SmbiosPlatformDxe", >> +            "SmmAccess", >> +            "SmmControl2Dxe", >> +            "Tcg", >> +            "Virtio10Dxe", >> +            "VirtioBlkDxe", >> +            "VirtioGpuDxe", >> +            "VirtioNetDxe", >> +            "VirtioPciDeviceDxe", >> +            "VirtioRngDxe", >> +            "VirtioScsiDxe", >> +            "XenBusDxe", >> +            "XenIoPciDxe", >> +            "XenIoPvhDxe", >> +            "XenPlatformPei", >> +            "XenPvBlkDxe", >> +            "XenTimerDxe" >>           ] >>       }, >>       ## options defined .pytool/Plugin/CompilerPlugin >> > > Thanks > Sean >