(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/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
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
