Hi Laszlo, On 11 March 2018 at 01:48, Laszlo Ersek <[email protected]> wrote: > Repo: https://github.com/lersek/edk2.git > Branch: hdr_inf_cleanup > > In > <http://mid.mail-archive.com/E92EE9817A31E24EB0585FDF735412F56327F7D3@ORSMSX113.amr.corp.intel.com>, > Mike explained why it's a good idea to list module-internal *.h files in > the [Sources*] sections of the INF files: > > On 11/23/15 21:28, Kinney, Michael D wrote: >> There are 2 reasons to list all source files used in a module build in >> the [Sources] section. >> >> 1) Support incremental builds. If a change to the .h file is made, >> then the module may not be rebuilt if the .h file is not listed in >> [Sources] >> 2) Support of UEFI Distribution Package distribution format. The UPT >> tools that creates UDP packages uses the [Sources] section for the >> inventory of files. If a file is missing, then it will not be >> included in the UDP file. > > In only two years and three-four months, I've finally come around > addressing (1) under ArmVirtPkg and OvmfPkg.
Thanks for doing this. However, while I highly appreciate your thoroughness and verbosity in most cases, I do think you've crossed a line this time :-) Do we *really* need 4 different patches for CsmSupportLib, each adding a single .h file to [Sources], with an elaborate description how it is being used? If it is used, it needs to be listed, and if it is not, it needs to be removed, that's all there is to it IMO. Apart from that, the series looks correct to me. Thanks, Ard. > The affected *.inf and *.h > files were located with the following crude script: > >> #!/bin/bash >> >> export LC_ALL=C >> >> find ArmVirtPkg/ OvmfPkg/ -type f -name '*.inf' \ >> | sort \ >> | while read INF; do >> INF_D=$(dirname -- "$INF") >> INF_F=$(basename -- "$INF") >> ( >> cd "$INF_D" >> find -type f -name '*.h' \ >> | cut -c 3- \ >> | sort \ >> | while read REL_H; do >> if ! grep -q -F -e " $REL_H" -- "$INF_F"; then >> printf '%s: %s\n' "$INF" "$REL_H" >> fi >> done >> ) >> done > > This patch set brings the output down to nil, from the following 45 > lines (note that the patch count equaling 45 is a coincidence): > >> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf: >> PlatformBm.h >> ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf: PrePi.h >> OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf: AcpiPlatform.h >> OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf: QemuLoader.h >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf: AcpiPlatform.h >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf: QemuLoader.h >> OvmfPkg/BlockMmioToBlockIoDxe/BlockIo.inf: BlockIo.h >> OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf: CsmSupportLib.h >> OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf: LegacyInterrupt.h >> OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf: LegacyPlatform.h >> OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf: LegacyRegion.h >> OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf: Fvb.h >> OvmfPkg/IoMmuDxe/IoMmuDxe.inf: AmdSevIoMmu.h >> OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf: AcpiTimerLib.h >> OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf: AcpiTimerLib.h >> OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf: AcpiTimerLib.h >> OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf: >> X64/VirtualMemory.h >> OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf: LoadLinuxLib.h >> OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf: LockBoxLib.h >> OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf: LockBoxLib.h >> OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf: NvVarsFileLib.h >> OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf: >> DebugLibDetect.h >> OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf: >> DebugLibDetect.h >> OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf: ExtraRootBusMap.h >> OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf: >> SerializeVariablesLib.h >> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf: >> VirtioMmioDevice.h >> OvmfPkg/PlatformDxe/Platform.inf: Platform.h >> OvmfPkg/PlatformDxe/Platform.inf: PlatformConfig.h >> OvmfPkg/PlatformPei/PlatformPei.inf: Cmos.h >> OvmfPkg/PlatformPei/PlatformPei.inf: Platform.h >> OvmfPkg/PlatformPei/PlatformPei.inf: Xen.h >> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf: >> FwBlockService.h >> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf: QemuFlash.h >> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf: FwBlockService.h >> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf: QemuFlash.h >> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf: Qemu.h >> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf: UnalignedIoInternal.h >> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf: VbeShim.h >> OvmfPkg/Virtio10Dxe/Virtio10.inf: Virtio10.h >> OvmfPkg/VirtioBlkDxe/VirtioBlk.inf: VirtioBlk.h >> OvmfPkg/VirtioNetDxe/VirtioNet.inf: VirtioNet.h >> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf: VirtioPciDevice.h >> OvmfPkg/VirtioRngDxe/VirtioRng.inf: VirtioRng.h >> OvmfPkg/VirtioScsiDxe/VirtioScsi.inf: VirtioScsi.h >> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf: DriverBinding.h > > In the future, we shall ask for patches to be respun unless they (a) > keep the [Sources*] sections sorted and (b) list any new module-internal > header files there. > > Cc: Anthony Perard <[email protected]> > Cc: Ard Biesheuvel <[email protected]> > Cc: Brijesh Singh <[email protected]> > Cc: Jordan Justen <[email protected]> > Cc: Julien Grall <[email protected]> > Cc: Phil Dennis-Jordan <[email protected]> > > Thanks, > Laszlo > > Laszlo Ersek (45): > ArmVirtPkg/PlatformBootManagerLib: list "PlatformBm.h" in INF file > ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: sort [Sources*] sections in > INF > ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: list "PrePi.h" in INF file > OvmfPkg/AcpiPlatformDxe: sort [Sources*] sections in the INF files > OvmfPkg/AcpiPlatformDxe: list "AcpiPlatform.h" in the INF files > OvmfPkg/AcpiPlatformDxe: don't #include "QemuLoader.h" in "Qemu.c" > OvmfPkg/AcpiPlatformDxe: list "QemuLoader.h" in the INF files > OvmfPkg/BlockMmioToBlockIoDxe: list "BlockIo.h" in the INF file > OvmfPkg/CsmSupportLib: sort [Sources*] sections in the INF file > OvmfPkg/CsmSupportLib: list "CsmSupportLib.h" in the INF file > OvmfPkg/CsmSupportLib: list "LegacyInterrupt.h" in the INF file > OvmfPkg/CsmSupportLib: list "LegacyPlatform.h" in the INF file > OvmfPkg/CsmSupportLib: list "LegacyRegion.h" in the INF file > OvmfPkg/EmuVariableFvbRuntimeDxe: list "Fvb.h" in the INF file > OvmfPkg/IoMmuDxe: list "AmdSevIoMmu.h" in the INF file > OvmfPkg/AcpiTimerLib: list "AcpiTimerLib.h" in the INF files > OvmfPkg/BaseMemEncryptSevLib: list "X64/VirtualMemory.h" in the INF > file > OvmfPkg/LoadLinuxLib: list "LoadLinuxLib.h" in the INF file > OvmfPkg/LockBoxLib: list "LockBoxLib.h" in the INF files > OvmfPkg/NvVarsFileLib: list "NvVarsFileLib.h" in the INF file > OvmfPkg/PlatformDebugLibIoPort: list "DebugLibDetect.h" in the INF > files > OvmfPkg/QemuBootOrderLib: sort [Sources*] sections in the INF file > OvmfPkg/QemuBootOrderLib: list "ExtraRootBusMap.h" in the INF file > OvmfPkg/SerializeVariablesLib: list "SerializeVariablesLib.h" in INF > file > OvmfPkg/VirtioMmioDeviceLib: list "VirtioMmioDevice.h" in the INF file > OvmfPkg/VirtioMmioDeviceLib: improve style of > mMmioDeviceProtocolTemplate > OvmfPkg/PlatformDxe: list "Platform.h" in the INF file > OvmfPkg/PlatformDxe: list "PlatformConfig.h" in the INF file > OvmfPkg/PlatformPei: list "Cmos.h" in the INF file > OvmfPkg/PlatformPei: list "Platform.h" in the INF file > OvmfPkg/PlatformPei: list "Xen.h" in the INF file > OvmfPkg/QemuFlashFvbServicesRuntimeDxe: list "FwBlockService.h" in > INFs > OvmfPkg/QemuFlashFvbServicesRuntimeDxe: list "QemuFlash.h" in INF > files > OvmfPkg/QemuVideoDxe: sort [Sources*] sections in the INF file > OvmfPkg/QemuVideoDxe: list "Qemu.h" in the INF file > OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file > OvmfPkg/QemuVideoDxe: list "VbeShim.h" in the INF file > OvmfPkg/Virtio10Dxe: list "Virtio10.h" in the INF file > OvmfPkg/VirtioBlkDxe: list "VirtioBlk.h" in the INF file > OvmfPkg/VirtioNetDxe: list "VirtioNet.h" in the INF file > OvmfPkg/VirtioPciDeviceDxe: list "VirtioPciDevice.h" in the INF file > OvmfPkg/VirtioRngDxe: list "VirtioRng.h" in the INF file > OvmfPkg/VirtioScsiDxe: list "VirtioScsi.h" in the INF file > OvmfPkg/XenPvBlkDxe: sort [Sources*] sections in the INF file > OvmfPkg/XenPvBlkDxe: list "DriverBinding.h" in the INF file > > ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 + > ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 3 ++- > OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 8 > +++++--- > OvmfPkg/AcpiPlatformDxe/Qemu.c | 1 - > OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf | 8 > +++++--- > OvmfPkg/BlockMmioToBlockIoDxe/BlockIo.inf | 1 + > OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf | 6 > +++++- > OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf | 1 + > OvmfPkg/IoMmuDxe/IoMmuDxe.inf | 1 + > OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf | 1 + > OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf | 1 + > OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf | 1 + > OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf | 1 + > OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf | 1 + > OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf | 1 + > OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf | 1 + > OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf | 1 + > OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf | 1 + > OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf | 1 + > OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf | 3 ++- > OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf | 1 + > OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.c | 2 +- > OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf | 1 + > OvmfPkg/PlatformDxe/Platform.inf | 2 ++ > OvmfPkg/PlatformPei/PlatformPei.inf | 3 +++ > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf | 2 ++ > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf | 2 ++ > OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 7 > +++++-- > OvmfPkg/Virtio10Dxe/Virtio10.inf | 1 + > OvmfPkg/VirtioBlkDxe/VirtioBlk.inf | 1 + > OvmfPkg/VirtioNetDxe/VirtioNet.inf | 1 + > OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf | 1 + > OvmfPkg/VirtioRngDxe/VirtioRng.inf | 1 + > OvmfPkg/VirtioScsiDxe/VirtioScsi.inf | 1 + > OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf | 9 > +++++---- > 35 files changed, 61 insertions(+), 17 deletions(-) > > -- > 2.14.1.3.gb7cf6e02401b > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
