From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 81D19202E53EF for ; Thu, 28 Jun 2018 06:40:21 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Jun 2018 06:40:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,283,1526367600"; d="scan'208";a="50601667" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga007.fm.intel.com with ESMTP; 28 Jun 2018 06:40:17 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 28 Jun 2018 06:40:17 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.87]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.82]) with mapi id 14.03.0319.002; Thu, 28 Jun 2018 21:40:15 +0800 From: "Gao, Liming" To: Evan Lloyd , Chris Co , "edk2-devel@lists.01.org" Thread-Topic: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison Thread-Index: AQHUDcsUCkcD+VT21EyYWFpJrbhBQqRzdBEAgABvUgCAAAK5gIAAG+6AgADzo4CAALhj8A== Date: Thu, 28 Jun 2018 13:40:14 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E29F8A2@SHSMSX104.ccr.corp.intel.com> References: <20180627035753.49848-1-christopher.co@microsoft.com> <20180627035753.49848-2-christopher.co@microsoft.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGJmMDlkYmUtYTM1NS00ZWY5LTllYzMtNmI3NzYyYjhlZWU5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiRTVuYmJGXC91dW0reHE4VzN2UGRLclhkQ1wvR3lxOXc1UFwvOXJSRWZkZ1NrWTFtdGxFRWJNZkh2alpWMnk4cFo1ZSJ9 dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix comparison X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jun 2018 13:40:21 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I understand this issue now. I agree this enhancement to compare the full p= ath. Thanks Liming > -----Original Message----- > From: Evan Lloyd [mailto:Evan.Lloyd@arm.com] > Sent: Thursday, June 28, 2018 6:35 PM > To: Chris Co ; edk2-devel@lists.01.org; Gao= , Liming > Cc: Leif Lindholm ; Sami Mujawar > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to fix= comparison >=20 > Hi Chris. >=20 > > -----Original Message----- > > From: Chris Co > > Sent: 27 June 2018 21:03 > > To: Evan Lloyd ; edk2-devel@lists.01.org; Liming Ga= o > > > > Cc: Leif Lindholm ; Sami Mujawar > > > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepaths to f= ix > > comparison > > > > > -----Original Message----- > > > From: Chris Co > > > Sent: Wednesday, June 27, 2018 11:23 AM > > > To: 'Evan Lloyd' ; 'edk2-devel@lists.01.org' > > > ; 'Liming Gao' > > > Cc: 'Leif Lindholm' ; Sami Mujawar > > > > > > 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, thes= e > > calls do achieve the desired result. > > > > Initial filepath string =3D > > "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 =3D > > "e:\rebase\Build\HUMMINGBOARD_EDGE_IMX6Q_2GB\RELEASE_GCC49xAS > > L\ARM\Platform\SolidRun\HUMMINGBOARD_EDGE_IMX6Q_2GB\AcpiTables > > \AcpiTables\OUTPUT\DSDT.i" > > After normcase =3D > > "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 manipul= ation > > approach so let me know if this change is acceptable and I can submit a= v2 > > using these functions instead. >=20 > [[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. >=20 > Thank you, again. (Are you getting the idea I'm pleased with this?) > Evan >=20 > > > > > > -----Original Message----- > > > > From: Chris Co > > > > Sent: Wednesday, June 27, 2018 11:13 AM > > > > To: 'Evan Lloyd' ; edk2-devel@lists.01.org; > > > > Liming Gao > > > > Cc: Leif Lindholm ; Sami Mujawar > > > > > > > > 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 > > > > > Sent: Wednesday, June 27, 2018 4:35 AM > > > > > To: Chris Co ; > > > > > edk2-devel@lists.01.org > > > > > Cc: Liming Gao ; Leif Lindholm > > > > > ; Sami Mujawar > > > > > Subject: RE: [PATCH v1 1/1] BaseTools/Trim: Canonicalize filepath= s > > > > > 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 On Behalf Of > > > > > > Chris Co > > > > > > Sent: 27 June 2018 04:58 > > > > > > To: edk2-devel@lists.01.org > > > > > > Cc: Liming Gao > > > > > > 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 fi= rst 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=3Dhttps%3A%2F%2Fbu= gs. > > > > > > > > > > > > > > > > > > > > linaro.org%2Fshow_bug.cgi%3Fid%3D2909&data=3D02%7C01%7CChristop > > > > > her.C > > > > > > > > > > > > > > > > > > > > o%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988bf8 > > 6 > > > > > f141a > > > > > > > > > > > > > > > > > > > > f91ab2d7cd011db47%7C1%7C0%7C636656961074375810&sdata=3D%2FvF > > v > > > > > maBD0ul > > > > > > z89CY%2BMaOpwvpTvK9%2FxXjeZeSiXUCYH0%3D&reserved=3D0 > > > > > > 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 > > > > > > Cc: Leif Lindholm > > > > > > Cc: Yonghong Zhu > > > > > > Cc: Liming Gao > > > > > > --- > > > > > > 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) =3D=3D 2: > > > > > > LineNumber =3D int(MatchList[0], 0) > > > > > > InjectedFile =3D MatchList[1] > > > > > > + InjectedFile =3D 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 somethin= g? > > > > > [[Evan Lloyd]] Would it be possible to achieve the same effect > > > > > with os.path.normpath (see Common/LongFilePathOsPath.py imported > > > > > via > > > line > > > > > 17)? > > > > > > > > > > > + InjectedFile =3D 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 preproc= essed file itself > > > > > > if PreprocessedFile =3D=3D "": > > > > > > PreprocessedFile =3D InjectedFile > > > > > > -- > > > > > > 2.16.2.gvfs.1.33.gf5370f1 > > > > > > > > > > > > _______________________________________________ > > > > > > edk2-devel mailing list > > > > > > edk2-devel@lists.01.org > > > > > > https://na01.safelinks.protection.outlook.com/?url=3Dhttps%3A%2= F%2 > > > > > > Fl > > > > > > is > > > > > > ts > > > > > > .01.org%2Fmailman%2Flistinfo%2Fedk2- > > > > > devel&data=3D02%7C01%7CChristoph > > > > > > > > > > > > > > > > > > > > er.Co%40microsoft.com%7C2abbef5e0163411f08e708d5dc220678%7C72f988 > > > > > bf86f > > > > > > > > > > > > > > > > > > > > 141af91ab2d7cd011db47%7C1%7C0%7C636656961074385815&sdata=3Du > > u > > > > > S9M90lN > > > > > > ewzqS6ScNwbSm8xC9wpdzAFVZhjEu8w2kk%3D&reserved=3D0 > > > > > 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 yo= u. > IMPORTANT NOTICE: The contents of this email and any attachments are conf= idential and may also be privileged. If you are not the > intended recipient, please notify the sender immediately and do not discl= ose the contents to any other person, use it for any purpose, > or store or copy the information in any medium. Thank you.