[PATCH v1 0/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[PATCH v1 0/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Chris Co]

REF: https://github.com/christopherco/edk2/tree/trim_gcc_v1

When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
DSDT.iii contains a canonicalized ("\.\" removed and lower case)
filepath for the preprocessed DSDT.i file in the first line.
Due to this, when Trim.exe is called to generate DSDT.iiii, future
filepath comparisons against this canonicalized filepath, which
should match, actually fail the comparison which results in an
empty DSDT.iiii.

Issue was first reported to Linaro here:
https://bugs.linaro.org/show_bug.cgi?id=2909
where the recommendation was to address the issue in Trim.exe.

This patch canonicalizes and lower cases all file paths encountered
during trim execution on preprocessed files.  Since file paths are
standarized, the comparison succeeds for files that should match
regardless of the presence of upper case or "\.\" characters in
the file path.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Christopher Co <christopher.co@microsoft.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>


Christopher Co (1):
  BaseTools/Trim: Canonicalize filepaths to fix comparison

 BaseTools/Source/Python/Trim/Trim.py | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.16.2.gvfs.1.33.gf5370f1

[PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Chris Co]

When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
DSDT.iii contains a canonicalized ("\.\" removed and lower case)
filepath for the preprocessed DSDT.i file in the first line.
Due to this, when Trim.exe is called to generate DSDT.iiii, future
filepath comparisons against this canonicalized filepath, which
should match, actually fail the comparison which results in an
empty DSDT.iiii.

Issue was first reported to Linaro here:
https://bugs.linaro.org/show_bug.cgi?id=2909
where the recommendation was to address the issue in Trim.exe.

This patch canonicalizes and lower cases all file paths encountered
during trim execution on preprocessed files.  Since file paths are
standarized, the comparison succeeds for files that should match
regardless of the presence of upper case or "\.\" characters in
the file path.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Christopher Co <christopher.co@microsoft.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Source/Python/Trim/Trim.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Source/Python/Trim/Trim.py b/BaseTools/Source/Python/Trim/Trim.py
index a74075859148..cca4e5c9694a 100644
--- a/BaseTools/Source/Python/Trim/Trim.py
+++ b/BaseTools/Source/Python/Trim/Trim.py
@@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target, ConvertHex, TrimLong):
             if len(MatchList) == 2:
                 LineNumber = int(MatchList[0], 0)
                 InjectedFile = MatchList[1]
+                InjectedFile = InjectedFile.replace("\\.\\","")
+                InjectedFile = InjectedFile.lower()
                 # The first injetcted file must be the preprocessed file itself
                 if PreprocessedFile == "":
                     PreprocessedFile = InjectedFile
-- 
2.16.2.gvfs.1.33.gf5370f1

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Gao, Liming]

Chris:
  Could you provide the generated Makefile for DSDT.asl? I would like to see what command is generated in Makefile to handle DSDT.asl. 

Thanks
Liming
>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Chris Co
>Sent: Wednesday, June 27, 2018 11:58 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming <liming.gao@intel.com>
>Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
>comparison
>
>When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
>DSDT.iii contains a canonicalized ("\.\" removed and lower case)
>filepath for the preprocessed DSDT.i file in the first line.
>Due to this, when Trim.exe is called to generate DSDT.iiii, future
>filepath comparisons against this canonicalized filepath, which
>should match, actually fail the comparison which results in an
>empty DSDT.iiii.
>
>Issue was first reported to Linaro here:
>https://bugs.linaro.org/show_bug.cgi?id=2909
>where the recommendation was to address the issue in Trim.exe.
>
>This patch canonicalizes and lower cases all file paths encountered
>during trim execution on preprocessed files.  Since file paths are
>standarized, the comparison succeeds for files that should match
>regardless of the presence of upper case or "\.\" characters in
>the file path.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Christopher Co <christopher.co@microsoft.com>
>Cc: Leif Lindholm <leif.lindholm@linaro.org>
>Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>---
> BaseTools/Source/Python/Trim/Trim.py | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/BaseTools/Source/Python/Trim/Trim.py
>b/BaseTools/Source/Python/Trim/Trim.py
>index a74075859148..cca4e5c9694a 100644
>--- a/BaseTools/Source/Python/Trim/Trim.py
>+++ b/BaseTools/Source/Python/Trim/Trim.py
>@@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target, ConvertHex,
>TrimLong):
>             if len(MatchList) == 2:
>                 LineNumber = int(MatchList[0], 0)
>                 InjectedFile = MatchList[1]
>+                InjectedFile = InjectedFile.replace("\\.\\","")
>+                InjectedFile = InjectedFile.lower()
>                 # The first injetcted file must be the preprocessed file itself
>                 if PreprocessedFile == "":
>                     PreprocessedFile = InjectedFile
>--
>2.16.2.gvfs.1.33.gf5370f1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Leif Lindholm]

Sami, Evan? I think we need your input/review here.

On Wed, Jun 27, 2018 at 03:58:06AM +0000, Chris Co wrote:
> When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
> DSDT.iii contains a canonicalized ("\.\" removed and lower case)
> filepath for the preprocessed DSDT.i file in the first line.
> Due to this, when Trim.exe is called to generate DSDT.iiii, future
> filepath comparisons against this canonicalized filepath, which
> should match, actually fail the comparison which results in an
> empty DSDT.iiii.
> 
> Issue was first reported to Linaro here:
> https://bugs.linaro.org/show_bug.cgi?id=2909
> where the recommendation was to address the issue in Trim.exe.
> 
> This patch canonicalizes and lower cases all file paths encountered
> during trim execution on preprocessed files.  Since file paths are
> standarized, the comparison succeeds for files that should match
> regardless of the presence of upper case or "\.\" characters in
> the file path.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> ---
>  BaseTools/Source/Python/Trim/Trim.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/Trim/Trim.py b/BaseTools/Source/Python/Trim/Trim.py
> index a74075859148..cca4e5c9694a 100644
> --- a/BaseTools/Source/Python/Trim/Trim.py
> +++ b/BaseTools/Source/Python/Trim/Trim.py
> @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target, ConvertHex, TrimLong):
>              if len(MatchList) == 2:
>                  LineNumber = int(MatchList[0], 0)
>                  InjectedFile = MatchList[1]
> +                InjectedFile = InjectedFile.replace("\\.\\","")
> +                InjectedFile = InjectedFile.lower()
>                  # The first injetcted file must be the preprocessed file itself
>                  if PreprocessedFile == "":
>                      PreprocessedFile = InjectedFile
> -- 
> 2.16.2.gvfs.1.33.gf5370f1
> 

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Evan Lloyd]

Hi Chris.
Firstly, thank you: this is a useful, pragmatic solution to an major annoyance.
I personally think it unfortunate that the GCC guys can't be bothered to fix their compiler, but ...

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Chris Co
> Sent: 27 June 2018 04:58
> To: edk2-devel@lists.01.org
> Cc: Liming Gao <liming.gao@intel.com>
> Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
> comparison
>
> When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
> DSDT.iii contains a canonicalized ("\.\" removed and lower case) filepath for
> the preprocessed DSDT.i file in the first line.
> Due to this, when Trim.exe is called to generate DSDT.iiii, future filepath
> comparisons against this canonicalized filepath, which should match, actually
> fail the comparison which results in an empty DSDT.iiii.
>
> Issue was first reported to Linaro here:
> https://bugs.linaro.org/show_bug.cgi?id=2909
> where the recommendation was to address the issue in Trim.exe.
>
> This patch canonicalizes and lower cases all file paths encountered during
> trim execution on preprocessed files.  Since file paths are standarized, the
> comparison succeeds for files that should match regardless of the presence
> of upper case or "\.\" characters in the file path.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> ---
>  BaseTools/Source/Python/Trim/Trim.py | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/BaseTools/Source/Python/Trim/Trim.py
> b/BaseTools/Source/Python/Trim/Trim.py
> index a74075859148..cca4e5c9694a 100644
> --- a/BaseTools/Source/Python/Trim/Trim.py
> +++ b/BaseTools/Source/Python/Trim/Trim.py
> @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target, ConvertHex,
> TrimLong):
>              if len(MatchList) == 2:
>                  LineNumber = int(MatchList[0], 0)
>                  InjectedFile = MatchList[1]
> +                InjectedFile = InjectedFile.replace("\\.\\","")

[[Evan Lloyd]] I've not actually tried this yet, but it looks surprizing.  I'd have expected InjectedFile.replace("\\.\\","\\").
          Can I ask how it works, please?  Have I missed something?
[[Evan Lloyd]] Would it be possible to achieve the same effect with os.path.normpath (see Common/LongFilePathOsPath.py imported via line 17)?

> +                InjectedFile = InjectedFile.lower()
[[Evan Lloyd]] Similarly, there is "os.path.normcase" - but that may not get the comparison working (because it converts '/' to '\' on Windows).

>                  # The first injetcted file must be the preprocessed file itself
>                  if PreprocessedFile == "":
>                      PreprocessedFile = InjectedFile
> --
> 2.16.2.gvfs.1.33.gf5370f1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Chris Co]

Hi Liming, Evan,

I have attached an example of a generated DSDT.iii file which gets processed using Trim.  It will give us a concrete example of what these changes do.

The first line has its #line directive stripped and the filepath saved as the PreProcessedFile.  In GCC49, the filepath was:
"e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC49xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\AcpiTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"

while in GCC5+ it is:
"e:\\rebase\\build\\hummingboard_edge_imx6q_2gb\\release_gcc5\\arm\\platform\\solidrun\\hummingboard_edge_imx6q_2gb\\acpitables\\acpitables\\output\\dsdt.i"

So there is a difference in both lowercase as well as \\.\\ before dsdt.i.

As the Trim function goes through each line, it strips the #line directive and saves the filepath as InjectedFile, which is then compared against PreProcessedFile (https://github.com/tianocore/edk2/blob/975478f6bb22668efae311eb3f7406e1f18411c2/BaseTools/Source/Python/Trim/Trim.py#L174)
If it is not a match, skip to next line.  When using GCC5+, DSDT.iii's #line directives after the first keeps the old GCC49 filepath syntax so the string comparison always fails.  An example of a line which should match line 602 in DSDT.iii.

So my changes target these two cases directly, but admittedly I'm more of a kernel dev than a python dev so if there is a better way to address this, I'm all for it.

> -----Original Message-----
> From: Evan Lloyd <Evan.Lloyd@arm.com>
> Sent: Wednesday, June 27, 2018 4:35 AM
> To: Chris Co <Christopher.Co@microsoft.com>; edk2-devel@lists.01.org
> Cc: Liming Gao <liming.gao@intel.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>
> Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
> comparison
> 
> Hi Chris.
> Firstly, thank you: this is a useful, pragmatic solution to an major annoyance.
> I personally think it unfortunate that the GCC guys can't be bothered to fix
> their compiler, but ...
> 
> > -----Original Message-----
> > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Chris
> > Co
> > Sent: 27 June 2018 04:58
> > To: edk2-devel@lists.01.org
> > Cc: Liming Gao <liming.gao@intel.com>
> > Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths
> > to fix comparison
> >
> > When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
> > DSDT.iii contains a canonicalized ("\.\" removed and lower case)
> > filepath for the preprocessed DSDT.i file in the first line.
> > Due to this, when Trim.exe is called to generate DSDT.iiii, future
> > filepath comparisons against this canonicalized filepath, which should
> > match, actually fail the comparison which results in an empty DSDT.iiii.
> >
> > Issue was first reported to Linaro here:
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.
> >
> linaro.org%2Fshow_bug.cgi%3Fid%3D2909&amp;data=02%7C01%7CChristop
> her.C
> >
> o%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988bf86
> f141a
> >
> f91ab2d7cd011db47%7C1%7C0%7C636656961074375810&amp;sdata=%2FvFv
> maBD0ul
> > z89CY%2BMaOpwvpTvK9%2FxXjeZeSiXUCYH0%3D&amp;reserved=0
> > where the recommendation was to address the issue in Trim.exe.
> >
> > This patch canonicalizes and lower cases all file paths encountered
> > during trim execution on preprocessed files.  Since file paths are
> > standarized, the comparison succeeds for files that should match
> > regardless of the presence of upper case or "\.\" characters in the file path.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > ---
> >  BaseTools/Source/Python/Trim/Trim.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/BaseTools/Source/Python/Trim/Trim.py
> > b/BaseTools/Source/Python/Trim/Trim.py
> > index a74075859148..cca4e5c9694a 100644
> > --- a/BaseTools/Source/Python/Trim/Trim.py
> > +++ b/BaseTools/Source/Python/Trim/Trim.py
> > @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target,
> > ConvertHex,
> > TrimLong):
> >              if len(MatchList) == 2:
> >                  LineNumber = int(MatchList[0], 0)
> >                  InjectedFile = MatchList[1]
> > +                InjectedFile = InjectedFile.replace("\\.\\","")
> 
> [[Evan Lloyd]] I've not actually tried this yet, but it looks surprizing.  I'd have
> expected InjectedFile.replace("\\.\\","\\").
>           Can I ask how it works, please?  Have I missed something?
> [[Evan Lloyd]] Would it be possible to achieve the same effect with
> os.path.normpath (see Common/LongFilePathOsPath.py imported via line
> 17)?
> 
> > +                InjectedFile = InjectedFile.lower()
> [[Evan Lloyd]] Similarly, there is "os.path.normcase" - but that may not get
> the comparison working (because it converts '/' to '\' on Windows).
> 
> >                  # The first injetcted file must be the preprocessed file itself
> >                  if PreprocessedFile == "":
> >                      PreprocessedFile = InjectedFile
> > --
> > 2.16.2.gvfs.1.33.gf5370f1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists
> > .01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&amp;data=02%7C01%7CChristoph
> >
> er.Co%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988
> bf86f
> >
> 141af91ab2d7cd011db47%7C1%7C0%7C636656961074385815&amp;sdata=uu
> S9M90lN
> > ewzqS6ScNwbSm8xC9wpdzAFVZhjEu8w2kk%3D&amp;reserved=0
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Chris Co]

Forgot to add that I'll try out the os.path.normpath/normcase to see if this will work.

> -----Original Message-----
> From: Chris Co
> Sent: Wednesday, June 27, 2018 11:13 AM
> To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; edk2-devel@lists.01.org; Liming
> Gao <liming.gao@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar
> <Sami.Mujawar@arm.com>
> Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
> comparison
> 
> Hi Liming, Evan,
> 
> I have attached an example of a generated DSDT.iii file which gets processed
> using Trim.  It will give us a concrete example of what these changes do.
> 
> The first line has its #line directive stripped and the filepath saved as the
> PreProcessedFile.  In GCC49, the filepath was:
> "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC4
> 9xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\
> AcpiTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
> 
> while in GCC5+ it is:
> "e:\\rebase\\build\\hummingboard_edge_imx6q_2gb\\release_gcc5\\arm\
> \platform\\solidrun\\hummingboard_edge_imx6q_2gb\\acpitables\\acpitab
> les\\output\\dsdt.i"
> 
> So there is a difference in both lowercase as well as \\.\\ before dsdt.i.
> 
> As the Trim function goes through each line, it strips the #line directive and
> saves the filepath as InjectedFile, which is then compared against
> PreProcessedFile
> (https://github.com/tianocore/edk2/blob/975478f6bb22668efae311eb3f740
> 6e1f18411c2/BaseTools/Source/Python/Trim/Trim.py#L174)
> If it is not a match, skip to next line.  When using GCC5+, DSDT.iii's #line
> directives after the first keeps the old GCC49 filepath syntax so the string
> comparison always fails.  An example of a line which should match line 602 in
> DSDT.iii.
> 
> So my changes target these two cases directly, but admittedly I'm more of a
> kernel dev than a python dev so if there is a better way to address this, I'm all
> for it.
> 
> > -----Original Message-----
> > From: Evan Lloyd <Evan.Lloyd@arm.com>
> > Sent: Wednesday, June 27, 2018 4:35 AM
> > To: Chris Co <Christopher.Co@microsoft.com>; edk2-devel@lists.01.org
> > Cc: Liming Gao <liming.gao@intel.com>; Leif Lindholm
> > <leif.lindholm@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>
> > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to
> > fix comparison
> >
> > Hi Chris.
> > Firstly, thank you: this is a useful, pragmatic solution to an major
> annoyance.
> > I personally think it unfortunate that the GCC guys can't be bothered
> > to fix their compiler, but ...
> >
> > > -----Original Message-----
> > > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
> > > Chris Co
> > > Sent: 27 June 2018 04:58
> > > To: edk2-devel@lists.01.org
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize
> > > filepaths to fix comparison
> > >
> > > When using Linaro GCC5+ arm-eabi toolchain on Windows, the generated
> > > DSDT.iii contains a canonicalized ("\.\" removed and lower case)
> > > filepath for the preprocessed DSDT.i file in the first line.
> > > Due to this, when Trim.exe is called to generate DSDT.iiii, future
> > > filepath comparisons against this canonicalized filepath, which
> > > should match, actually fail the comparison which results in an empty
> DSDT.iiii.
> > >
> > > Issue was first reported to Linaro here:
> > >
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.
> > >
> >
> linaro.org%2Fshow_bug.cgi%3Fid%3D2909&amp;data=02%7C01%7CChristop
> > her.C
> > >
> >
> o%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988bf86
> > f141a
> > >
> >
> f91ab2d7cd011db47%7C1%7C0%7C636656961074375810&amp;sdata=%2FvFv
> > maBD0ul
> > > z89CY%2BMaOpwvpTvK9%2FxXjeZeSiXUCYH0%3D&amp;reserved=0
> > > where the recommendation was to address the issue in Trim.exe.
> > >
> > > This patch canonicalizes and lower cases all file paths encountered
> > > during trim execution on preprocessed files.  Since file paths are
> > > standarized, the comparison succeeds for files that should match
> > > regardless of the presence of upper case or "\.\" characters in the file
> path.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > ---
> > >  BaseTools/Source/Python/Trim/Trim.py | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/BaseTools/Source/Python/Trim/Trim.py
> > > b/BaseTools/Source/Python/Trim/Trim.py
> > > index a74075859148..cca4e5c9694a 100644
> > > --- a/BaseTools/Source/Python/Trim/Trim.py
> > > +++ b/BaseTools/Source/Python/Trim/Trim.py
> > > @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target,
> > > ConvertHex,
> > > TrimLong):
> > >              if len(MatchList) == 2:
> > >                  LineNumber = int(MatchList[0], 0)
> > >                  InjectedFile = MatchList[1]
> > > +                InjectedFile = InjectedFile.replace("\\.\\","")
> >
> > [[Evan Lloyd]] I've not actually tried this yet, but it looks
> > surprizing.  I'd have expected InjectedFile.replace("\\.\\","\\").
> >           Can I ask how it works, please?  Have I missed something?
> > [[Evan Lloyd]] Would it be possible to achieve the same effect with
> > os.path.normpath (see Common/LongFilePathOsPath.py imported via line
> > 17)?
> >
> > > +                InjectedFile = InjectedFile.lower()
> > [[Evan Lloyd]] Similarly, there is "os.path.normcase" - but that may
> > not get the comparison working (because it converts '/' to '\' on Windows).
> >
> > >                  # The first injetcted file must be the preprocessed file itself
> > >                  if PreprocessedFile == "":
> > >                      PreprocessedFile = InjectedFile
> > > --
> > > 2.16.2.gvfs.1.33.gf5370f1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > > ts
> > > .01.org%2Fmailman%2Flistinfo%2Fedk2-
> > devel&amp;data=02%7C01%7CChristoph
> > >
> >
> er.Co%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988
> > bf86f
> > >
> >
> 141af91ab2d7cd011db47%7C1%7C0%7C636656961074385815&amp;sdata=uu
> > S9M90lN
> > > ewzqS6ScNwbSm8xC9wpdzAFVZhjEu8w2kk%3D&amp;reserved=0
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> > confidential and may also be privileged. If you are not the intended
> > recipient, please notify the sender immediately and do not disclose
> > the contents to any other person, use it for any purpose, or store or
> > copy the information in any medium. Thank you.

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Chris Co]

> -----Original Message-----
> From: Chris Co
> Sent: Wednesday, June 27, 2018 11:23 AM
> To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; 'edk2-devel@lists.01.org' <edk2-
> devel@lists.01.org>; 'Liming Gao' <liming.gao@intel.com>
> Cc: 'Leif Lindholm' <leif.lindholm@linaro.org>; Sami Mujawar
> <Sami.Mujawar@arm.com>
> Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
> comparison
> 
> Forgot to add that I'll try out the os.path.normpath/normcase to see if this
> will work.
> 

Just tried out os.path.normpath and os.path.normcase and together, these calls do achieve the desired result.

Initial filepath string = "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC49xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\AcpiTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
After normpath = "e:\rebase\Build\HUMMINGBOARD_EDGE_IMX6Q_2GB\RELEASE_GCC49xASL\ARM\Platform\SolidRun\HUMMINGBOARD_EDGE_IMX6Q_2GB\AcpiTables\AcpiTables\OUTPUT\DSDT.i"
After normcase = "e:\rebase\build\hummingboard_edge_imx6q_2gb\release_gcc49xasl\arm\platform\solidrun\hummingboard_edge_imx6q_2gb\acpitables\acpitables\output\dsdt.i"

I would prefer to use the os module instead of my direct string manipulation approach so let me know if this change is acceptable and I can submit a v2 using these functions instead.

> > -----Original Message-----
> > From: Chris Co
> > Sent: Wednesday, June 27, 2018 11:13 AM
> > To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; edk2-devel@lists.01.org; Liming
> > Gao <liming.gao@intel.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar
> > <Sami.Mujawar@arm.com>
> > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to
> > fix comparison
> >
> > Hi Liming, Evan,
> >
> > I have attached an example of a generated DSDT.iii file which gets
> > processed using Trim.  It will give us a concrete example of what these
> changes do.
> >
> > The first line has its #line directive stripped and the filepath saved
> > as the PreProcessedFile.  In GCC49, the filepath was:
> >
> "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC4
> >
> 9xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\
> > AcpiTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
> >
> > while in GCC5+ it is:
> >
> "e:\\rebase\\build\\hummingboard_edge_imx6q_2gb\\release_gcc5\\arm\
> >
> \platform\\solidrun\\hummingboard_edge_imx6q_2gb\\acpitables\\acpitab
> > les\\output\\dsdt.i"
> >
> > So there is a difference in both lowercase as well as \\.\\ before dsdt.i.
> >
> > As the Trim function goes through each line, it strips the #line
> > directive and saves the filepath as InjectedFile, which is then
> > compared against PreProcessedFile
> >
> (https://github.com/tianocore/edk2/blob/975478f6bb22668efae311eb3f740
> > 6e1f18411c2/BaseTools/Source/Python/Trim/Trim.py#L174)
> > If it is not a match, skip to next line.  When using GCC5+, DSDT.iii's
> > #line directives after the first keeps the old GCC49 filepath syntax
> > so the string comparison always fails.  An example of a line which
> > should match line 602 in DSDT.iii.
> >
> > So my changes target these two cases directly, but admittedly I'm more
> > of a kernel dev than a python dev so if there is a better way to
> > address this, I'm all for it.
> >
> > > -----Original Message-----
> > > From: Evan Lloyd <Evan.Lloyd@arm.com>
> > > Sent: Wednesday, June 27, 2018 4:35 AM
> > > To: Chris Co <Christopher.Co@microsoft.com>; edk2-devel@lists.01.org
> > > Cc: Liming Gao <liming.gao@intel.com>; Leif Lindholm
> > > <leif.lindholm@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>
> > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths
> > > to fix comparison
> > >
> > > Hi Chris.
> > > Firstly, thank you: this is a useful, pragmatic solution to an major
> > annoyance.
> > > I personally think it unfortunate that the GCC guys can't be
> > > bothered to fix their compiler, but ...
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
> > > > Chris Co
> > > > Sent: 27 June 2018 04:58
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize
> > > > filepaths to fix comparison
> > > >
> > > > When using Linaro GCC5+ arm-eabi toolchain on Windows, the
> > > > generated DSDT.iii contains a canonicalized ("\.\" removed and
> > > > lower case) filepath for the preprocessed DSDT.i file in the first line.
> > > > Due to this, when Trim.exe is called to generate DSDT.iiii, future
> > > > filepath comparisons against this canonicalized filepath, which
> > > > should match, actually fail the comparison which results in an
> > > > empty
> > DSDT.iiii.
> > > >
> > > > Issue was first reported to Linaro here:
> > > >
> > >
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.
> > > >
> > >
> >
> linaro.org%2Fshow_bug.cgi%3Fid%3D2909&amp;data=02%7C01%7CChristop
> > > her.C
> > > >
> > >
> >
> o%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988bf86
> > > f141a
> > > >
> > >
> >
> f91ab2d7cd011db47%7C1%7C0%7C636656961074375810&amp;sdata=%2FvFv
> > > maBD0ul
> > > > z89CY%2BMaOpwvpTvK9%2FxXjeZeSiXUCYH0%3D&amp;reserved=0
> > > > where the recommendation was to address the issue in Trim.exe.
> > > >
> > > > This patch canonicalizes and lower cases all file paths
> > > > encountered during trim execution on preprocessed files.  Since
> > > > file paths are standarized, the comparison succeeds for files that
> > > > should match regardless of the presence of upper case or "\.\"
> > > > characters in the file
> > path.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > ---
> > > >  BaseTools/Source/Python/Trim/Trim.py | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/BaseTools/Source/Python/Trim/Trim.py
> > > > b/BaseTools/Source/Python/Trim/Trim.py
> > > > index a74075859148..cca4e5c9694a 100644
> > > > --- a/BaseTools/Source/Python/Trim/Trim.py
> > > > +++ b/BaseTools/Source/Python/Trim/Trim.py
> > > > @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target,
> > > > ConvertHex,
> > > > TrimLong):
> > > >              if len(MatchList) == 2:
> > > >                  LineNumber = int(MatchList[0], 0)
> > > >                  InjectedFile = MatchList[1]
> > > > +                InjectedFile = InjectedFile.replace("\\.\\","")
> > >
> > > [[Evan Lloyd]] I've not actually tried this yet, but it looks
> > > surprizing.  I'd have expected InjectedFile.replace("\\.\\","\\").
> > >           Can I ask how it works, please?  Have I missed something?
> > > [[Evan Lloyd]] Would it be possible to achieve the same effect with
> > > os.path.normpath (see Common/LongFilePathOsPath.py imported via
> line
> > > 17)?
> > >
> > > > +                InjectedFile = InjectedFile.lower()
> > > [[Evan Lloyd]] Similarly, there is "os.path.normcase" - but that may
> > > not get the comparison working (because it converts '/' to '\' on
> Windows).
> > >
> > > >                  # The first injetcted file must be the preprocessed file itself
> > > >                  if PreprocessedFile == "":
> > > >                      PreprocessedFile = InjectedFile
> > > > --
> > > > 2.16.2.gvfs.1.33.gf5370f1
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl
> > > > is
> > > > ts
> > > > .01.org%2Fmailman%2Flistinfo%2Fedk2-
> > > devel&amp;data=02%7C01%7CChristoph
> > > >
> > >
> >
> er.Co%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988
> > > bf86f
> > > >
> > >
> >
> 141af91ab2d7cd011db47%7C1%7C0%7C636656961074385815&amp;sdata=uu
> > > S9M90lN
> > > > ewzqS6ScNwbSm8xC9wpdzAFVZhjEu8w2kk%3D&amp;reserved=0
> > > IMPORTANT NOTICE: The contents of this email and any attachments are
> > > confidential and may also be privileged. If you are not the intended
> > > recipient, please notify the sender immediately and do not disclose
> > > the contents to any other person, use it for any purpose, or store
> > > or copy the information in any medium. Thank you.

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Evan Lloyd]

Hi Chris.

> -----Original Message-----
> From: Chris Co <Christopher.Co@microsoft.com>
> Sent: 27 June 2018 21:03
> To: Evan Lloyd <Evan.Lloyd@arm.com>; edk2-devel@lists.01.org; Liming Gao
> <liming.gao@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar
> <Sami.Mujawar@arm.com>
> Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
> comparison
>
> > -----Original Message-----
> > From: Chris Co
> > Sent: Wednesday, June 27, 2018 11:23 AM
> > To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; 'edk2-devel@lists.01.org'
> > <edk2- devel@lists.01.org>; 'Liming Gao' <liming.gao@intel.com>
> > Cc: 'Leif Lindholm' <leif.lindholm@linaro.org>; Sami Mujawar
> > <Sami.Mujawar@arm.com>
> > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to
> > fix comparison
> >
> > Forgot to add that I'll try out the os.path.normpath/normcase to see
> > if this will work.
> >
>
> Just tried out os.path.normpath and os.path.normcase and together, these
> calls do achieve the desired result.
>
> Initial filepath string =
> "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC49
> xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\Ac
> piTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
> After normpath =
> "e:\rebase\Build\HUMMINGBOARD_EDGE_IMX6Q_2GB\RELEASE_GCC49xAS
> L\ARM\Platform\SolidRun\HUMMINGBOARD_EDGE_IMX6Q_2GB\AcpiTables
> \AcpiTables\OUTPUT\DSDT.i"
> After normcase =
> "e:\rebase\build\hummingboard_edge_imx6q_2gb\release_gcc49xasl\arm\
> platform\solidrun\hummingboard_edge_imx6q_2gb\acpitables\acpitables\o
> utput\dsdt.i"
>
> I would prefer to use the os module instead of my direct string manipulation
> approach so let me know if this change is acceptable and I can submit a v2
> using these functions instead.

 [[Evan Lloyd]] This is not my call but, as Leif asked me to stick my oar in, I'll say that I think it would be very acceptable.
Of course, the guy to convince is Liming.

Thank you, again.  (Are you getting the idea I'm pleased with this?)
Evan

>
> > > -----Original Message-----
> > > From: Chris Co
> > > Sent: Wednesday, June 27, 2018 11:13 AM
> > > To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; edk2-devel@lists.01.org;
> > > Liming Gao <liming.gao@intel.com>
> > > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar
> > > <Sami.Mujawar@arm.com>
> > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths
> > > to fix comparison
> > >
> > > Hi Liming, Evan,
> > >
> > > I have attached an example of a generated DSDT.iii file which gets
> > > processed using Trim.  It will give us a concrete example of what
> > > these
> > changes do.
> > >
> > > The first line has its #line directive stripped and the filepath
> > > saved as the PreProcessedFile.  In GCC49, the filepath was:
> > >
> >
> "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC4
> > >
> >
> 9xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\
> > > AcpiTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
> > >
> > > while in GCC5+ it is:
> > >
> >
> "e:\\rebase\\build\\hummingboard_edge_imx6q_2gb\\release_gcc5\\arm\
> > >
> >
> \platform\\solidrun\\hummingboard_edge_imx6q_2gb\\acpitables\\acpitab
> > > les\\output\\dsdt.i"
> > >
> > > So there is a difference in both lowercase as well as \\.\\ before dsdt.i.
> > >
> > > As the Trim function goes through each line, it strips the #line
> > > directive and saves the filepath as InjectedFile, which is then
> > > compared against PreProcessedFile
> > >
> >
> (https://github.com/tianocore/edk2/blob/975478f6bb22668efae311eb3f740
> > > 6e1f18411c2/BaseTools/Source/Python/Trim/Trim.py#L174)
> > > If it is not a match, skip to next line.  When using GCC5+,
> > > DSDT.iii's #line directives after the first keeps the old GCC49
> > > filepath syntax so the string comparison always fails.  An example
> > > of a line which should match line 602 in DSDT.iii.
> > >
> > > So my changes target these two cases directly, but admittedly I'm
> > > more of a kernel dev than a python dev so if there is a better way
> > > to address this, I'm all for it.
> > >
> > > > -----Original Message-----
> > > > From: Evan Lloyd <Evan.Lloyd@arm.com>
> > > > Sent: Wednesday, June 27, 2018 4:35 AM
> > > > To: Chris Co <Christopher.Co@microsoft.com>;
> > > > edk2-devel@lists.01.org
> > > > Cc: Liming Gao <liming.gao@intel.com>; Leif Lindholm
> > > > <leif.lindholm@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>
> > > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths
> > > > to fix comparison
> > > >
> > > > Hi Chris.
> > > > Firstly, thank you: this is a useful, pragmatic solution to an
> > > > major
> > > annoyance.
> > > > I personally think it unfortunate that the GCC guys can't be
> > > > bothered to fix their compiler, but ...
> > > >
> > > > > -----Original Message-----
> > > > > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
> > > > > Chris Co
> > > > > Sent: 27 June 2018 04:58
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize
> > > > > filepaths to fix comparison
> > > > >
> > > > > When using Linaro GCC5+ arm-eabi toolchain on Windows, the
> > > > > generated DSDT.iii contains a canonicalized ("\.\" removed and
> > > > > lower case) filepath for the preprocessed DSDT.i file in the first line.
> > > > > Due to this, when Trim.exe is called to generate DSDT.iiii,
> > > > > future filepath comparisons against this canonicalized filepath,
> > > > > which should match, actually fail the comparison which results
> > > > > in an empty
> > > DSDT.iiii.
> > > > >
> > > > > Issue was first reported to Linaro here:
> > > > >
> > > >
> > >
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.
> > > > >
> > > >
> > >
> >
> linaro.org%2Fshow_bug.cgi%3Fid%3D2909&amp;data=02%7C01%7CChristop
> > > > her.C
> > > > >
> > > >
> > >
> >
> o%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988bf8
> 6
> > > > f141a
> > > > >
> > > >
> > >
> >
> f91ab2d7cd011db47%7C1%7C0%7C636656961074375810&amp;sdata=%2FvF
> v
> > > > maBD0ul
> > > > > z89CY%2BMaOpwvpTvK9%2FxXjeZeSiXUCYH0%3D&amp;reserved=0
> > > > > where the recommendation was to address the issue in Trim.exe.
> > > > >
> > > > > This patch canonicalizes and lower cases all file paths
> > > > > encountered during trim execution on preprocessed files.  Since
> > > > > file paths are standarized, the comparison succeeds for files
> > > > > that should match regardless of the presence of upper case or "\.\"
> > > > > characters in the file
> > > path.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> > > > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > > > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > ---
> > > > >  BaseTools/Source/Python/Trim/Trim.py | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/BaseTools/Source/Python/Trim/Trim.py
> > > > > b/BaseTools/Source/Python/Trim/Trim.py
> > > > > index a74075859148..cca4e5c9694a 100644
> > > > > --- a/BaseTools/Source/Python/Trim/Trim.py
> > > > > +++ b/BaseTools/Source/Python/Trim/Trim.py
> > > > > @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target,
> > > > > ConvertHex,
> > > > > TrimLong):
> > > > >              if len(MatchList) == 2:
> > > > >                  LineNumber = int(MatchList[0], 0)
> > > > >                  InjectedFile = MatchList[1]
> > > > > +                InjectedFile = InjectedFile.replace("\\.\\","")
> > > >
> > > > [[Evan Lloyd]] I've not actually tried this yet, but it looks
> > > > surprizing.  I'd have expected InjectedFile.replace("\\.\\","\\").
> > > >           Can I ask how it works, please?  Have I missed something?
> > > > [[Evan Lloyd]] Would it be possible to achieve the same effect
> > > > with os.path.normpath (see Common/LongFilePathOsPath.py imported
> > > > via
> > line
> > > > 17)?
> > > >
> > > > > +                InjectedFile = InjectedFile.lower()
> > > > [[Evan Lloyd]] Similarly, there is "os.path.normcase" - but that
> > > > may not get the comparison working (because it converts '/' to '\'
> > > > on
> > Windows).
> > > >
> > > > >                  # The first injetcted file must be the preprocessed file itself
> > > > >                  if PreprocessedFile == "":
> > > > >                      PreprocessedFile = InjectedFile
> > > > > --
> > > > > 2.16.2.gvfs.1.33.gf5370f1
> > > > >
> > > > > _______________________________________________
> > > > > edk2-devel mailing list
> > > > > edk2-devel@lists.01.org
> > > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> > > > > Fl
> > > > > is
> > > > > ts
> > > > > .01.org%2Fmailman%2Flistinfo%2Fedk2-
> > > > devel&amp;data=02%7C01%7CChristoph
> > > > >
> > > >
> > >
> >
> er.Co%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988
> > > > bf86f
> > > > >
> > > >
> > >
> >
> 141af91ab2d7cd011db47%7C1%7C0%7C636656961074385815&amp;sdata=u
> u
> > > > S9M90lN
> > > > > ewzqS6ScNwbSm8xC9wpdzAFVZhjEu8w2kk%3D&amp;reserved=0
> > > > IMPORTANT NOTICE: The contents of this email and any attachments
> > > > are confidential and may also be privileged. If you are not the
> > > > intended recipient, please notify the sender immediately and do
> > > > not disclose the contents to any other person, use it for any
> > > > purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison

[Gao, Liming]

I understand this issue now. I agree this enhancement to compare the full path.

Thanks
Liming
> -----Original Message-----
> From: Evan Lloyd [mailto:Evan.Lloyd@arm.com]
> Sent: Thursday, June 28, 2018 6:35 PM
> To: Chris Co <Christopher.Co@microsoft.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>
> Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison
> 
> Hi Chris.
> 
> > -----Original Message-----
> > From: Chris Co <Christopher.Co@microsoft.com>
> > Sent: 27 June 2018 21:03
> > To: Evan Lloyd <Evan.Lloyd@arm.com>; edk2-devel@lists.01.org; Liming Gao
> > <liming.gao@intel.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar
> > <Sami.Mujawar@arm.com>
> > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix
> > comparison
> >
> > > -----Original Message-----
> > > From: Chris Co
> > > Sent: Wednesday, June 27, 2018 11:23 AM
> > > To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; 'edk2-devel@lists.01.org'
> > > <edk2- devel@lists.01.org>; 'Liming Gao' <liming.gao@intel.com>
> > > Cc: 'Leif Lindholm' <leif.lindholm@linaro.org>; Sami Mujawar
> > > <Sami.Mujawar@arm.com>
> > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to
> > > fix comparison
> > >
> > > Forgot to add that I'll try out the os.path.normpath/normcase to see
> > > if this will work.
> > >
> >
> > Just tried out os.path.normpath and os.path.normcase and together, these
> > calls do achieve the desired result.
> >
> > Initial filepath string =
> > "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC49
> > xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\Ac
> > piTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
> > After normpath =
> > "e:\rebase\Build\HUMMINGBOARD_EDGE_IMX6Q_2GB\RELEASE_GCC49xAS
> > L\ARM\Platform\SolidRun\HUMMINGBOARD_EDGE_IMX6Q_2GB\AcpiTables
> > \AcpiTables\OUTPUT\DSDT.i"
> > After normcase =
> > "e:\rebase\build\hummingboard_edge_imx6q_2gb\release_gcc49xasl\arm\
> > platform\solidrun\hummingboard_edge_imx6q_2gb\acpitables\acpitables\o
> > utput\dsdt.i"
> >
> > I would prefer to use the os module instead of my direct string manipulation
> > approach so let me know if this change is acceptable and I can submit a v2
> > using these functions instead.
> 
>  [[Evan Lloyd]] This is not my call but, as Leif asked me to stick my oar in, I'll say that I think it would be very acceptable.
> Of course, the guy to convince is Liming.
> 
> Thank you, again.  (Are you getting the idea I'm pleased with this?)
> Evan
> 
> >
> > > > -----Original Message-----
> > > > From: Chris Co
> > > > Sent: Wednesday, June 27, 2018 11:13 AM
> > > > To: 'Evan Lloyd' <Evan.Lloyd@arm.com>; edk2-devel@lists.01.org;
> > > > Liming Gao <liming.gao@intel.com>
> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Sami Mujawar
> > > > <Sami.Mujawar@arm.com>
> > > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths
> > > > to fix comparison
> > > >
> > > > Hi Liming, Evan,
> > > >
> > > > I have attached an example of a generated DSDT.iii file which gets
> > > > processed using Trim.  It will give us a concrete example of what
> > > > these
> > > changes do.
> > > >
> > > > The first line has its #line directive stripped and the filepath
> > > > saved as the PreProcessedFile.  In GCC49, the filepath was:
> > > >
> > >
> > "e:\\rebase\\Build\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\RELEASE_GCC4
> > > >
> > >
> > 9xASL\\ARM\\Platform\\SolidRun\\HUMMINGBOARD_EDGE_IMX6Q_2GB\\
> > > > AcpiTables\\AcpiTables\\OUTPUT\\.\\DSDT.i"
> > > >
> > > > while in GCC5+ it is:
> > > >
> > >
> > "e:\\rebase\\build\\hummingboard_edge_imx6q_2gb\\release_gcc5\\arm\
> > > >
> > >
> > \platform\\solidrun\\hummingboard_edge_imx6q_2gb\\acpitables\\acpitab
> > > > les\\output\\dsdt.i"
> > > >
> > > > So there is a difference in both lowercase as well as \\.\\ before dsdt.i.
> > > >
> > > > As the Trim function goes through each line, it strips the #line
> > > > directive and saves the filepath as InjectedFile, which is then
> > > > compared against PreProcessedFile
> > > >
> > >
> > (https://github.com/tianocore/edk2/blob/975478f6bb22668efae311eb3f740
> > > > 6e1f18411c2/BaseTools/Source/Python/Trim/Trim.py#L174)
> > > > If it is not a match, skip to next line.  When using GCC5+,
> > > > DSDT.iii's #line directives after the first keeps the old GCC49
> > > > filepath syntax so the string comparison always fails.  An example
> > > > of a line which should match line 602 in DSDT.iii.
> > > >
> > > > So my changes target these two cases directly, but admittedly I'm
> > > > more of a kernel dev than a python dev so if there is a better way
> > > > to address this, I'm all for it.
> > > >
> > > > > -----Original Message-----
> > > > > From: Evan Lloyd <Evan.Lloyd@arm.com>
> > > > > Sent: Wednesday, June 27, 2018 4:35 AM
> > > > > To: Chris Co <Christopher.Co@microsoft.com>;
> > > > > edk2-devel@lists.01.org
> > > > > Cc: Liming Gao <liming.gao@intel.com>; Leif Lindholm
> > > > > <leif.lindholm@linaro.org>; Sami Mujawar <Sami.Mujawar@arm.com>
> > > > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths
> > > > > to fix comparison
> > > > >
> > > > > Hi Chris.
> > > > > Firstly, thank you: this is a useful, pragmatic solution to an
> > > > > major
> > > > annoyance.
> > > > > I personally think it unfortunate that the GCC guys can't be
> > > > > bothered to fix their compiler, but ...
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
> > > > > > Chris Co
> > > > > > Sent: 27 June 2018 04:58
> > > > > > To: edk2-devel@lists.01.org
> > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > Subject: [edk2] [PATCH v1 1/1] BaseTools/Trim: Canonicalize
> > > > > > filepaths to fix comparison
> > > > > >
> > > > > > When using Linaro GCC5+ arm-eabi toolchain on Windows, the
> > > > > > generated DSDT.iii contains a canonicalized ("\.\" removed and
> > > > > > lower case) filepath for the preprocessed DSDT.i file in the first line.
> > > > > > Due to this, when Trim.exe is called to generate DSDT.iiii,
> > > > > > future filepath comparisons against this canonicalized filepath,
> > > > > > which should match, actually fail the comparison which results
> > > > > > in an empty
> > > > DSDT.iiii.
> > > > > >
> > > > > > Issue was first reported to Linaro here:
> > > > > >
> > > > >
> > > >
> > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.
> > > > > >
> > > > >
> > > >
> > >
> > linaro.org%2Fshow_bug.cgi%3Fid%3D2909&amp;data=02%7C01%7CChristop
> > > > > her.C
> > > > > >
> > > > >
> > > >
> > >
> > o%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988bf8
> > 6
> > > > > f141a
> > > > > >
> > > > >
> > > >
> > >
> > f91ab2d7cd011db47%7C1%7C0%7C636656961074375810&amp;sdata=%2FvF
> > v
> > > > > maBD0ul
> > > > > > z89CY%2BMaOpwvpTvK9%2FxXjeZeSiXUCYH0%3D&amp;reserved=0
> > > > > > where the recommendation was to address the issue in Trim.exe.
> > > > > >
> > > > > > This patch canonicalizes and lower cases all file paths
> > > > > > encountered during trim execution on preprocessed files.  Since
> > > > > > file paths are standarized, the comparison succeeds for files
> > > > > > that should match regardless of the presence of upper case or "\.\"
> > > > > > characters in the file
> > > > path.
> > > > > >
> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> > > > > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > > > > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > ---
> > > > > >  BaseTools/Source/Python/Trim/Trim.py | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/BaseTools/Source/Python/Trim/Trim.py
> > > > > > b/BaseTools/Source/Python/Trim/Trim.py
> > > > > > index a74075859148..cca4e5c9694a 100644
> > > > > > --- a/BaseTools/Source/Python/Trim/Trim.py
> > > > > > +++ b/BaseTools/Source/Python/Trim/Trim.py
> > > > > > @@ -166,6 +166,8 @@ def TrimPreprocessedFile(Source, Target,
> > > > > > ConvertHex,
> > > > > > TrimLong):
> > > > > >              if len(MatchList) == 2:
> > > > > >                  LineNumber = int(MatchList[0], 0)
> > > > > >                  InjectedFile = MatchList[1]
> > > > > > +                InjectedFile = InjectedFile.replace("\\.\\","")
> > > > >
> > > > > [[Evan Lloyd]] I've not actually tried this yet, but it looks
> > > > > surprizing.  I'd have expected InjectedFile.replace("\\.\\","\\").
> > > > >           Can I ask how it works, please?  Have I missed something?
> > > > > [[Evan Lloyd]] Would it be possible to achieve the same effect
> > > > > with os.path.normpath (see Common/LongFilePathOsPath.py imported
> > > > > via
> > > line
> > > > > 17)?
> > > > >
> > > > > > +                InjectedFile = InjectedFile.lower()
> > > > > [[Evan Lloyd]] Similarly, there is "os.path.normcase" - but that
> > > > > may not get the comparison working (because it converts '/' to '\'
> > > > > on
> > > Windows).
> > > > >
> > > > > >                  # The first injetcted file must be the preprocessed file itself
> > > > > >                  if PreprocessedFile == "":
> > > > > >                      PreprocessedFile = InjectedFile
> > > > > > --
> > > > > > 2.16.2.gvfs.1.33.gf5370f1
> > > > > >
> > > > > > _______________________________________________
> > > > > > edk2-devel mailing list
> > > > > > edk2-devel@lists.01.org
> > > > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> > > > > > Fl
> > > > > > is
> > > > > > ts
> > > > > > .01.org%2Fmailman%2Flistinfo%2Fedk2-
> > > > > devel&amp;data=02%7C01%7CChristoph
> > > > > >
> > > > >
> > > >
> > >
> > er.Co%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988
> > > > > bf86f
> > > > > >
> > > > >
> > > >
> > >
> > 141af91ab2d7cd011db47%7C1%7C0%7C636656961074385815&amp;sdata=u
> > u
> > > > > S9M90lN
> > > > > > ewzqS6ScNwbSm8xC9wpdzAFVZhjEu8w2kk%3D&amp;reserved=0
> > > > > IMPORTANT NOTICE: The contents of this email and any attachments
> > > > > are confidential and may also be privileged. If you are not the
> > > > > intended recipient, please notify the sender immediately and do
> > > > > not disclose the contents to any other person, use it for any
> > > > > purpose, or store or copy the information in any medium. Thank you.
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the
> intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose,
> or store or copy the information in any medium. Thank you.