From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by ml01.01.org (Postfix) with ESMTP id 6D1431A1E12 for ; Mon, 1 Aug 2016 09:56:16 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 01 Aug 2016 09:56:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,456,1464678000"; d="scan'208";a="1017684148" Received: from orsmsx108.amr.corp.intel.com ([10.22.240.6]) by fmsmga001.fm.intel.com with ESMTP; 01 Aug 2016 09:56:16 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.118]) by ORSMSX108.amr.corp.intel.com ([169.254.2.61]) with mapi id 14.03.0248.002; Mon, 1 Aug 2016 09:56:15 -0700 From: "Kinney, Michael D" To: Leif Lindholm , "Justen, Jordan L" , "Kinney, Michael D" CC: Tim Lewis , Laszlo Ersek , "edk2-devel@lists.01.org" , Andrew Fish Thread-Topic: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex Thread-Index: AQHR2FtRU6xb1Fcsl0q8tneXlsikH6ANhf+AgCKvmoCAAAfmgIABqO+AgAHcR4D//5iD4IAA7usAgAAVUICAABotsA== Date: Mon, 1 Aug 2016 16:56:15 +0000 Message-ID: References: <1467901459-18840-1-git-send-email-leif.lindholm@linaro.org> <20160729164433.GO31760@bivouac.eciton.net> <7236196A5DF6C040855A6D96F556A53F3D487A@msmail.insydesw.com.tw> <20160730183343.GP31760@bivouac.eciton.net> <147000590377.19855.16400196489651537046@jljusten-ivb> <147003498665.24223.12312501403228549999@jljusten-ivb> <20160801081922.GQ31760@bivouac.eciton.net> In-Reply-To: <20160801081922.GQ31760@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMWNjOGZkNDYtNTc4ZC00Mjg5LWI2OTEtYzNjZjdmOTIyZjk3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImFGNHlqS2VcL09WeWt4UTh5SUlmbHhCYXlCY1FLbDBsSnlWRTR5SWN0WDJVPSJ9 x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: [PATCH] add top-level .gitattributes file, dealing with .depex X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2016 16:56:16 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jordan, I agree that we should not be adding .efi binaries or .depex binary files to our source repos. There are several repos for EDK II, and some of those do accept binaries. Specifically, the edk2-non-osi and edk2-platform= s would be repos where .depex may occur. Possibly edk2-staging as well. I think it is easier to manage things like GIT attributes so they are the=20 same for all repos in the project. Mike > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: Monday, August 1, 2016 1:19 AM > To: Justen, Jordan L > Cc: Kinney, Michael D ; Tim Lewis ; > Laszlo Ersek ; edk2-devel@lists.01.org; Andrew Fish > > Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing wi= th .depex >=20 > On Mon, Aug 01, 2016 at 12:03:06AM -0700, Jordan Justen wrote: > > On 2016-07-31 16:52:23, Kinney, Michael D wrote: > > > Jordan, > > > > > > UEFI Drivers distributed as binaries do not need depex sections. > > > > > > PI modules distributed as binaries do require a .depex binary. > > > > > > > They may require a depex, but, as mentioned below, they can also add > > it directly in the .fdf. As it stands, apparently we have 1 .depex > > file in the tree, and it is unused. > > > > Aside from this, under what conditions would we take such binaries > > into the EDK II tree? Today we have the ShellPkg and FatPkg binaries > > in the EDK II tree, but we recently discussed removing even those. >=20 > While I don't disagree, the PI dependency expression instruction set > (section 10.7, PI spec 1.4 vol2) does not look Turing complete to me. > Meaning it's "binary" in much the same way a .uni file is. >=20 > (This is historically where someone pulls out an operating system > kernel written entirely in PI depex binary.) >=20 > > For an open source project, I think it is best to not have pre-built > > binaries, unless there is some very compelling reason. Previously > > there was some license funniness on FatPkg, but now that is gone. If > > it took an hour to build FatPkg, then that might also be something to > > discuss. :) > > > > I don't think adding the .gitattributes is really a problem, aside > > from the fact that it implies that we might actually have a reason to > > add a .depex file to the source tree. >=20 > And I agree it would send that signal. >=20 > Regards, >=20 > Leif >=20 > > -Jordan > > > > > So I would recommend .depex binary files be treated the same as > > > binary .efi files by GIT. So it does sound like we need some > > > minor updates to GIT attributes. > > > > > > If we have an example of a binary module that is providing more > > > binary leaf sections than are actually required and/or used, then > > > yes, the binary module should be cleaned up to remove the unused > > > content. > > > > > > Thanks, > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: Justen, Jordan L > > > > Sent: Sunday, July 31, 2016 3:58 PM > > > > To: Leif Lindholm ; Tim Lewis > > > > Cc: Laszlo Ersek ; Kinney, Michael D > ; > > > > edk2-devel@ml01.01.org ; Andrew Fish > > > > Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, deal= ing with > .depex > > > > > > > > On 2016-07-30 11:33:43, Leif Lindholm wrote: > > > > > Hi Tim, > > > > > > > > > > Thanks for the warning, and investigation. > > > > > > > > > > Does this mean that you think we should ban the inclusion of .dep= ex > > > > > files in EDK2, including future platform trees? > > > > > > > > I don't know about banning it, but at least we could wait for someo= ne > > > > to make a reasonable argument why they are needed. > > > > > > > > Even for binary only modules, it looks like the fdf method outlined > > > > below is preferable to a pre-built .depex. > > > > > > > > If (at a future point) the reason for using a .depex is to support = a > > > > binary only module in a supposedly open platform under EDK II, then= I > > > > guess we can decide if that is a good idea at that point. > > > > > > > > Should we delete this one unused .depex from the tree? > > > > > > > > -Jordan > > > > > > > > > (If not, this patch is > > > > > still needed for git to work predictably with these files.) > > > > > > > > > > Regards, > > > > > > > > > > Leif > > > > > > > > > > On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote: > > > > > > It appears that this file is not actually used. It is only > > > > > > referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule = in > > > > > > PlatformPkg.fdf. A little further research shows that an altern= ate > > > > > > method was used for the actual GOP binary (see below). A grep o= f the > > > > > > entire tree shows that no one uses this rule NATIVE_BINARY. So = it > > > > > > looks like it can just be cut out. > > > > > > > > > > > > BTW, the downside of the method used for the binary version of = the > > > > > > GOP driver, is that those drivers cannot use PCDs, since the PC= D > > > > > > database is created based on references in the .inf. GOP works > > > > > > because it is pure UEFI and (therefore) doesn't use PCDs. > > > > > > > > > > > > Tim > > > > > > > > > > > > FILE DRIVER =3D FF0C8745-3270-4439-B74F-3E45F8C77064 { > > > > > > SECTION DXE_DEPEX_EXP =3D {gPlatformGOPPolicyGuid} > > > > > > SECTION PE32 =3D > > > > > Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/In= telGopDriver.e > > > > fi > > > > > > SECTION UI =3D "IntelGopDriver" > > > > > > } > > > > > > > > > > > > -----Original Message----- > > > > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Be= half Of Leif > > > > Lindholm > > > > > > Sent: Friday, July 29, 2016 9:45 AM > > > > > > To: Laszlo Ersek > > > > > > Cc: michael.d.kinney@intel.com; Jordan Justen ; > edk2- > > > > devel@ml01.01.org; Andrew Fish > > > > > > Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, = dealing with > .depex > > > > > > > > > > > > On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote: > > > > > > > On 07/07/16 16:24, Leif Lindholm wrote: > > > > > > > > Git tends to see .depex files as text, causing hideous patc= hes being > > > > > > > > generated (and breaking PatchCheck.py). > > > > > > > > > > > > > > > > Add a .gitattributes file instructing git to treat them as = binary. > > > > > > > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > > > > > > Signed-off-by: Leif Lindholm > > > > > > > > --- > > > > > > > > .gitattributes | 1 + > > > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > create mode 100644 .gitattributes > > > > > > > > > > > > > > > > diff --git a/.gitattributes b/.gitattributes new file mode = 100644 > > > > > > > > index 0000000..2d8a45b > > > > > > > > --- /dev/null > > > > > > > > +++ b/.gitattributes > > > > > > > > @@ -0,0 +1 @@ > > > > > > > > +*.depex binary > > > > > > > > > > > > > > > > > > > > > > What generates .depex files? I've never seen any. > > > > > > > > > > > > > > Also, unless you add .depex files with "git add" to the set o= f tracked > > > > > > > files, no patches / diffs should cover them. What am I missin= g? :) > > > > > > > > > > > > > > ... Hm, after > > > > > > > > > > > > > > $ find . -iname "*.depex" > > > > > > > > > > > > > > I see .depex files in Build/ (which should be ignored altoget= her), and > > > > > > > > > > > > > > ./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex > > > > > > > > > > > > > > Why does that file exist in the tree? Let me see... git log s= ays nothing > relevant > > > > (the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed > Vlv2TbltDevicePkg and > > > > Vlv2DeviceRefCodePkg to"). > > > > > > > > > > > > > > Grepping the tree for the filename itself leads to: > > > > > > > > > > > > > > Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Opt= ional > > > > $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex > > > > > > > Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX = Optional > > > > $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex > > > > > > > > > > > > > > Do these rules exist to override the DEPEX sections of binary= -only modules? > If > > > > so: that's horrible. > > > > > > > > > > > > > > Anyway, given that edk2 contains at least one .depex file, an= d your patch > is > > > > correct according to Attributes>: > > > > > > > > > > > > > > Reviewed-by: Laszlo Ersek > > > > > > > > > > > > Thanks! > > > > > > > > > > > > I had hoped for comments from someone else on cc, since we don'= t have any > > > > Maintainers.txt entry for the top level directory :) > > > > > > > > > > > > But if I don't hear anything before Monday, I'll push it then. > > > > > > > > > > > > Regards, > > > > > > > > > > > > Leif > > > > > > > > > > > > _______________________________________________ > > > > > > edk2-devel mailing list > > > > > > edk2-devel@lists.01.org > > > > > > https://lists.01.org/mailman/listinfo/edk2-devel