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

[Chris Co <Christopher.Co@microsoft.com>]

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.