On 9 June 2015 at 19:37, Laszlo Ersek <ler...@redhat.com> wrote: > (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_., ]+]" >
Nice find! Very useful indeed. -- Ard. > 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/DebugAgentSymbolsBaseLib.inf >> >> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.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.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/ArmTrustedMonitorLibNull.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
