* [PATCH v1 0/1] Spell Check File Mask Leak into Sequential Package Checks @ 2021-06-10 1:47 Kun Qin 2021-06-10 1:47 ` [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices Kun Qin 0 siblings, 1 reply; 5+ messages in thread From: Kun Qin @ 2021-06-10 1:47 UTC (permalink / raw) To: devel; +Cc: Sean Brogan, Bret Barkelew, Michael D Kinney, Liming Gao REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3443 Current spell check routine on CI build pipelines is ignoring some files incorrectly. The issue is class variable, STANDARD_PLUGIN_DEFINED_PATHS from SpellCheck.py, is a list. In a local function the list is assigned to a local variable, which is then modified based on the package config. But the modification of this local variable will change the class variable and then leaks to the next package. The proposed solution in this patch series is to convert class member to tuple so that is read-only. Then copy into list in the local function before package specific modifications. Patch v1 branch: https://github.com/kuqin12/edk2/tree/spell_check_v1 Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Sean Brogan (1): Pytool: SpellCheck: Fix incorrect file mask across package matrices .pytool/Plugin/SpellCheck/SpellCheck.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) -- 2.31.1.windows.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices 2021-06-10 1:47 [PATCH v1 0/1] Spell Check File Mask Leak into Sequential Package Checks Kun Qin @ 2021-06-10 1:47 ` Kun Qin 2021-06-11 3:23 ` 回复: [edk2-devel] " gaoliming 0 siblings, 1 reply; 5+ messages in thread From: Kun Qin @ 2021-06-10 1:47 UTC (permalink / raw) To: devel; +Cc: Sean Brogan, Bret Barkelew, Michael D Kinney, Liming Gao From: Sean Brogan <spbrogan@live.com> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3443 Existing implementation could modify class global data that causes potential incorrect file mask to be used for execution of plugin. This change switches class variable to be tuple so that it cannot be accidently modified. Local usage of STANDARD_PLUGIN_DEFINED_PATHS is also changed to copy to new list before modification. Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Signed-off-by: Sean Brogan <sean.brogan@microsoft.com> --- .pytool/Plugin/SpellCheck/SpellCheck.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.pytool/Plugin/SpellCheck/SpellCheck.py b/.pytool/Plugin/SpellCheck/SpellCheck.py index 43365441b91c..9ad57632a6e8 100644 --- a/.pytool/Plugin/SpellCheck/SpellCheck.py +++ b/.pytool/Plugin/SpellCheck/SpellCheck.py @@ -37,12 +37,12 @@ class SpellCheck(ICiBuildPlugin): # # A package can remove any of these using IgnoreStandardPaths # - STANDARD_PLUGIN_DEFINED_PATHS = ["*.c", "*.h", + STANDARD_PLUGIN_DEFINED_PATHS = ("*.c", "*.h", "*.nasm", "*.asm", "*.masm", "*.s", "*.asl", "*.dsc", "*.dec", "*.fdf", "*.inf", "*.md", "*.txt" - ] + ) def GetTestName(self, packagename: str, environment: VarDict) -> tuple: """ Provide the testcase name and classname for use in reporting @@ -107,7 +107,8 @@ class SpellCheck(ICiBuildPlugin): version_aggregator.GetVersionAggregator().ReportVersion( "CSpell", cspell_version, version_aggregator.VersionTypes.INFO) - package_relative_paths_to_spell_check = SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS + # copy the default as a list + package_relative_paths_to_spell_check = list(SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS) # # Allow the ci.yaml to remove any of the above standard paths -- 2.31.1.windows.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices 2021-06-10 1:47 ` [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices Kun Qin @ 2021-06-11 3:23 ` gaoliming 2021-06-12 3:49 ` Kun Qin 0 siblings, 1 reply; 5+ messages in thread From: gaoliming @ 2021-06-11 3:23 UTC (permalink / raw) To: devel, kuqin12 Cc: 'Sean Brogan', 'Bret Barkelew', 'Michael D Kinney' Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> > -----邮件原件----- > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Kun Qin > 发送时间: 2021年6月10日 9:48 > 收件人: devel@edk2.groups.io > 抄送: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew > <Bret.Barkelew@microsoft.com>; Michael D Kinney > <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn> > 主题: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask > across package matrices > > From: Sean Brogan <spbrogan@live.com> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3443 > > Existing implementation could modify class global data that causes > potential incorrect file mask to be used for execution of plugin. > > This change switches class variable to be tuple so that it cannot be > accidently modified. Local usage of STANDARD_PLUGIN_DEFINED_PATHS is > also > changed to copy to new list before modification. > > Cc: Sean Brogan <sean.brogan@microsoft.com> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Signed-off-by: Sean Brogan <sean.brogan@microsoft.com> > --- > .pytool/Plugin/SpellCheck/SpellCheck.py | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/.pytool/Plugin/SpellCheck/SpellCheck.py > b/.pytool/Plugin/SpellCheck/SpellCheck.py > index 43365441b91c..9ad57632a6e8 100644 > --- a/.pytool/Plugin/SpellCheck/SpellCheck.py > +++ b/.pytool/Plugin/SpellCheck/SpellCheck.py > @@ -37,12 +37,12 @@ class SpellCheck(ICiBuildPlugin): > # > # A package can remove any of these using IgnoreStandardPaths > # > - STANDARD_PLUGIN_DEFINED_PATHS = ["*.c", "*.h", > + STANDARD_PLUGIN_DEFINED_PATHS = ("*.c", "*.h", > "*.nasm", "*.asm", "*.masm", > "*.s", > "*.asl", > "*.dsc", "*.dec", "*.fdf", > "*.inf", > "*.md", "*.txt" > - ] > + ) > > def GetTestName(self, packagename: str, environment: VarDict) -> > tuple: > """ Provide the testcase name and classname for use in reporting > @@ -107,7 +107,8 @@ class SpellCheck(ICiBuildPlugin): > version_aggregator.GetVersionAggregator().ReportVersion( > "CSpell", cspell_version, > version_aggregator.VersionTypes.INFO) > > - package_relative_paths_to_spell_check = > SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS > + # copy the default as a list > + package_relative_paths_to_spell_check = > list(SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS) > > # > # Allow the ci.yaml to remove any of the above standard paths > -- > 2.31.1.windows.1 > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 回复: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices 2021-06-11 3:23 ` 回复: [edk2-devel] " gaoliming @ 2021-06-12 3:49 ` Kun Qin 2021-06-15 7:00 ` 回复: " gaoliming 0 siblings, 1 reply; 5+ messages in thread From: Kun Qin @ 2021-06-12 3:49 UTC (permalink / raw) To: gaoliming, devel Cc: 'Sean Brogan', 'Bret Barkelew', 'Michael D Kinney' Thanks for the review, Liming. Could you please help merging this patch to the master when you have a chance? Thanks in advance! Kun On 06/10/2021 20:23, gaoliming wrote: > Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> > >> -----邮件原件----- >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Kun Qin >> 发送时间: 2021年6月10日 9:48 >> 收件人: devel@edk2.groups.io >> 抄送: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew >> <Bret.Barkelew@microsoft.com>; Michael D Kinney >> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn> >> 主题: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file > mask >> across package matrices >> >> From: Sean Brogan <spbrogan@live.com> >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3443 >> >> Existing implementation could modify class global data that causes >> potential incorrect file mask to be used for execution of plugin. >> >> This change switches class variable to be tuple so that it cannot be >> accidently modified. Local usage of STANDARD_PLUGIN_DEFINED_PATHS is >> also >> changed to copy to new list before modification. >> >> Cc: Sean Brogan <sean.brogan@microsoft.com> >> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> >> Cc: Michael D Kinney <michael.d.kinney@intel.com> >> Cc: Liming Gao <gaoliming@byosoft.com.cn> >> >> Signed-off-by: Sean Brogan <sean.brogan@microsoft.com> >> --- >> .pytool/Plugin/SpellCheck/SpellCheck.py | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/.pytool/Plugin/SpellCheck/SpellCheck.py >> b/.pytool/Plugin/SpellCheck/SpellCheck.py >> index 43365441b91c..9ad57632a6e8 100644 >> --- a/.pytool/Plugin/SpellCheck/SpellCheck.py >> +++ b/.pytool/Plugin/SpellCheck/SpellCheck.py >> @@ -37,12 +37,12 @@ class SpellCheck(ICiBuildPlugin): >> # >> # A package can remove any of these using IgnoreStandardPaths >> # >> - STANDARD_PLUGIN_DEFINED_PATHS = ["*.c", "*.h", >> + STANDARD_PLUGIN_DEFINED_PATHS = ("*.c", "*.h", >> "*.nasm", "*.asm", "*.masm", >> "*.s", >> "*.asl", >> "*.dsc", "*.dec", "*.fdf", >> "*.inf", >> "*.md", "*.txt" >> - ] >> + ) >> >> def GetTestName(self, packagename: str, environment: VarDict) -> >> tuple: >> """ Provide the testcase name and classname for use in reporting >> @@ -107,7 +107,8 @@ class SpellCheck(ICiBuildPlugin): >> version_aggregator.GetVersionAggregator().ReportVersion( >> "CSpell", cspell_version, >> version_aggregator.VersionTypes.INFO) >> >> - package_relative_paths_to_spell_check = >> SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS >> + # copy the default as a list >> + package_relative_paths_to_spell_check = >> list(SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS) >> >> # >> # Allow the ci.yaml to remove any of the above standard paths >> -- >> 2.31.1.windows.1 >> >> >> >> >> > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* 回复: 回复: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices 2021-06-12 3:49 ` Kun Qin @ 2021-06-15 7:00 ` gaoliming 0 siblings, 0 replies; 5+ messages in thread From: gaoliming @ 2021-06-15 7:00 UTC (permalink / raw) To: devel, kuqin12 Cc: 'Sean Brogan', 'Bret Barkelew', 'Michael D Kinney' Create PR https://github.com/tianocore/edk2/pull/1713 Thanks Liming > -----邮件原件----- > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Kun Qin > 发送时间: 2021年6月12日 11:50 > 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io > 抄送: 'Sean Brogan' <sean.brogan@microsoft.com>; 'Bret Barkelew' > <Bret.Barkelew@microsoft.com>; 'Michael D Kinney' > <michael.d.kinney@intel.com> > 主题: Re: 回复: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect > file mask across package matrices > > Thanks for the review, Liming. Could you please help merging this patch > to the master when you have a chance? > > Thanks in advance! > Kun > > On 06/10/2021 20:23, gaoliming wrote: > > Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> > > > >> -----邮件原件----- > >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Kun Qin > >> 发送时间: 2021年6月10日 9:48 > >> 收件人: devel@edk2.groups.io > >> 抄送: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew > >> <Bret.Barkelew@microsoft.com>; Michael D Kinney > >> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn> > >> 主题: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file > > mask > >> across package matrices > >> > >> From: Sean Brogan <spbrogan@live.com> > >> > >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3443 > >> > >> Existing implementation could modify class global data that causes > >> potential incorrect file mask to be used for execution of plugin. > >> > >> This change switches class variable to be tuple so that it cannot be > >> accidently modified. Local usage of STANDARD_PLUGIN_DEFINED_PATHS > is > >> also > >> changed to copy to new list before modification. > >> > >> Cc: Sean Brogan <sean.brogan@microsoft.com> > >> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > >> Cc: Michael D Kinney <michael.d.kinney@intel.com> > >> Cc: Liming Gao <gaoliming@byosoft.com.cn> > >> > >> Signed-off-by: Sean Brogan <sean.brogan@microsoft.com> > >> --- > >> .pytool/Plugin/SpellCheck/SpellCheck.py | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/.pytool/Plugin/SpellCheck/SpellCheck.py > >> b/.pytool/Plugin/SpellCheck/SpellCheck.py > >> index 43365441b91c..9ad57632a6e8 100644 > >> --- a/.pytool/Plugin/SpellCheck/SpellCheck.py > >> +++ b/.pytool/Plugin/SpellCheck/SpellCheck.py > >> @@ -37,12 +37,12 @@ class SpellCheck(ICiBuildPlugin): > >> # > >> # A package can remove any of these using IgnoreStandardPaths > >> # > >> - STANDARD_PLUGIN_DEFINED_PATHS = ["*.c", "*.h", > >> + STANDARD_PLUGIN_DEFINED_PATHS = ("*.c", "*.h", > >> "*.nasm", "*.asm", > "*.masm", > >> "*.s", > >> "*.asl", > >> "*.dsc", "*.dec", "*.fdf", > >> "*.inf", > >> "*.md", "*.txt" > >> - ] > >> + ) > >> > >> def GetTestName(self, packagename: str, environment: VarDict) -> > >> tuple: > >> """ Provide the testcase name and classname for use in > reporting > >> @@ -107,7 +107,8 @@ class SpellCheck(ICiBuildPlugin): > >> version_aggregator.GetVersionAggregator().ReportVersion( > >> "CSpell", cspell_version, > >> version_aggregator.VersionTypes.INFO) > >> > >> - package_relative_paths_to_spell_check = > >> SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS > >> + # copy the default as a list > >> + package_relative_paths_to_spell_check = > >> list(SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS) > >> > >> # > >> # Allow the ci.yaml to remove any of the above standard > paths > >> -- > >> 2.31.1.windows.1 > >> > >> > >> > >> > >> > > > > > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-15 7:00 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-06-10 1:47 [PATCH v1 0/1] Spell Check File Mask Leak into Sequential Package Checks Kun Qin 2021-06-10 1:47 ` [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices Kun Qin 2021-06-11 3:23 ` 回复: [edk2-devel] " gaoliming 2021-06-12 3:49 ` Kun Qin 2021-06-15 7:00 ` 回复: " gaoliming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox