On 06/10/15 16:34, Kirkendall, Garrett wrote: > On my internal projects, I use for example *.inc.dsc. This allows > any special filetype parsing in my tools to recognize it for its > intended use, and allows me to easily tell visually that it is > intended to be included in another file of the same final extension > type.
I'm not trying to dictate what suffixes the include files should have. My only point is that *whatever* the suffixes are, those should be listed in .git/info/attributes, with diff=FOOBAR, and then the .git/config file should assign "diff.FOOBAR.xfuncname" such that the ini format section names show up in the patches. I don't really care: - about the particular suffixes - the actual value of FOOBAR - the actual regular expression used in xfuncname as long as the patches I have to look at carry the section names in the hunk headers. Note that the two files mentioned above, ".git/info/attributes" and ".git/config" are private to everyone's own git clone. *If* we wanted to store the attribute assignment inside the tree itself (ie. in a ".gitattributes" file right under the project root dir), *then* we'd have to agree on the (common subset of) suffixes, and the actual value of FOOBAR. (The git config, that is, the diff.FOOBAR.xfuncname regular expression, would remain private nevertheless.) If we don't insist on centralizing the attributes (and I don't), then we don't have to agree about the suffixes and the actual FOOBAR value. It's also possible to place only the following in the central (ie. tracked) .gitattributes file: *.dec diff=ini *.dsc diff=ini *.fdf diff=ini *.inf diff=ini And the less standard extensions (dsc.inc, fdf.inc, inc.dsc) can be specified privately *in addition*, in ".git/info/attributes". Thanks Laszlo > GARRETT KIRKENDALL > SMTS Firmware Engineer | CTE > 7171 Southwest Parkway, Austin, TX 78735 USA > facebook | amd.com > > -----Original Message----- > From: Gao, Liming [mailto:liming....@intel.com] > Sent: Wednesday, June 10, 2015 9:24 AM > To: edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] attention git users: a productivity boost for edk2 > > BKM: Best known method. > > I mean .dsc.inc is not standard file postfix. Other people may not use it. > Our internal project stills use *.dsc file as the subset of DSC to be > included in the full DSC file. > > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, June 10, 2015 7:32 PM > To: edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] attention git users: a productivity boost for edk2 > > On 06/10/15 03:34, Gao, Liming wrote: >> Good BKM sharing. > > What does BKM mean? > >> EDKII meta data file has .inf, .dec, *.dsc and *.fdf. What's *.dsc.inc >> and *.fdf.inc? They are the part of DSC and FDF files that included in >> the full DSC and FDF files? > > Yes. We have a few examples for them: > > - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc > - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc > - ArmVirtPkg/ArmVirt.dsc.inc > - OvmfPkg/OvmfPkg.fdf.inc > - OvmfPkg/VarStore.fdf.inc > > They are included by DSC and FDF files with the !include directive. > > Hm, actually, I just grepped for !include per se, and the suffixes of include > files are not very consistent. Beyond those listed above, we have: > > AppPkg/AppPkg.dsc:!include StdLib/StdLib.inc AppPkg/AppPkg.dsc:!include > AppPkg/Applications/Sockets/Sockets.inc > StdLib/StdLib.dsc:!include StdLib/StdLib.inc > > ie. the full suffix is just .inc. Then, > > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc: !include > $(PLATFORM_PACKAGE)/PlatformPkgConfig.dsc > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc: !include > $(PLATFORM_PACKAGE)/PlatformPkgConfig.dsc > Vlv2TbltDevicePkg/PlatformPkgX64.dsc: !include > $(PLATFORM_PACKAGE)/PlatformPkgConfig.dsc > > ie. just .dsc. Finally, > > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc: !include > $(PLATFORM_PACKAGE)/AutoPlatformCFG.txt > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc: !include > $(PLATFORM_PACKAGE)/AutoPlatformCFG.txt > Vlv2TbltDevicePkg/PlatformPkgX64.dsc: !include > $(PLATFORM_PACKAGE)/AutoPlatformCFG.txt > > ... I think choosing .txt for DSC include files was a really bad choice. > > In any case, I'm adding *.inc to my .git/info/attributes file (which in turn > obviates *.dsc.inc and *.fdf.inc). > > ... Hm, no I'm not. For example, BeagleBoardPkg has a number of .inc files > that don't follow the INI format... Sigh. I guess I'll stick with my current > entries, and accept that they won't cover the AppPkg and StdLib DSC include > files. > > Thanks! > Laszlo > >> >> Thanks >> Liming >> -----Original Message----- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Wednesday, June 10, 2015 1:38 AM >> To: edk2-devel list >> Subject: [edk2] attention git users: a productivity boost for edk2 >> >> (Sorry about the sensationalist subject line, I don't have a degree in >> marketing :)) >> >> Edk2 uses a large number of text files with *sections*: >> >> [Section.LOL] >> >> I'm sure you've been annoyed quite a few times, while reviewing patches, >> that you couldn't immediately see the section that a hunk modified. (I know >> I have.) We used to have two solutions for this: >> >> - On the reviewer side, apply the patchset and review it patch-wise, >> against the full source code as context. >> >> - On the sender side, generate the patches with a larger context. >> Options are -U<n>, --inter-hunk-context=<lines>, and even >> --function-context. Unfortunately, these don't resolve the question of >> sections reliably (or they produce overkill output). >> >> Turns out git has a trick for this up its sleeve: see gitattributes(5). >> >> (1) Edit your .git/info/attributes file, adding the following lines: >> >> *.dec diff=ini >> *.dsc diff=ini >> *.dsc.inc diff=ini >> *.fdf diff=ini >> *.fdf.inc diff=ini >> *.inf diff=ini >> >> (2) in your .git/config, add >> >> [diff "ini"] >> xfuncname = "^\\[[A-Za-z0-9_., ]+]" >> >> I just set these in my edk2 clone, and tested them on my local branch >> where I had applied Ard's series >> >> [PATCH 0/7] ArmPkg/ArmVirtPkg: GIC revision detection >> >> for review. A good example is patch #4, "ArmPkg: copy ArmGicArchLib to >> ArmGicArchSecLib". Now I can immediately see, in the hunk headers (@@), what >> section each change belongs to. >> >> Here it is (git show output trimmed to relevant files): >> >>> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc >>> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc >>> index 0859bc3..6e6687c 100644 >>> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc >>> +++ b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc >>> @@ -138,6 +138,7 @@ [LibraryClasses.common.SEC] >>> >>> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf >>> >>> PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf >>> PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf >>> + ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf >>> >>> [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] >>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf >>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc >>> b/ArmPlatformPkg/ArmPlatformPkg.dsc >>> index be31025..14d82f6 100644 >>> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc >>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc >>> @@ -134,6 +134,8 @@ [LibraryClasses.common.SEC] >>> >>> DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbo >>> lsBaseLib.inf >>> >>> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/ >>> DefaultExceptionHandlerLib|D >>> efaultExceptionHandlerLibBase.inf >>> >>> + ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf >>> + >>> [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] >>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf >>> >>> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc >>> b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc >>> index 19de381..4b4867f 100644 >>> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc >>> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc >>> @@ -128,6 +128,8 @@ [LibraryClasses.common.SEC] >>> >>> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/ >>> PrePiHobListPointerLib|P >>> rePiHobListPointerLib.inf >>> !endif >>> >>> + ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf >>> + >>> [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] >>> MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf >>> >>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>> index 84e2a99..8f7b5f1 100644 >>> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>> @@ -144,6 +144,8 @@ [LibraryClasses.common.SEC] >>> # Trustzone Support >>> >>> ArmTrustedMonitorLib|ArmPlatformPkg/Library/ArmTrustedMonitorLibNull/ >>> ArmTrustedMonitorLib|A >>> rmTrustedMonitorLibNull.inf >>> >>> + ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf >>> + >>> [LibraryClasses.common.PEI_CORE] >>> HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf >>> PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf >> >> This git-diff output shows quickly that Ard resolved the library class for >> SEC modules, a fact that was not visible otherwise from the patch itself. >> >> I recommend that all git users working with edk2 apply these settings -- >> they are *very* helpful for reviewers. Personally, I will make this a >> requirement for patches that I'm expected (or asked) to review. >> >> Thanks >> Laszlo >> >> ---------------------------------------------------------------------- >> -------- _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/edk2-devel >> >> ---------------------------------------------------------------------- >> -------- _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/edk2-devel >> > > > ------------------------------------------------------------------------------ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > ------------------------------------------------------------------------------ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > ------------------------------------------------------------------------------ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
