public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* please let's disable the ECC plugin in CI, in its current form
@ 2020-12-08  2:04 Laszlo Ersek
  2020-12-08  7:03 ` Ard Biesheuvel
  2020-12-08 15:38 ` 回复: [edk2-devel] " gaoliming
  0 siblings, 2 replies; 11+ messages in thread
From: Laszlo Ersek @ 2020-12-08  2:04 UTC (permalink / raw)
  To: Michael Kinney, Sean Brogan, Ard Biesheuvel (ARM address),
	Leif Lindholm (Nuvia address), Andrew Fish
  Cc: edk2-devel-groups-io

Hi All,

in my opinion, the ECC plugin in CI has not been productive or helpful.
The errors it reports are not convincing, and exceptions are difficult
to add, or even express. I request that we disable ECC globally for
edk2, or at least make it controllable through a github PR flag (not
through CI metafile changes in edk2).

(Metafile changes could be an acceptable way of controlling ECC if (a)
they didn't have to go through review, (b) if technically such changes
would take effect in CI even if they were included in the patch series
subject to the CI run. Then a maintainer could evaluate and
*immediately* suppress such ECC issues by adding the exceptions as a
prepended patch, and force-pushing the updated branch to the same open
PR. But this would remain much inferior to simply disabling ECC, or
controlling it through a PR label.)

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: please let's disable the ECC plugin in CI, in its current form
  2020-12-08  2:04 please let's disable the ECC plugin in CI, in its current form Laszlo Ersek
@ 2020-12-08  7:03 ` Ard Biesheuvel
  2020-12-08 15:38 ` 回复: [edk2-devel] " gaoliming
  1 sibling, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-12-08  7:03 UTC (permalink / raw)
  To: Laszlo Ersek, Michael Kinney, Sean Brogan,
	Leif Lindholm (Nuvia address), Andrew Fish
  Cc: edk2-devel-groups-io

On 12/8/20 3:04 AM, Laszlo Ersek wrote:
> Hi All,
> 
> in my opinion, the ECC plugin in CI has not been productive or helpful.
> The errors it reports are not convincing, and exceptions are difficult
> to add, or even express. I request that we disable ECC globally for
> edk2, or at least make it controllable through a github PR flag (not
> through CI metafile changes in edk2).
> 
> (Metafile changes could be an acceptable way of controlling ECC if (a)
> they didn't have to go through review, (b) if technically such changes
> would take effect in CI even if they were included in the patch series
> subject to the CI run. Then a maintainer could evaluate and
> *immediately* suppress such ECC issues by adding the exceptions as a
> prepended patch, and force-pushing the updated branch to the same open
> PR. But this would remain much inferior to simply disabling ECC, or
> controlling it through a PR label.)
> 

+1

-- 
Ard.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-08  2:04 please let's disable the ECC plugin in CI, in its current form Laszlo Ersek
  2020-12-08  7:03 ` Ard Biesheuvel
@ 2020-12-08 15:38 ` gaoliming
  2020-12-08 18:37   ` Sean
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: gaoliming @ 2020-12-08 15:38 UTC (permalink / raw)
  To: devel, lersek, 'Michael Kinney', 'Sean Brogan',
	'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Laszlo:
  ECC tool is not perfect. It is still helpful. I am OK to disable ECC plugin in CI. But, I request to expose the option to enable it per package. If the package maintainer thinks ECC is valuable, he can enable ECC plugin in one package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg patch. So, I would keep ECC plugin in MdePkg.

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+68414+4905953+8761045@groups.io
> <bounce+27952+68414+4905953+8761045@groups.io> 代表 Laszlo Ersek
> 发送时间: 2020年12月8日 10:05
> 收件人: Michael Kinney <michael.d.kinney@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Ard Biesheuvel (ARM address)
> <ard.biesheuvel@arm.com>; Leif Lindholm (Nuvia address)
> <leif@nuviainc.com>; Andrew Fish <afish@apple.com>
> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>
> 主题: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
> 
> Hi All,
> 
> in my opinion, the ECC plugin in CI has not been productive or helpful.
> The errors it reports are not convincing, and exceptions are difficult
> to add, or even express. I request that we disable ECC globally for
> edk2, or at least make it controllable through a github PR flag (not
> through CI metafile changes in edk2).
> 
> (Metafile changes could be an acceptable way of controlling ECC if (a)
> they didn't have to go through review, (b) if technically such changes
> would take effect in CI even if they were included in the patch series
> subject to the CI run. Then a maintainer could evaluate and
> *immediately* suppress such ECC issues by adding the exceptions as a
> prepended patch, and force-pushing the updated branch to the same open
> PR. But this would remain much inferior to simply disabling ECC, or
> controlling it through a PR label.)
> 
> Thanks
> Laszlo
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-08 15:38 ` 回复: [edk2-devel] " gaoliming
@ 2020-12-08 18:37   ` Sean
  2020-12-10  1:34     ` 回复: " gaoliming
       [not found]   ` <164ED1ABFE90B1B0.9298@groups.io>
  2020-12-10  8:52   ` Laszlo Ersek
  2 siblings, 1 reply; 11+ messages in thread
From: Sean @ 2020-12-08 18:37 UTC (permalink / raw)
  To: devel, gaoliming, lersek, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Liming

I agree it would be great to get to a place where a package maintainer 
could turn this on but because of this bug 
https://bugzilla.tianocore.org/show_bug.cgi?id=2986 I think this needs 
to be completely disabled until the above is resolved.

Thanks
Sean


On 12/8/2020 7:38 AM, gaoliming wrote:
> Laszlo:
>    ECC tool is not perfect. It is still helpful. I am OK to disable ECC plugin in CI. But, I request to expose the option to enable it per package. If the package maintainer thinks ECC is valuable, he can enable ECC plugin in one package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg patch. So, I would keep ECC plugin in MdePkg.
> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: bounce+27952+68414+4905953+8761045@groups.io
>> <bounce+27952+68414+4905953+8761045@groups.io> 代表 Laszlo Ersek
>> 发送时间: 2020年12月8日 10:05
>> 收件人: Michael Kinney <michael.d.kinney@intel.com>; Sean Brogan
>> <sean.brogan@microsoft.com>; Ard Biesheuvel (ARM address)
>> <ard.biesheuvel@arm.com>; Leif Lindholm (Nuvia address)
>> <leif@nuviainc.com>; Andrew Fish <afish@apple.com>
>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>
>> 主题: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
>>
>> Hi All,
>>
>> in my opinion, the ECC plugin in CI has not been productive or helpful.
>> The errors it reports are not convincing, and exceptions are difficult
>> to add, or even express. I request that we disable ECC globally for
>> edk2, or at least make it controllable through a github PR flag (not
>> through CI metafile changes in edk2).
>>
>> (Metafile changes could be an acceptable way of controlling ECC if (a)
>> they didn't have to go through review, (b) if technically such changes
>> would take effect in CI even if they were included in the patch series
>> subject to the CI run. Then a maintainer could evaluate and
>> *immediately* suppress such ECC issues by adding the exceptions as a
>> prepended patch, and force-pushing the updated branch to the same open
>> PR. But this would remain much inferior to simply disabling ECC, or
>> controlling it through a PR label.)
>>
>> Thanks
>> Laszlo
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
       [not found]   ` <164ED1ABFE90B1B0.9298@groups.io>
@ 2020-12-09 10:24     ` Sean
  2020-12-10  8:12       ` Laszlo Ersek
  0 siblings, 1 reply; 11+ messages in thread
From: Sean @ 2020-12-09 10:24 UTC (permalink / raw)
  To: devel, gaoliming, lersek, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Laszlo,

Two options provided here for disabling EccCheck.

See commits in PR
https://github.com/tianocore/edk2/pull/1201

Thanks
Sean


On 12/8/2020 10:37 AM, Sean wrote:
> Liming
> 
> I agree it would be great to get to a place where a package maintainer 
> could turn this on but because of this bug 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2986 I think this needs 
> to be completely disabled until the above is resolved.
> 
> Thanks
> Sean
> 
> 
> On 12/8/2020 7:38 AM, gaoliming wrote:
>> Laszlo:
>>    ECC tool is not perfect. It is still helpful. I am OK to disable 
>> ECC plugin in CI. But, I request to expose the option to enable it per 
>> package. If the package maintainer thinks ECC is valuable, he can 
>> enable ECC plugin in one package. As MdePkg maintainer, I can help to 
>> resolve ECC issue in MdePkg patch. So, I would keep ECC plugin in MdePkg.
>>
>> Thanks
>> Liming
>>> -----邮件原件-----
>>> 发件人: bounce+27952+68414+4905953+8761045@groups.io
>>> <bounce+27952+68414+4905953+8761045@groups.io> 代表 Laszlo Ersek
>>> 发送时间: 2020年12月8日 10:05
>>> 收件人: Michael Kinney <michael.d.kinney@intel.com>; Sean Brogan
>>> <sean.brogan@microsoft.com>; Ard Biesheuvel (ARM address)
>>> <ard.biesheuvel@arm.com>; Leif Lindholm (Nuvia address)
>>> <leif@nuviainc.com>; Andrew Fish <afish@apple.com>
>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>
>>> 主题: [edk2-devel] please let's disable the ECC plugin in CI, in its 
>>> current form
>>>
>>> Hi All,
>>>
>>> in my opinion, the ECC plugin in CI has not been productive or helpful.
>>> The errors it reports are not convincing, and exceptions are difficult
>>> to add, or even express. I request that we disable ECC globally for
>>> edk2, or at least make it controllable through a github PR flag (not
>>> through CI metafile changes in edk2).
>>>
>>> (Metafile changes could be an acceptable way of controlling ECC if (a)
>>> they didn't have to go through review, (b) if technically such changes
>>> would take effect in CI even if they were included in the patch series
>>> subject to the CI run. Then a maintainer could evaluate and
>>> *immediately* suppress such ECC issues by adding the exceptions as a
>>> prepended patch, and force-pushing the updated branch to the same open
>>> PR. But this would remain much inferior to simply disabling ECC, or
>>> controlling it through a PR label.)
>>>
>>> Thanks
>>> Laszlo
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* 回复: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-08 18:37   ` Sean
@ 2020-12-10  1:34     ` gaoliming
  2020-12-10  3:39       ` Sean
  0 siblings, 1 reply; 11+ messages in thread
From: gaoliming @ 2020-12-10  1:34 UTC (permalink / raw)
  To: 'Sean Brogan', devel, lersek, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Sean:
 I will give the proposal to fix this issue https://bugzilla.tianocore.org/show_bug.cgi?id=2986. In fact, we can enhance ECC to skip the folder or files. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Sean Brogan <spbrogan@outlook.com>
> 发送时间: 2020年12月9日 2:37
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
> lersek@redhat.com; 'Michael Kinney' <michael.d.kinney@intel.com>; 'Sean
> Brogan' <sean.brogan@microsoft.com>; 'Ard Biesheuvel (ARM address)'
> <ard.biesheuvel@arm.com>; 'Leif Lindholm (Nuvia address)'
> <leif@nuviainc.com>; 'Andrew Fish' <afish@apple.com>
> 主题: Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its
> current form
> 
> Liming
> 
> I agree it would be great to get to a place where a package maintainer
> could turn this on but because of this bug
> https://bugzilla.tianocore.org/show_bug.cgi?id=2986 I think this needs
> to be completely disabled until the above is resolved.
> 
> Thanks
> Sean
> 
> 
> On 12/8/2020 7:38 AM, gaoliming wrote:
> > Laszlo:
> >    ECC tool is not perfect. It is still helpful. I am OK to disable ECC plugin in
> CI. But, I request to expose the option to enable it per package. If the
> package maintainer thinks ECC is valuable, he can enable ECC plugin in one
> package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg
> patch. So, I would keep ECC plugin in MdePkg.
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: bounce+27952+68414+4905953+8761045@groups.io
> >> <bounce+27952+68414+4905953+8761045@groups.io> 代表 Laszlo
> Ersek
> >> 发送时间: 2020年12月8日 10:05
> >> 收件人: Michael Kinney <michael.d.kinney@intel.com>; Sean Brogan
> >> <sean.brogan@microsoft.com>; Ard Biesheuvel (ARM address)
> >> <ard.biesheuvel@arm.com>; Leif Lindholm (Nuvia address)
> >> <leif@nuviainc.com>; Andrew Fish <afish@apple.com>
> >> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>
> >> 主题: [edk2-devel] please let's disable the ECC plugin in CI, in its current
> form
> >>
> >> Hi All,
> >>
> >> in my opinion, the ECC plugin in CI has not been productive or helpful.
> >> The errors it reports are not convincing, and exceptions are difficult
> >> to add, or even express. I request that we disable ECC globally for
> >> edk2, or at least make it controllable through a github PR flag (not
> >> through CI metafile changes in edk2).
> >>
> >> (Metafile changes could be an acceptable way of controlling ECC if (a)
> >> they didn't have to go through review, (b) if technically such changes
> >> would take effect in CI even if they were included in the patch series
> >> subject to the CI run. Then a maintainer could evaluate and
> >> *immediately* suppress such ECC issues by adding the exceptions as a
> >> prepended patch, and force-pushing the updated branch to the same open
> >> PR. But this would remain much inferior to simply disabling ECC, or
> >> controlling it through a PR label.)
> >>
> >> Thanks
> >> Laszlo
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> > 
> >
> >



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: 回复: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-10  1:34     ` 回复: " gaoliming
@ 2020-12-10  3:39       ` Sean
  2020-12-11  1:10         ` 回复: " gaoliming
  0 siblings, 1 reply; 11+ messages in thread
From: Sean @ 2020-12-10  3:39 UTC (permalink / raw)
  To: devel, gaoliming, lersek, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

I have been thinking about the idea of a CI plugin type that is provided 
a file list and only operates on the supplied file list.

Then during CI execution if a file list was provided these plugins would 
run and evaluate on that file list.  In azure pipeline we already get 
the diff between target branch and PR.  We could then write that to file 
and pass that file into the stuart_ci_build process.

Anyway if you have other ideas i'll leave it to you.  Please let me know 
if you are interested, want to discuss more, or go in this direction.

Thanks
Sean


On 12/9/2020 5:34 PM, gaoliming wrote:
> Sean:
>   I will give the proposal to fix this issue https://bugzilla.tianocore.org/show_bug.cgi?id=2986. In fact, we can enhance ECC to skip the folder or files.
> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Sean Brogan <spbrogan@outlook.com>
>> 发送时间: 2020年12月9日 2:37
>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
>> lersek@redhat.com; 'Michael Kinney' <michael.d.kinney@intel.com>; 'Sean
>> Brogan' <sean.brogan@microsoft.com>; 'Ard Biesheuvel (ARM address)'
>> <ard.biesheuvel@arm.com>; 'Leif Lindholm (Nuvia address)'
>> <leif@nuviainc.com>; 'Andrew Fish' <afish@apple.com>
>> 主题: Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its
>> current form
>>
>> Liming
>>
>> I agree it would be great to get to a place where a package maintainer
>> could turn this on but because of this bug
>> https://bugzilla.tianocore.org/show_bug.cgi?id=2986 I think this needs
>> to be completely disabled until the above is resolved.
>>
>> Thanks
>> Sean
>>
>>
>> On 12/8/2020 7:38 AM, gaoliming wrote:
>>> Laszlo:
>>>     ECC tool is not perfect. It is still helpful. I am OK to disable ECC plugin in
>> CI. But, I request to expose the option to enable it per package. If the
>> package maintainer thinks ECC is valuable, he can enable ECC plugin in one
>> package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg
>> patch. So, I would keep ECC plugin in MdePkg.
>>>
>>> Thanks
>>> Liming
>>>> -----邮件原件-----
>>>> 发件人: bounce+27952+68414+4905953+8761045@groups.io
>>>> <bounce+27952+68414+4905953+8761045@groups.io> 代表 Laszlo
>> Ersek
>>>> 发送时间: 2020年12月8日 10:05
>>>> 收件人: Michael Kinney <michael.d.kinney@intel.com>; Sean Brogan
>>>> <sean.brogan@microsoft.com>; Ard Biesheuvel (ARM address)
>>>> <ard.biesheuvel@arm.com>; Leif Lindholm (Nuvia address)
>>>> <leif@nuviainc.com>; Andrew Fish <afish@apple.com>
>>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>
>>>> 主题: [edk2-devel] please let's disable the ECC plugin in CI, in its current
>> form
>>>>
>>>> Hi All,
>>>>
>>>> in my opinion, the ECC plugin in CI has not been productive or helpful.
>>>> The errors it reports are not convincing, and exceptions are difficult
>>>> to add, or even express. I request that we disable ECC globally for
>>>> edk2, or at least make it controllable through a github PR flag (not
>>>> through CI metafile changes in edk2).
>>>>
>>>> (Metafile changes could be an acceptable way of controlling ECC if (a)
>>>> they didn't have to go through review, (b) if technically such changes
>>>> would take effect in CI even if they were included in the patch series
>>>> subject to the CI run. Then a maintainer could evaluate and
>>>> *immediately* suppress such ECC issues by adding the exceptions as a
>>>> prepended patch, and force-pushing the updated branch to the same open
>>>> PR. But this would remain much inferior to simply disabling ECC, or
>>>> controlling it through a PR label.)
>>>>
>>>> Thanks
>>>> Laszlo
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-09 10:24     ` Sean
@ 2020-12-10  8:12       ` Laszlo Ersek
  0 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2020-12-10  8:12 UTC (permalink / raw)
  To: devel, spbrogan, gaoliming, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

On 12/09/20 11:24, Sean Brogan wrote:
> Laszlo,
> 
> Two options provided here for disabling EccCheck.
> 
> See commits in PR
> https://github.com/tianocore/edk2/pull/1201

The second patch looks very attractive to me; I didn't know about the
existence of the "skip" key. I thought I'd have to list every top-level
directory in the package as an exception. Do I understand right that the
toolset already understand the "skip" key?

In retrospect, the following article:

https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#running-ci

does say

"""
Finally setting the check to the value skip will skip that plugin. Examples:
CompilerPlugin=skip skip the build test
"""

I guess I didn't really understand the intended command line syntax; let
alone the syntax for the yaml file.

I'll reach back to this topic from the other thread.

Thank you!
Laszlo


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-08 15:38 ` 回复: [edk2-devel] " gaoliming
  2020-12-08 18:37   ` Sean
       [not found]   ` <164ED1ABFE90B1B0.9298@groups.io>
@ 2020-12-10  8:52   ` Laszlo Ersek
  2020-12-11  1:13     ` 回复: " gaoliming
  2 siblings, 1 reply; 11+ messages in thread
From: Laszlo Ersek @ 2020-12-10  8:52 UTC (permalink / raw)
  To: gaoliming, devel, 'Michael Kinney', 'Sean Brogan',
	'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Hi Liming,

On 12/08/20 16:38, gaoliming wrote:
> Laszlo:
>   ECC tool is not perfect. It is still helpful. I am OK to disable ECC plugin in CI. But, I request to expose the option to enable it per package. If the package maintainer thinks ECC is valuable, he can enable ECC plugin in one package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg patch. So, I would keep ECC plugin in MdePkg.

the "skip trick" in Sean's patch at
<https://github.com/tianocore/edk2/pull/1201/commits/3a6b58f06cc33e78ba66b297013dd3d5bfb5f962>:

    "EccCheck": {
        ## Exception sample looks like below:
        ## "ExceptionList": [
        ##     "<ErrorID>", "<KeyWord>"
        ## ]
        "ExceptionList": [
        ],
        ## Both file path and directory path are accepted.
        "IgnoreFiles": [
        ],
        "skip": True <------ this
    },

looks nice. While it is not as dynamic as a PR label, it is
package-level, and it needs no change when we add a new directory under
the package directory (because, the exception list would need a new
entry then).

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 11+ messages in thread

* 回复: 回复: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-10  3:39       ` Sean
@ 2020-12-11  1:10         ` gaoliming
  0 siblings, 0 replies; 11+ messages in thread
From: gaoliming @ 2020-12-11  1:10 UTC (permalink / raw)
  To: devel, spbrogan, lersek, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Sean:
  Thanks for your information. I will evaluate current implementation and give the detail proposal. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+68630+4905953+8761045@groups.io
> <bounce+27952+68630+4905953+8761045@groups.io> 代表 Sean
> 发送时间: 2020年12月10日 11:39
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
> lersek@redhat.com; 'Michael Kinney' <michael.d.kinney@intel.com>; 'Sean
> Brogan' <sean.brogan@microsoft.com>; 'Ard Biesheuvel (ARM address)'
> <ard.biesheuvel@arm.com>; 'Leif Lindholm (Nuvia address)'
> <leif@nuviainc.com>; 'Andrew Fish' <afish@apple.com>
> 主题: Re: 回复: 回复: [edk2-devel] please let's disable the ECC plugin in CI,
> in its current form
> 
> I have been thinking about the idea of a CI plugin type that is provided
> a file list and only operates on the supplied file list.
> 
> Then during CI execution if a file list was provided these plugins would
> run and evaluate on that file list.  In azure pipeline we already get
> the diff between target branch and PR.  We could then write that to file
> and pass that file into the stuart_ci_build process.
> 
> Anyway if you have other ideas i'll leave it to you.  Please let me know
> if you are interested, want to discuss more, or go in this direction.
> 
> Thanks
> Sean
> 
> 
> On 12/9/2020 5:34 PM, gaoliming wrote:
> > Sean:
> >   I will give the proposal to fix this issue
> https://bugzilla.tianocore.org/show_bug.cgi?id=2986. In fact, we can
> enhance ECC to skip the folder or files.
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: Sean Brogan <spbrogan@outlook.com>
> >> 发送时间: 2020年12月9日 2:37
> >> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
> >> lersek@redhat.com; 'Michael Kinney' <michael.d.kinney@intel.com>;
> 'Sean
> >> Brogan' <sean.brogan@microsoft.com>; 'Ard Biesheuvel (ARM address)'
> >> <ard.biesheuvel@arm.com>; 'Leif Lindholm (Nuvia address)'
> >> <leif@nuviainc.com>; 'Andrew Fish' <afish@apple.com>
> >> 主题: Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in
> its
> >> current form
> >>
> >> Liming
> >>
> >> I agree it would be great to get to a place where a package maintainer
> >> could turn this on but because of this bug
> >> https://bugzilla.tianocore.org/show_bug.cgi?id=2986 I think this needs
> >> to be completely disabled until the above is resolved.
> >>
> >> Thanks
> >> Sean
> >>
> >>
> >> On 12/8/2020 7:38 AM, gaoliming wrote:
> >>> Laszlo:
> >>>     ECC tool is not perfect. It is still helpful. I am OK to disable ECC
> plugin in
> >> CI. But, I request to expose the option to enable it per package. If the
> >> package maintainer thinks ECC is valuable, he can enable ECC plugin in one
> >> package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg
> >> patch. So, I would keep ECC plugin in MdePkg.
> >>>
> >>> Thanks
> >>> Liming
> >>>> -----邮件原件-----
> >>>> 发件人: bounce+27952+68414+4905953+8761045@groups.io
> >>>> <bounce+27952+68414+4905953+8761045@groups.io> 代表 Laszlo
> >> Ersek
> >>>> 发送时间: 2020年12月8日 10:05
> >>>> 收件人: Michael Kinney <michael.d.kinney@intel.com>; Sean Brogan
> >>>> <sean.brogan@microsoft.com>; Ard Biesheuvel (ARM address)
> >>>> <ard.biesheuvel@arm.com>; Leif Lindholm (Nuvia address)
> >>>> <leif@nuviainc.com>; Andrew Fish <afish@apple.com>
> >>>> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>
> >>>> 主题: [edk2-devel] please let's disable the ECC plugin in CI, in its
> current
> >> form
> >>>>
> >>>> Hi All,
> >>>>
> >>>> in my opinion, the ECC plugin in CI has not been productive or helpful.
> >>>> The errors it reports are not convincing, and exceptions are difficult
> >>>> to add, or even express. I request that we disable ECC globally for
> >>>> edk2, or at least make it controllable through a github PR flag (not
> >>>> through CI metafile changes in edk2).
> >>>>
> >>>> (Metafile changes could be an acceptable way of controlling ECC if (a)
> >>>> they didn't have to go through review, (b) if technically such changes
> >>>> would take effect in CI even if they were included in the patch series
> >>>> subject to the CI run. Then a maintainer could evaluate and
> >>>> *immediately* suppress such ECC issues by adding the exceptions as a
> >>>> prepended patch, and force-pushing the updated branch to the same
> open
> >>>> PR. But this would remain much inferior to simply disabling ECC, or
> >>>> controlling it through a PR label.)
> >>>>
> >>>> Thanks
> >>>> Laszlo
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 11+ messages in thread

* 回复: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its current form
  2020-12-10  8:52   ` Laszlo Ersek
@ 2020-12-11  1:13     ` gaoliming
  0 siblings, 0 replies; 11+ messages in thread
From: gaoliming @ 2020-12-11  1:13 UTC (permalink / raw)
  To: 'Laszlo Ersek', devel, 'Michael Kinney',
	'Sean Brogan', 'Ard Biesheuvel (ARM address)',
	'Leif Lindholm (Nuvia address)', 'Andrew Fish'

Laszlo:
  Yes. This is a clean way to skip ECC for package level. I will leave the choice to the package maintainer to disable or enable ECC checker. Meanwhile, I will continue to investigate how to improve ECC plugin. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Laszlo Ersek <lersek@redhat.com>
> 发送时间: 2020年12月10日 16:52
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> 'Michael Kinney' <michael.d.kinney@intel.com>; 'Sean Brogan'
> <sean.brogan@microsoft.com>; 'Ard Biesheuvel (ARM address)'
> <ard.biesheuvel@arm.com>; 'Leif Lindholm (Nuvia address)'
> <leif@nuviainc.com>; 'Andrew Fish' <afish@apple.com>
> 主题: Re: 回复: [edk2-devel] please let's disable the ECC plugin in CI, in its
> current form
> 
> Hi Liming,
> 
> On 12/08/20 16:38, gaoliming wrote:
> > Laszlo:
> >   ECC tool is not perfect. It is still helpful. I am OK to disable ECC plugin in
> CI. But, I request to expose the option to enable it per package. If the
> package maintainer thinks ECC is valuable, he can enable ECC plugin in one
> package. As MdePkg maintainer, I can help to resolve ECC issue in MdePkg
> patch. So, I would keep ECC plugin in MdePkg.
> 
> the "skip trick" in Sean's patch at
> <https://github.com/tianocore/edk2/pull/1201/commits/3a6b58f06cc33e78b
> a66b297013dd3d5bfb5f962>:
> 
>     "EccCheck": {
>         ## Exception sample looks like below:
>         ## "ExceptionList": [
>         ##     "<ErrorID>", "<KeyWord>"
>         ## ]
>         "ExceptionList": [
>         ],
>         ## Both file path and directory path are accepted.
>         "IgnoreFiles": [
>         ],
>         "skip": True <------ this
>     },
> 
> looks nice. While it is not as dynamic as a PR label, it is
> package-level, and it needs no change when we add a new directory under
> the package directory (because, the exception list would need a new
> entry then).
> 
> Thanks
> Laszlo




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-12-11  1:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-08  2:04 please let's disable the ECC plugin in CI, in its current form Laszlo Ersek
2020-12-08  7:03 ` Ard Biesheuvel
2020-12-08 15:38 ` 回复: [edk2-devel] " gaoliming
2020-12-08 18:37   ` Sean
2020-12-10  1:34     ` 回复: " gaoliming
2020-12-10  3:39       ` Sean
2020-12-11  1:10         ` 回复: " gaoliming
     [not found]   ` <164ED1ABFE90B1B0.9298@groups.io>
2020-12-09 10:24     ` Sean
2020-12-10  8:12       ` Laszlo Ersek
2020-12-10  8:52   ` Laszlo Ersek
2020-12-11  1:13     ` 回复: " gaoliming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox