* 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
* 回复: 回复: [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
* 回复: 回复: 回复: [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
[parent not found: <164ED1ABFE90B1B0.9298@groups.io>]
* 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
* 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 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