Re: [edk2-devel] [PATCH 1/2] OvmfPkg: start using the ECC plugin exception list

["Sean" <spbrogan@outlook.com>]

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.

2. Not sure if license check has same problem or not.



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.


> 
> 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
> <https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-7def1f19d478/_apis/build/builds/16071/logs/122>),
> namely 4001, 4002, 5007, 8001, 8004, 8005. However, to my horror,
> "EccCheck.ExceptionList" only supports the following format:
> 
>    "<ErrorID>", "<KeyWord>", "<ErrorID>", "<KeyWord>", ...
> 
> 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 <ard.biesheuvel@arm.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   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