From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.92.21.43]) by mx.groups.io with SMTP id smtpd.web12.659.1635440952056927346 for ; Thu, 28 Oct 2021 10:09:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@outlook.com header.s=selector1 header.b=McGCuzQj; spf=pass (domain: outlook.com, ip: 40.92.21.43, mailfrom: spbrogan@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=G8BcjaEewtbPHotgPyb0jUlylkaqFMeNj+4QfMydTKiDlwJsUFBH1BZlhSgZWXwlKOmMxwEfVUru44GWWtv4buSqnlf8A9frhq6AnH4wYgKzYqX36xNLa0aJFeU5EuPasn6INKWbrCXtQRe69r+wJoLoVCnfRqtguPLlAoBBV5QYB7cOqDHN/1AOp5jOTw6QHFe1pu75+WYU19tKOKVh+Wwra8UQKAu8DoWFyC4dRQ1CE2LS+y0i8gO635r68AbwrSvpu3TTGeKu44WnOwqFr8GFp0fBiwDVf4zXksGrMYMzw9MnASnf0QcsG81sg/0JlRQ21rP0JdlWzltSlPiElw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=PaJ8ZzuXu5PieExz3mWBniDoVPYhPgPRoXFdhay/Nyo=; b=gyKehIOBEY7flUqjMzC+OKgfUtbkfDsIiv7lSPiyhlAu+VK8qOtPktqY0naO3rpV++Hd5ZP3toN2uvTNyJujkT42xa+4bMuKtDNXWkwXU6y3nT4su/2w9iw9xKjUgR4fdikWWco/ziIMFeS34wTki0nYBy7eBgnrM6WNEhYWzkshiWE11UpJPpDPKkN/M2wRRDifmdSDGjFUftIb6lMkLaGxE3qDyIkyVjx6vhDiHdL4+IvgtjWCkE6DsL+JD5LRyi6ZJsHxSB/wqLQ9kzlRA2b6ZgaILL8ZU3uOjK+apqkd7nLrSJyF65C5VeRQ/iFwJr4RaL7pyOyDY6UU8s8kUA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=PaJ8ZzuXu5PieExz3mWBniDoVPYhPgPRoXFdhay/Nyo=; b=McGCuzQjd7nsaDTyCPkdDy4cYEW9dPMG7+PvlDQqs6R2WGygOzyz6JT4cG7FQ9qSyTiybGB/UR43v/m6eL113vcZrfCPoy+h8cltfMvdmAMKsdej4NxPMr/r8X9KV0IsZvNAVVI50zN2NdFcMbO29WDfe4MboDf2xZ01sq70T0IOeHnDYJcLPrvbKtTlMHWVf6kOw9OIq1YOtyVH0u8X4AhLd4Pm7IDTyX4xrfo2m1L4rcDfavPVqKl5Z+RtyjrFqamquN1YizNTBJ0o5denGzrTLCXri+BoI7jbDo9+YrJRVmhKE4UyTZ07bT7ysbhiGtYFo5pU3KSuzgxmJf1uPA== Received: from BY3PR19MB4900.namprd19.prod.outlook.com (2603:10b6:a03:354::11) by BY5PR19MB3873.namprd19.prod.outlook.com (2603:10b6:a03:225::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4649.13; Thu, 28 Oct 2021 17:09:09 +0000 Received: from BY3PR19MB4900.namprd19.prod.outlook.com ([fe80::44c5:11a3:13d9:c2fb]) by BY3PR19MB4900.namprd19.prod.outlook.com ([fe80::44c5:11a3:13d9:c2fb%7]) with mapi id 15.20.4649.015; Thu, 28 Oct 2021 17:09:09 +0000 Subject: Re: [edk2-devel] [PATCH 1/6] OvmfPkg/PlatformCI: factor out PlatformBuildLib.py To: devel@edk2.groups.io, kraxel@redhat.com Cc: Jiewen Yao , Jordan Justen , Ard Biesheuvel References: <20211019080832.265545-1-kraxel@redhat.com> <20211019080832.265545-2-kraxel@redhat.com> From: "Sean" Message-ID: Date: Thu, 28 Oct 2021 10:09:07 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 In-Reply-To: <20211019080832.265545-2-kraxel@redhat.com> X-TMN: [Ul5J1FoEDn32LdPQzRQDbMDlwq0T4rny] X-ClientProxiedBy: MW4PR04CA0036.namprd04.prod.outlook.com (2603:10b6:303:6a::11) To BY3PR19MB4900.namprd19.prod.outlook.com (2603:10b6:a03:354::11) Return-Path: spbrogan@outlook.com X-Microsoft-Original-Message-ID: <24cdeeb9-d747-aecf-e87a-673fac9d8a81@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.2.78] (50.47.113.221) by MW4PR04CA0036.namprd04.prod.outlook.com (2603:10b6:303:6a::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4649.13 via Frontend Transport; Thu, 28 Oct 2021 17:09:08 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 9a92fca1-287b-4366-16cc-08d99a35a7c6 X-MS-TrafficTypeDiagnostic: BY5PR19MB3873: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fnXArDODDUHiAw81mJkXY4uLXEjwzMZpO31SDP4RbmWYaesDt1vmFK9lqrC5aHcSB4oob08Zj9WyEmpq4Pd8Zrbj2fwvzx0Kru7a3Yd128SpcOgpQYO5Ja/YJN3xyi2XH2sm5DUblcB1xvOsKLKKjqT8MTBbxSKV52LxHLkmyeDpybcZFM96shII0BIv/hh4Hk9pDgOnDgI170xPqwUIrd2NMAU8HVRe3+vgghWw47l9SFXlq7HFd+SSFE8NIsZxK8I4/lUfY1qfmFXfw3PXkk2aedLyC8j6torpBJPPnN+ddYqmMijU7uh1cRNs3jmE5mQl9SsAnAHdPicGDSRR4aaUI0d3I/SduRRhDwkZ708okSbc0h+ikPqB7KqPIeKhI5k2M0f4ja3gfaEPJJ5hfuE0IiqxE6ob8OnpzTVBdm+m1TTQerqBG1c507euwhzU49/qK4J5bGuVL3jttQeQyfFzQnRfzBQwLnsvObfIZR2fusf1JzXOo56wXLPfLiChQBKR83idfrLH6+JHlULYCC3uhxVWect79BFbAYLDMW0= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: l/J9OGvuxrA56zrPr/U/7VccOXkrG/xrg+xYAVMyAsZtgT/Hs92NK99m5AH41LsEfwk6wABASD4IzONpJdHAB/3v0spe5E6jnqBsGxruulDwMVZCcuQ/FlD32vMZtEoIG+6D6hDODaOa7nNSuRBt+cog3qSzprQdJQMtECwEEtG30DnwQZQ6wJQ/fxuFVlcrnEG0qsYTMIOhMh3+xetUNCZXW8WdWsBu6t6q9DGoxp5WiIWhf68onXwJAZSvot3/OSIif+e0MT/AoWTLZEzOpCJURGjp/tctxDlS2/dTMr/zjd7JV7VutP5Psk6hj6plDj7bGVuHbqiKIggW59EtDLIYOnEVCA9wleRl+ouVqyIRav/iZtK+oNP89EPH3TSLdFPuCl/6Vk9EFnmplu5kmGcY0lPTU5JueUgz86kaVLQO5APV3pXGCJCF3G+Ws1u+lQRIy/qXSyYzJl3i2+py1JL5m6Sz4FaNU2XCLOH+yJn/5fgeD1zPT/D3s9DM1ufkE1zamulP40ceIm1B+BNLTIfO3JL3JMPj8wy7u2QBYkKs8UPm6RfxuYzsqNTg6vZk/wuu6S9AIBrtIVhnN7XdZPS6hgXgkiQJ7RFNfCrk5DxqWY11aRFQa0J+FZZN2Hh/1osnYeZeWQueuqXlwT3Vft1mS98R1q04dOxZoyDFXNxIBcFUNWJg0WNssbpudTA+SDd0kHDZaKo9h0YAqds+UA== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9a92fca1-287b-4366-16cc-08d99a35a7c6 X-MS-Exchange-CrossTenant-AuthSource: BY3PR19MB4900.namprd19.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Oct 2021 17:09:09.6222 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR19MB3873 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit one thing that jumped out quickly (comment below). Do you have a branch that i can look at and compare online? Thanks Sean On 10/19/2021 1:08 AM, Gerd Hoffmann wrote: > Move SettingsManager and PlatformBuilder classes to PlatformBuildLib.py > file, keep only CommonPlatform class in PlatformBuild.py. Allows > reusing these classes for other builds. Pure code motion, no functional > change. > > Signed-off-by: Gerd Hoffmann > --- > OvmfPkg/PlatformCI/PlatformBuild.py | 223 +----------------- > .../{PlatformBuild.py => PlatformBuildLib.py} | 32 --- > 2 files changed, 6 insertions(+), 249 deletions(-) > copy OvmfPkg/PlatformCI/{PlatformBuild.py => PlatformBuildLib.py} (87%) > > diff --git a/OvmfPkg/PlatformCI/PlatformBuild.py b/OvmfPkg/PlatformCI/PlatformBuild.py > index 627bb7b992db..413f004990e5 100644 > --- a/OvmfPkg/PlatformCI/PlatformBuild.py > +++ b/OvmfPkg/PlatformCI/PlatformBuild.py > @@ -5,17 +5,11 @@ > # SPDX-License-Identifier: BSD-2-Clause-Patent > ## > import os > -import logging > -import io > - > -from edk2toolext.environment import shell_environment > -from edk2toolext.environment.uefi_build import UefiBuilder > -from edk2toolext.invocables.edk2_platform_build import BuildSettingsManager > -from edk2toolext.invocables.edk2_setup import SetupSettingsManager, RequiredSubmodule > -from edk2toolext.invocables.edk2_update import UpdateSettingsManager > -from edk2toolext.invocables.edk2_pr_eval import PrEvalSettingsManager > -from edk2toollib.utility_functions import RunCmd > +import sys > > +sys.path.append(os.path.join(os.getcwd(), 'OvmfPkg', 'PlatformCI')) I would strongly suggest you base your path off your current file location rather than CWD. This makes invocation much more flexible. See how workspace root is determined for similar solution. > +from PlatformBuildLib import SettingsManager > +from PlatformBuildLib import PlatformBuilder > > # ####################################################################################### # > # Common Configuration # > @@ -45,210 +39,5 @@ class CommonPlatform(): > dsc += ".dsc" > return dsc > > - > - # ####################################################################################### # > - # Configuration for Update & Setup # > - # ####################################################################################### # > -class SettingsManager(UpdateSettingsManager, SetupSettingsManager, PrEvalSettingsManager): > - > - def GetPackagesSupported(self): > - ''' return iterable of edk2 packages supported by this build. > - These should be edk2 workspace relative paths ''' > - return CommonPlatform.PackagesSupported > - > - def GetArchitecturesSupported(self): > - ''' return iterable of edk2 architectures supported by this build ''' > - return CommonPlatform.ArchSupported > - > - def GetTargetsSupported(self): > - ''' return iterable of edk2 target tags supported by this build ''' > - return CommonPlatform.TargetsSupported > - > - def GetRequiredSubmodules(self): > - ''' return iterable containing RequiredSubmodule objects. > - If no RequiredSubmodules return an empty iterable > - ''' > - rs = [] > - > - # intentionally declare this one with recursive false to avoid overhead > - rs.append(RequiredSubmodule( > - "CryptoPkg/Library/OpensslLib/openssl", False)) > - > - # To avoid maintenance of this file for every new submodule > - # lets just parse the .gitmodules and add each if not already in list. > - # The GetRequiredSubmodules is designed to allow a build to optimize > - # the desired submodules but it isn't necessary for this repository. > - result = io.StringIO() > - ret = RunCmd("git", "config --file .gitmodules --get-regexp path", workingdir=self.GetWorkspaceRoot(), outstream=result) > - # Cmd output is expected to look like: > - # submodule.CryptoPkg/Library/OpensslLib/openssl.path CryptoPkg/Library/OpensslLib/openssl > - # submodule.SoftFloat.path ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3 > - if ret == 0: > - for line in result.getvalue().splitlines(): > - _, _, path = line.partition(" ") > - if path is not None: > - if path not in [x.path for x in rs]: > - rs.append(RequiredSubmodule(path, True)) # add it with recursive since we don't know > - return rs > - > - def SetArchitectures(self, list_of_requested_architectures): > - ''' Confirm the requests architecture list is valid and configure SettingsManager > - to run only the requested architectures. > - > - Raise Exception if a list_of_requested_architectures is not supported > - ''' > - unsupported = set(list_of_requested_architectures) - set(self.GetArchitecturesSupported()) > - if(len(unsupported) > 0): > - errorString = ( "Unsupported Architecture Requested: " + " ".join(unsupported)) > - logging.critical( errorString ) > - raise Exception( errorString ) > - self.ActualArchitectures = list_of_requested_architectures > - > - def GetWorkspaceRoot(self): > - ''' get WorkspacePath ''' > - return CommonPlatform.WorkspaceRoot > - > - def GetActiveScopes(self): > - ''' return tuple containing scopes that should be active for this process ''' > - return CommonPlatform.Scopes > - > - def FilterPackagesToTest(self, changedFilesList: list, potentialPackagesList: list) -> list: > - ''' Filter other cases that this package should be built > - based on changed files. This should cover things that can't > - be detected as dependencies. ''' > - build_these_packages = [] > - possible_packages = potentialPackagesList.copy() > - for f in changedFilesList: > - # BaseTools files that might change the build > - if "BaseTools" in f: > - if os.path.splitext(f) not in [".txt", ".md"]: > - build_these_packages = possible_packages > - break > - > - # if the azure pipeline platform template file changed > - if "platform-build-run-steps.yml" in f: > - build_these_packages = possible_packages > - break > - > - return build_these_packages > - > - def GetPlatformDscAndConfig(self) -> tuple: > - ''' If a platform desires to provide its DSC then Policy 4 will evaluate if > - any of the changes will be built in the dsc. > - > - The tuple should be (, ) > - ''' > - dsc = CommonPlatform.GetDscName(",".join(self.ActualArchitectures)) > - return (f"OvmfPkg/{dsc}", {}) > - > - > - # ####################################################################################### # > - # Actual Configuration for Platform Build # > - # ####################################################################################### # > -class PlatformBuilder( UefiBuilder, BuildSettingsManager): > - def __init__(self): > - UefiBuilder.__init__(self) > - > - def AddCommandLineOptions(self, parserObj): > - ''' Add command line options to the argparser ''' > - parserObj.add_argument('-a', "--arch", dest="build_arch", type=str, default="IA32,X64", > - help="Optional - CSV of architecture to build. IA32 will use IA32 for Pei & Dxe. " > - "X64 will use X64 for both PEI and DXE. IA32,X64 will use IA32 for PEI and " > - "X64 for DXE. default is IA32,X64") > - > - def RetrieveCommandLineOptions(self, args): > - ''' Retrieve command line options from the argparser ''' > - > - shell_environment.GetBuildVars().SetValue("TARGET_ARCH"," ".join(args.build_arch.upper().split(",")), "From CmdLine") > - dsc = CommonPlatform.GetDscName(args.build_arch) > - shell_environment.GetBuildVars().SetValue("ACTIVE_PLATFORM", f"OvmfPkg/{dsc}", "From CmdLine") > - > - def GetWorkspaceRoot(self): > - ''' get WorkspacePath ''' > - return CommonPlatform.WorkspaceRoot > - > - def GetPackagesPath(self): > - ''' Return a list of workspace relative paths that should be mapped as edk2 PackagesPath ''' > - return () > - > - def GetActiveScopes(self): > - ''' return tuple containing scopes that should be active for this process ''' > - return CommonPlatform.Scopes > - > - def GetName(self): > - ''' Get the name of the repo, platform, or product being build ''' > - ''' Used for naming the log file, among others ''' > - # check the startup nsh flag and if set then rename the log file. > - # this helps in CI so we don't overwrite the build log since running > - # uses the stuart_build command. > - if(shell_environment.GetBuildVars().GetValue("MAKE_STARTUP_NSH", "FALSE") == "TRUE"): > - return "OvmfPkg_With_Run" > - return "OvmfPkg" > - > - def GetLoggingLevel(self, loggerType): > - ''' Get the logging level for a given type > - base == lowest logging level supported > - con == Screen logging > - txt == plain text file logging > - md == markdown file logging > - ''' > - return logging.DEBUG > - > - def SetPlatformEnv(self): > - logging.debug("PlatformBuilder SetPlatformEnv") > - self.env.SetValue("PRODUCT_NAME", "OVMF", "Platform Hardcoded") > - self.env.SetValue("MAKE_STARTUP_NSH", "FALSE", "Default to false") > - self.env.SetValue("QEMU_HEADLESS", "FALSE", "Default to false") > - return 0 > - > - def PlatformPreBuild(self): > - return 0 > - > - def PlatformPostBuild(self): > - return 0 > - > - def FlashRomImage(self): > - VirtualDrive = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "VirtualDrive") > - os.makedirs(VirtualDrive, exist_ok=True) > - OutputPath_FV = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "FV") > - > - # > - # QEMU must be on the path > - # > - cmd = "qemu-system-x86_64" > - args = "-debugcon stdio" # write messages to stdio > - args += " -global isa-debugcon.iobase=0x402" # debug messages out thru virtual io port > - args += " -net none" # turn off network > - args += f" -drive file=fat:rw:{VirtualDrive},format=raw,media=disk" # Mount disk with startup.nsh > - > - if (self.env.GetValue("QEMU_HEADLESS").upper() == "TRUE"): > - args += " -display none" # no graphics > - > - if (self.env.GetBuildValue("SMM_REQUIRE") == "1"): > - args += " -machine q35,smm=on" #,accel=(tcg|kvm)" > - #args += " -m ..." > - #args += " -smp ..." > - args += " -global driver=cfi.pflash01,property=secure,value=on" > - args += " -drive if=pflash,format=raw,unit=0,file=" + os.path.join(OutputPath_FV, "OVMF_CODE.fd") + ",readonly=on" > - args += " -drive if=pflash,format=raw,unit=1,file=" + os.path.join(OutputPath_FV, "OVMF_VARS.fd") > - else: > - args += " -pflash " + os.path.join(OutputPath_FV, "OVMF.fd") # path to firmware > - > - > - if (self.env.GetValue("MAKE_STARTUP_NSH").upper() == "TRUE"): > - f = open(os.path.join(VirtualDrive, "startup.nsh"), "w") > - f.write("BOOT SUCCESS !!! \n") > - ## add commands here > - f.write("reset -s\n") > - f.close() > - > - ret = RunCmd(cmd, args) > - > - if ret == 0xc0000005: > - #for some reason getting a c0000005 on successful return > - return 0 > - > - return ret > - > - > - > +import PlatformBuildLib > +PlatformBuildLib.CommonPlatform = CommonPlatform > diff --git a/OvmfPkg/PlatformCI/PlatformBuild.py b/OvmfPkg/PlatformCI/PlatformBuildLib.py > similarity index 87% > copy from OvmfPkg/PlatformCI/PlatformBuild.py > copy to OvmfPkg/PlatformCI/PlatformBuildLib.py > index 627bb7b992db..90ac0b29a892 100644 > --- a/OvmfPkg/PlatformCI/PlatformBuild.py > +++ b/OvmfPkg/PlatformCI/PlatformBuildLib.py > @@ -17,35 +17,6 @@ from edk2toolext.invocables.edk2_pr_eval import PrEvalSettingsManager > from edk2toollib.utility_functions import RunCmd > > > - # ####################################################################################### # > - # Common Configuration # > - # ####################################################################################### # > -class CommonPlatform(): > - ''' Common settings for this platform. Define static data here and use > - for the different parts of stuart > - ''' > - PackagesSupported = ("OvmfPkg",) > - ArchSupported = ("IA32", "X64") > - TargetsSupported = ("DEBUG", "RELEASE", "NOOPT") > - Scopes = ('ovmf', 'edk2-build') > - WorkspaceRoot = os.path.realpath(os.path.join( > - os.path.dirname(os.path.abspath(__file__)), "..", "..")) > - > - @classmethod > - def GetDscName(cls, ArchCsv: str) -> str: > - ''' return the DSC given the architectures requested. > - > - ArchCsv: csv string containing all architectures to build > - ''' > - dsc = "OvmfPkg" > - if "IA32" in ArchCsv.upper().split(","): > - dsc += "Ia32" > - if "X64" in ArchCsv.upper().split(","): > - dsc += "X64" > - dsc += ".dsc" > - return dsc > - > - > # ####################################################################################### # > # Configuration for Update & Setup # > # ####################################################################################### # > @@ -249,6 +220,3 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager): > return 0 > > return ret > - > - > - >