Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning invalid escape sequence \C

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi JP,

I agree this is a simple change and the review was started before
the soft freeze, but a review by the maintainer did not occur until
the hard freeze.  This means it needs to meet the critical bug criteria.

This change removes an incorrect warning message from the build log,
but does not appear to have any impact on build behavior/output.

I recommend this patch merge be deferred until after the 
edk2-stable202402 release.

Thanks,

Mike

> -----Original Message-----
> From: Jayaprakash, N <n.jayaprakash@intel.com>
> Sent: Wednesday, February 14, 2024 7:10 AM
> To: Rebecca Cran <rebecca@bsdio.com>; devel@edk2.groups.io; Pedro
> Falcato <pedro.falcato@gmail.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Liming Gao <gaoliming@byosoft.com.cn>; Feng, Bob C
> <bob.c.feng@intel.com>; Chen, Christine <yuwei.chen@intel.com>
> Subject: RE: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning
> invalid escape sequence \C
> 
> Thanks Rebecca for the review.
> 
> This is an harmless simple change
> Will it be considered for the Feb 2024 stable release of edk2?
> 
> Regards,
> JP
> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Wednesday, February 14, 2024 8:33 PM
> To: Jayaprakash, N <n.jayaprakash@intel.com>; devel@edk2.groups.io;
> Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Liming Gao <gaoliming@byosoft.com.cn>; Feng, Bob C
> <bob.c.feng@intel.com>; Chen, Christine <yuwei.chen@intel.com>
> Subject: Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning
> invalid escape sequence \C
> 
> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> 
> 
> On 2/13/24 09:24, Jayaprakash, N wrote:
> > Could one of the maintainers of this package review this patch?
> >
> > Regards,
> > JP
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Jayaprakash, N
> > Sent: Friday, February 9, 2024 4:07 PM
> > To: devel@edk2.groups.io; Jayaprakash, N <n.jayaprakash@intel.com>;
> > Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Liming
> > Gao <gaoliming@byosoft.com.cn>; Feng, Bob C <bob.c.feng@intel.com>;
> > Chen, Christine <yuwei.chen@intel.com>
> > Subject: Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning
> > invalid escape sequence \C
> >
> > Hi Pedro,
> >
> > Do you agree with the below thoughts? Let me know if you have any
> questions.
> >
> > Regards,
> > JP
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Jayaprakash, N
> > Sent: Wednesday, February 7, 2024 8:58 AM
> > To: Pedro Falcato <pedro.falcato@gmail.com>; devel@edk2.groups.io
> > Cc: Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Liming
> > Gao <gaoliming@byosoft.com.cn>; Feng, Bob C <bob.c.feng@intel.com>;
> > Chen, Christine <yuwei.chen@intel.com>
> > Subject: Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning
> > invalid escape sequence \C
> >
> > Thanks Pedro.
> >
> > I thought about using / instead of \\ but in many places, the path
> separator \\ is still used across the code.
> > But after looking at the usage of path separator in other parts of
> the code. I decided to go with \\.
> >
> > For example:
> > In the same file:
> > 2953 : MakeApp = MakeApp + '%s\\PcdValueCommon.c : %s\n' %
> > (self.OutputPath, PcdValueCommonPath)
> >
> > 2842: if sys.platform == "win32":
> >              MakeApp = MakeApp + 'APPFILE = %s\\%s.exe\n' %
> (self.OutputPath, PcdValueInitName) + 'APPNAME = %s\n' %
> (PcdValueInitName) + 'OBJECTS = %s\\%s.obj %s.obj\n' %
> (self.OutputPath, PcdValueInitName, os.path.join(self.OutputPath,
> PcdValueCommonName)) + 'INC = '
> >
> > Hence to align with the usage of path separator for "win32", I have
> used the \\ instead of /.
> >
> > Hope this is fine.
> >
> > Regards,
> > JP
> >
> > -----Original Message-----
> > From: Pedro Falcato <pedro.falcato@gmail.com>
> > Sent: Tuesday, February 6, 2024 10:14 PM
> > To: devel@edk2.groups.io; Jayaprakash, N <n.jayaprakash@intel.com>
> > Cc: Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Liming
> > Gao <gaoliming@byosoft.com.cn>; Feng, Bob C <bob.c.feng@intel.com>;
> > Chen, Christine <yuwei.chen@intel.com>
> > Subject: Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning
> > invalid escape sequence \C
> >
> > On Tue, Feb 6, 2024 at 7:02 AM Jayaprakash, N
> <n.jayaprakash@intel.com> wrote:
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4666
> >>
> >> This commit fixes the issue reported through BZ4666.
> >> The Syntax warning related to invalid escape sequence for \C is seen
> >> on Windows OS based builds of edk2 sources.
> >> On Windows the path seperator needs to prefixed with \ so
> essentially
> >> we need to use \\ as path seperator.
> >>
> >> Cc: Rebecca Cran <rebecca@bsdio.com>
> >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >> Cc: Bob Feng <bob.c.feng@intel.com>
> >> Cc: Yuwei Chen <yuwei.chen@intel.com>
> >> Cc: Jayaprakash N <n.jayaprakash@intel.com>
> >> Signed-off-by: Jayaprakash N <n.jayaprakash@intel.com>
> >> ---
> >>   BaseTools/Source/Python/Workspace/DscBuildData.py | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> >> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> >> index 4768099343..b69d406249 100644
> >> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> >> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> >> @@ -2949,7 +2949,7 @@ class DscBuildData(PlatformBuildClassObject):
> >>           for include_file in IncFileList:
> >>               MakeApp += "$(OBJECTS) : %s\n" % include_file
> >>           if sys.platform == "win32":
> >> -            PcdValueCommonPath =
> os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"],
> "Source\C\Common\PcdValueCommon.c"))
> >> +            PcdValueCommonPath =
> >> +
> os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"
> >> + ], "Source\\C\\Common\\PcdValueCommon.c"))
> > AIUI, changing the \ to / would also work (and it would be
> prettier/more readable), since we're calling normpath right after.
> >
> > --
> > Pedro
> >
> >
> >
> >
> >
> >
> >
> > 
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115468): https://edk2.groups.io/g/devel/message/115468
Mute This Topic: https://groups.io/mt/104193926/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-