From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=HpRVnMbe; spf=pass (domain: linaro.org, ip: 209.85.128.67, mailfrom: leif.lindholm@linaro.org) Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by groups.io with SMTP; Wed, 19 Jun 2019 02:35:37 -0700 Received: by mail-wm1-f67.google.com with SMTP id 207so983256wma.1 for ; Wed, 19 Jun 2019 02:35:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=YNGuNGS0w4YHAvxPT5x0nXwH1D8pHhL88v51pRKT49A=; b=HpRVnMbeSseIVQOMZPhbVY1p1jbZZ8XI9xxxXHfVhTr1JIJd0Q9W+kPWUDwYdckmOY Wco6LG9mFxn1yt9Po1q86o8Y9wh+B5hlweQHSRw0Kkg8JPz5WjMRDGjDC2eDrVd8eBo6 azP/Fb/MmCKcC0Ik4yFQy6KoengX/qSMErOd2vYszdn1n4I8ytKz6rgV2yyC9Wg3ODO+ v4ax1WYlQFphZGVfmYWLDYJCKuuRzQxJ4wRMbWGpo1hfwI+EWZkP1qUHS+zktW3FlfPv g1MPta2HprgP/ZE4ugXCCYEp1+XIIMeE41WpWtcRtC9nAiKiH64RaSCkxX+TCJfeIa3u jgdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=YNGuNGS0w4YHAvxPT5x0nXwH1D8pHhL88v51pRKT49A=; b=dlbm9BgOjVcjRr0Al8O4sCjBvXUaormV2mRB+uvAZXbMfGy0szCn6cgVt1EROEmaba pOOfDVo5/GGhQervc+f6jHtyb59/1nAu1V2TYW7I+xoEnqbt67vcxtYDXZagh3wUQfbi 7q0bGbeAT7yzEgYL1THDyYGuDyhsrDYI8LrrueU6f9f/wQGGUZjQ7MvQAA/eToO+5mn3 c6dvrSewIL15FKufuCxwhKBuXZwRCwPcRXoF7n7bN4TAtFZyRccOBgBvNnBPtR/5n3pt jzGpr5SEriFaENnMtfGMTOgjoDstkkVvtPfk0Q7bQt3ic45UVhqaQqNZBtqp3HyLKauM QJeQ== X-Gm-Message-State: APjAAAX7MXvWzLHXkLX7ZqK9FfjV+vJqgcPpkxH+CQ2RyPN6ZWT9FZaW hysWzG+jo3DWmC8xiyeo2y5MAA== X-Google-Smtp-Source: APXvYqwpzKgt+8WrcCnQctMhsjM3KOO9byglHeGRJo+aVffQcqRdZ4s+0gxT7hJPUKfXz2uBXCXsrQ== X-Received: by 2002:a1c:4041:: with SMTP id n62mr7665243wma.100.1560936935903; Wed, 19 Jun 2019 02:35:35 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 32sm36076387wra.35.2019.06.19.02.35.34 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 19 Jun 2019 02:35:35 -0700 (PDT) Date: Wed, 19 Jun 2019 10:35:33 +0100 From: "Leif Lindholm" To: "Wu, Hao A" Cc: "devel@edk2.groups.io" , "lersek@redhat.com" , "Kinney, Michael D" , Andrew Fish , "Justen, Jordan L" , Ard Biesheuvel , "Gao, Liming" , "Yao, Jiewen" , "Zeng, Star" Subject: Re: [RFC] Fine-grained review ownership for MdeModulePkg Message-ID: <20190619093533.ueex63rrkovdhyz2@bivouac.eciton.net> References: <20190611095150.l7vmqyn3ln4gmvyq@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jun 19, 2019 at 05:09:40AM +0000, Wu, Hao A wrote: > Hello all, > > As suggested by Ray and Leif, modules (with wildcard) in MdeModulePkg are > classified to a list of features. > > Please note that: > * The below list is a draft at this moment, please help to provide > feedbacks/comments; > * Modules with no clear classification are listed under the 'Misc' section > at the bottom of the list. Thank you for doing the heavy lifting. A few comments below on how this could be more easily described in the Maintainer.txt syntax. > ACPI: > MdeModulePkg/Include/*/*Acpi*.h > MdeModulePkg/Universal/Acpi/ > > BDS: > MdeModulePkg/Include/Library/PlatformBootManagerLib.h > MdeModulePkg/Include/Library/UefiBootManagerLib.h > MdeModulePkg/Library/PlatformBootManagerLibNull/ > MdeModulePkg/Library/UefiBootManagerLib/ MdeModulePkg/*BootManagerLib*/ > MdeModulePkg/Universal/BdsDxe/ > MdeModulePkg/Universal/BootManagerPolicyDxe/ Or maybe even MdeModulePkg/*BootManager*/, which would also match the line above. > MdeModulePkg/Universal/LoadFileOnFv2/ > MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.* > > Console: > MdeModulePkg/Include/Guid/ConnectConInEvent.h > MdeModulePkg/Include/Guid/ConsoleInDevice.h > MdeModulePkg/Include/Guid/ConsoleOutDevice.h > MdeModulePkg/Include/Guid/StandardErrorDevice.h > MdeModulePkg/Include/Guid/TtyTerm.h > MdeModulePkg/Universal/Console/ConPlatformDxe/ > MdeModulePkg/Universal/Console/ConSplitterDxe/ > MdeModulePkg/Universal/Console/GraphicsConsoleDxe/ > MdeModulePkg/Universal/Console/TerminalDxe/ I was intrigued as to why this did not specify MdeModulePkg/Universal/Console/ See [1] below. However, even if suggestions included below were not implemented, the situation could be described as: F: MdeModulePkg/Universal/Console/ X: MdeModulePkg/Universal/Console/GraphicsOutputDxe/ > Core (PEI, DXE and Runtime): > MdeModulePkg/Core/Dxe/* > MdeModulePkg/Core/Dxe/Dispatcher/ > MdeModulePkg/Core/Dxe/DxeMain/ > MdeModulePkg/Core/Dxe/Event/ > MdeModulePkg/Core/Dxe/FwVol*/ > MdeModulePkg/Core/Dxe/Hand/ > MdeModulePkg/Core/Dxe/Image/ > MdeModulePkg/Core/Dxe/Library/ > MdeModulePkg/Core/Dxe/Misc/ > MdeModulePkg/Core/Dxe/SectionExtraction/ F: MdeModulePkg/Core/Dxe/ X: MdeModulePkg/Core/Dxe/Mem/ > MdeModulePkg/Core/DxeIplPeim/ > MdeModulePkg/Core/Pei/* > MdeModulePkg/Core/Pei/BootMode/ > MdeModulePkg/Core/Pei/CpuIo/ > MdeModulePkg/Core/Pei/Dependency/ > MdeModulePkg/Core/Pei/Dispatcher/ > MdeModulePkg/Core/Pei/FwVol/ > MdeModulePkg/Core/Pei/Hob/ > MdeModulePkg/Core/Pei/Image/ > MdeModulePkg/Core/Pei/PeiMain/ > MdeModulePkg/Core/Pei/Ppi/ > MdeModulePkg/Core/Pei/Security/ F: MdeModulePkg/Core/Pei/ X: MdeModulePkg/Core/Pei/Memory/ X: MdeModulePkg/Core/Pei/PciCfg2/ X: MdeModulePkg/Core/Pei/Reset/ X: MdeModulePkg/Core/Pei/StatusCode/ I'm going to stop there, because I'm lazy, and I realise this is about the responsibility areas rather than an actual patch to Maintainers.txt - and I think my point is made. Further comments below. > MdeModulePkg/Core/RuntimeDxe/ > MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h > MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h > MdeModulePkg/Include/Guid/IdleLoopEvent.h > MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h > MdeModulePkg/Include/Library/SecurityManagementLib.h > MdeModulePkg/Library/*SectionExtract*/ > MdeModulePkg/Library/DxeSecurityManagementLib/ > MdeModulePkg/Universal/PlatformDriOverrideDxe/ > MdeModulePkg/Universal/SectionExtraction*/ > MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c > > Debug: > MdeModulePkg/Include/Guid/DebugMask.h > MdeModulePkg/Include/Library/DebugAgentLib.h > MdeModulePkg/Include/Ppi/Debug.h > MdeModulePkg/Library/*Debug*/ > MdeModulePkg/Universal/Debug*/ > > Decompress: > MdeModulePkg/Include/Guid/LzmaDecompress.h > MdeModulePkg/Library/*Decompress*/ > > Device: > MdeModulePkg/Bus/Ata/ > MdeModulePkg/Bus/I2c/ > MdeModulePkg/Bus/Isa/ > MdeModulePkg/Bus/Pci/Ehci*/ > MdeModulePkg/Bus/Pci/IdeBusPei/ > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ > MdeModulePkg/Bus/Pci/NvmExpress*/ > MdeModulePkg/Bus/Pci/PciSioSerialDxe/ > MdeModulePkg/Bus/Pci/SataControllerDxe/ > MdeModulePkg/Bus/Pci/SdMmc*/ > MdeModulePkg/Bus/Pci/Ufs*/ > MdeModulePkg/Bus/Pci/Uhci*/ > MdeModulePkg/Bus/Pci/Xhci*/ > MdeModulePkg/Bus/Scsi/ > MdeModulePkg/Bus/Sd/ > MdeModulePkg/Bus/Ufs/ > MdeModulePkg/Bus/Usb/ > MdeModulePkg/Include/*/*Ata*.h > MdeModulePkg/Include/*/*NonDiscoverableDevice*.h > MdeModulePkg/Include/*/*NvmExpress*.h > MdeModulePkg/Include/*/*SerialPort*.h > MdeModulePkg/Include/*/*SdMmc*.h > MdeModulePkg/Include/*/*Ufs*.h > MdeModulePkg/Include/*/*Usb*.h > MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h > MdeModulePkg/Include/Guid/RecoveryDevice.h > MdeModulePkg/Include/Guid/UsbKeyBoardLayout.h > MdeModulePkg/Include/Ppi/StorageSecurityCommand.h > MdeModulePkg/Include/Protocol/Ps2Policy.h > MdeModulePkg/Library/BaseSerialPortLib16550/ > MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/ > MdeModulePkg/Universal/SerialDxe/ > > Disk: > MdeModulePkg/Universal/Disk/ > > EBC: > MdeModulePkg/Include/*/*Ebc*.h > MdeModulePkg/Include/Protocol/DebuggerConfiguration.h > MdeModulePkg/Universal/EbcDxe/ > > Firmware Update: > MdeModulePkg/Application/CapsuleApp/ > MdeModulePkg/Include/*/*Capsule*.h > MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h > MdeModulePkg/Include/Library/FmpAuthenticationLib.h > MdeModulePkg/Include/Protocol/EsrtManagement.h > MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h > MdeModulePkg/Library/DisplayUpdateProgressLib*/ > MdeModulePkg/Library/DxeCapsuleLib*/ > MdeModulePkg/Library/FmpAuthenticationLibNull/ > MdeModulePkg/Universal/Capsule*/ > MdeModulePkg/Universal/Esrt*/ > > Graphic: > MdeModulePkg/Include/*/*Logo*.h > MdeModulePkg/Include/Library/BmpSupportLib.h > MdeModulePkg/Include/Library/FrameBufferBltLib.h > MdeModulePkg/Library/BaseBmpSupportLib/ > MdeModulePkg/Library/BootLogoLib/ > MdeModulePkg/Library/FrameBufferBltLib/ > MdeModulePkg/Logo/ > MdeModulePkg/Universal/Console/GraphicsOutputDxe/ [1] I would say this suggests GraphicsOutputDxe could be moved to, say, MdeModulePkg/Universal/Graphics - and Logo be moved there too. > > HII/UI: > MdeModulePkg/Application/BootManagerMenuApp/ > MdeModulePkg/Application/UiApp/ > MdeModulePkg/Include/*/*FileExplorer*.h > MdeModulePkg/Include/*/*FormBrowser*.h > MdeModulePkg/Include/*/*Hii*.h > MdeModulePkg/Include/Library/CustomizedDisplayLib.h > MdeModulePkg/Include/Protocol/DisplayProtocol.h > MdeModulePkg/Library/*FileExplorer*/ > MdeModulePkg/Library/*Hii*/ > MdeModulePkg/Library/*UiLib/ > MdeModulePkg/Library/CustomizedDisplayLib/ > MdeModulePkg/Universal/DisplayEngineDxe/ > MdeModulePkg/Universal/FileExplorerDxe/ > MdeModulePkg/Universal/Hii*/ > MdeModulePkg/Universal/SetupBrowserDxe/ > > IPMI: > MdeModulePkg/Include/*/*Ipmi*.h > MdeModulePkg/Library/*Ipmi*/ > > Memory Management: > MdeModulePkg/Application/MemoryProfileInfo/ > MdeModulePkg/Core/Dxe/Gcd/ > MdeModulePkg/Core/Dxe/Mem/ > MdeModulePkg/Core/Pei/Memory/ > MdeModulePkg/Include/*/*Mem*.h > MdeModulePkg/Include/*/*IoMmu*.h > MdeModulePkg/Library/*MemoryAllocation*/ > MdeModulePkg/Universal/MemoryTest/ > > PCD: > MdeModulePkg/Application/DumpDynPcd/ > MdeModulePkg/Include/*/*Pcd*.h > MdeModulePkg/Universal/PCD/ > > PCI Bus: > MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/ > MdeModulePkg/Bus/Pci/PciBusDxe/ > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/ > MdeModulePkg/Core/Pei/PciCfg2/ > MdeModulePkg/Include/Library/PciHostBridgeLib.h > MdeModulePkg/Library/PciHostBridgeLibNull/ > MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/ > > Performance: > MdeModulePkg/Include/*/*Perf*.h > MdeModulePkg/Library/*Perf*/ > > Reset: > MdeModulePkg/Core/Pei/Reset/ > MdeModulePkg/Include/*/*Reset*.h > MdeModulePkg/Library/*Reset*/ > MdeModulePkg/Universal/ResetSystem*/ > > S3: > MdeModulePkg/Include/*/*BootScript*.h > MdeModulePkg/Include/*/*LockBox*.h > MdeModulePkg/Include/*/*S3*.h > MdeModulePkg/Library/*LockBox*/ > MdeModulePkg/Library/*S3*/ > MdeModulePkg/Universal/LockBox/ > > SMBIOS: > MdeModulePkg/Universal/Smbios*/ > > SMM: > MdeModulePkg/Application/SmiHandlerProfileInfo/ > MdeModulePkg/Core/PiSmmCore/ > MdeModulePkg/Include/*/*Smi*.h > MdeModulePkg/Include/*/*Smm*.h > MdeModulePkg/Library/*Smi*/ > MdeModulePkg/Library/*Smm*/ > MdeModulePkg/Universal/SmmCommunicationBufferDxe/ > > Status Code: > MdeModulePkg/Core/Pei/StatusCode/ > MdeModulePkg/Include/*/*StatusCode*.h > MdeModulePkg/Library/*StatusCode*/ > MdeModulePkg/Universal/*StatusCode*/ > > Variable: > MdeModulePkg/Application/VariableInfo/ > MdeModulePkg/Include/*/*FaultTolerantWrite*.h > MdeModulePkg/Include/*/*Var*.h > MdeModulePkg/Include/Guid/SystemNvDataGuid.h > MdeModulePkg/Include/Protocol/SwapAddressRange.h > MdeModulePkg/Library/*Var*/ > MdeModulePkg/Universal/FaultTolerantWrite*/ > MdeModulePkg/Universal/Variable/ > > Misc: I have no issue with the below, but just wanted to point out that if there was an overarching MdeModulePkg/ section, these would automatically match against that anyway - we don't need to explicitly tag everything. > MdeModulePkg/Application/HelloWorld/ > MdeModulePkg/Include/Guid/MdeModulePkgTokenSpace.h > MdeModulePkg/Include/Guid/MtcVendor.h > MdeModulePkg/Include/Guid/ZeroGuid.h > MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > MdeModulePkg/Include/Library/PlatformHookLib.h > MdeModulePkg/Include/Library/RecoveryLib.h > MdeModulePkg/Include/Library/SortLib.h > MdeModulePkg/Include/Library/TpmMeasurementLib.h > MdeModulePkg/Include/Protocol/Dpc.h > MdeModulePkg/Include/Protocol/LoadPe32Image.h > MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h > MdeModulePkg/Include/Protocol/Print2.h > MdeModulePkg/Library/BaseHobLibNull/ > MdeModulePkg/Library/BasePlatformHookLibNull/ > MdeModulePkg/Library/BaseSortLib/ > MdeModulePkg/Library/CpuExceptionHandlerLibNull/ > MdeModulePkg/Library/DxePrintLibPrint2Protocol/ > MdeModulePkg/Library/PeiRecoveryLibNull/ > MdeModulePkg/Library/PlatformHookLibSerialPortPpi/ > MdeModulePkg/Library/TpmMeasurementLibNull/ > MdeModulePkg/Library/UefiSortLib/ > MdeModulePkg/Universal/DevicePathDxe/ > MdeModulePkg/Universal/DriverHealthManagerDxe/ > MdeModulePkg/Universal/DriverSampleDxe/ > MdeModulePkg/Universal/FvSimpleFileSystemDxe/ > MdeModulePkg/Universal/LegacyRegion2Dxe/ > MdeModulePkg/Universal/Metronome/ > MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/ > MdeModulePkg/Universal/PrintDxe/ > MdeModulePkg/Universal/RegularExpressionDxe/ > MdeModulePkg/Universal/TimestampDxe/ > MdeModulePkg/Universal/WatchdogTimerDxe/ Looking through this made me realise it would be useful to have a way of quickly looking up section matches for a given path. So I added a command line option -l/--lookup to GetMaintainer.py - inline below: / Leif >>From 11a04f4690e7b8bc348483a26bb3840b74b1fd9d Mon Sep 17 00:00:00 2001 From: Leif Lindholm Date: Wed, 19 Jun 2019 10:07:26 +0100 Subject: [RFC PATCH 1/1] BaseTools: add lookup function to GetMaintainer.py Add command line option -l/--lookup, which lets you specify a path (existent or not in the current tree) to look up maintainers for. Signed-off-by: Leif Lindholm --- BaseTools/Scripts/GetMaintainer.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py index 616342cdb434..444e601c55d1 100644 --- a/BaseTools/Scripts/GetMaintainer.py +++ b/BaseTools/Scripts/GetMaintainer.py @@ -160,15 +160,22 @@ if __name__ == '__main__': help='git revision to examine (default: HEAD)', nargs='?', default='HEAD') + PARSER.add_argument('-l', '--lookup', + help='Find section matches for path LOOKUP', + required=False) ARGS = PARSER.parse_args() REPO = SetupGit.locate_repo() - FILES = get_modified_files(REPO, ARGS) CONFIG_FILE = os.path.join(REPO.working_dir, 'Maintainers.txt') SECTIONS = parse_maintainers_file(CONFIG_FILE) + if ARGS.lookup: + FILES = [ARGS.lookup] + else: + FILES = get_modified_files(REPO, ARGS) + ADDRESSES = [] for file in FILES: -- 2.11.0