From af6f6cef0bd78f9d60c3b14377b3261f3990fec7 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Mon, 21 Aug 2023 14:24:49 -0700 Subject: [PATCH] instanced_inf_table: Account for supported phases in INF the instanced_inf_table generator did not account for the fact that library class instances can choose to only support phases. This commit adds that support in two phases: 1. Updates the DSC parser's ScopedLibraryDict to store a list of instances that match the scope, instead of the first instance it finds. 2. The instanced_inf_table generator now checks the list of instances ( in order of appearance in the DSC) to find the first instance that matches the scope, but also supports the specific phase. --- .../database/tables/instanced_inf_table.py | 63 ++++++++++++++---- edk2toollib/uefi/edk2/parsers/dsc_parser.py | 12 +++- edk2toollib/uefi/edk2/parsers/inf_parser.py | 2 +- tests.unit/database/common.py | 3 +- .../database/test_instanced_inf_table.py | 66 ++++++++++++++++++- 5 files changed, 130 insertions(+), 16 deletions(-) diff --git a/edk2toollib/database/tables/instanced_inf_table.py b/edk2toollib/database/tables/instanced_inf_table.py index 27275a10..14f8fe3a 100644 --- a/edk2toollib/database/tables/instanced_inf_table.py +++ b/edk2toollib/database/tables/instanced_inf_table.py @@ -43,6 +43,19 @@ def __init__(self, *args, **kwargs): self.arch = self.env["TARGET_ARCH"].split(" ") # REQUIRED self.target = self.env["TARGET"] # REQUIRED + # Prevent parsing the same INF multiple times + self._parsed_infs = {} + + def inf(self, inf: str) -> InfP: + """Returns a parsed INF object.""" + if inf in self._parsed_infs: + infp = self._parsed_infs[inf] + else: + infp = InfP().SetEdk2Path(self.pathobj) + infp.ParseFile(inf) + self._parsed_infs[inf] = infp + return infp + def parse(self, db: Edk2DB) -> None: """Parse the workspace and update the database.""" self.pathobj = db.pathobj @@ -114,8 +127,7 @@ def _parse_inf_recursively( # 0. Use the existing parser to parse the INF file. This parser parses an INF as an independent file # and does not take into account the context of a DSC. # - infp = InfP().SetEdk2Path(self.pathobj) - infp.ParseFile(inf) + infp = self.inf(inf) # # 1. Convert all libraries to their actual instances for this component. This takes into account @@ -158,39 +170,68 @@ def _lib_to_instance(self, library_class_name, scope, library_dict, override_dic """ arch, module = tuple(scope.split(".")) + # NOTE: it is recognized that the below code could be reduced to have less repetitiveness, + # but I personally believe that the below makes it more clear the order in which we search + # for matches, and that the order is quite important. + # https://tianocore-docs.github.io/edk2-DscSpecification/release-1.28/2_dsc_overview/27_[libraryclasses]_section_processing.html#27-libraryclasses-section-processing # 1. If a Library class instance (INF) is specified in the Edk2 II [Components] section (an override), - # then it will be used + # and the library supports the module, then it will be used. if library_class_name in override_dict: return override_dict[library_class_name] # 2/3. If the Library Class instance (INF) is defined in the [LibraryClasses.$(ARCH).$(MODULE_TYPE)] section, - # then it will be used. + # and the library supports the module, then it will be used. lookup = f'{arch}.{module}.{library_class_name}' if lookup in library_dict: - return library_dict[lookup] + library_instance = self._reduce_lib_instances(module, library_dict[lookup]) + if library_instance is not None: + return library_instance # 4. If the Library Class instance (INF) is defined in the [LibraryClasses.common.$(MODULE_TYPE)] section, - # then it will be used. + # and the library supports the module, then it will be used. lookup = f'common.{module}.{library_class_name}' if lookup in library_dict: - return library_dict[lookup] + library_instance = self._reduce_lib_instances(module, library_dict[lookup]) + if library_instance is not None: + return library_instance # 5. If the Library Class instance (INF) is defined in the [LibraryClasses.$(ARCH)] section, - # then it will be used. + # and the library supports the module, then it will be used. lookup = f'{arch}.{library_class_name}' if lookup in library_dict: - return library_dict[lookup] + library_instance = self._reduce_lib_instances(module, library_dict[lookup]) + if library_instance is not None: + return library_instance # 6. If the Library Class Instance (INF) is defined in the [LibraryClasses] section, - # then it will be used. + # and the library supports the module, then it will be used. lookup = f'common.{library_class_name}' if lookup in library_dict: - return library_dict[lookup] + library_instance = self._reduce_lib_instances(module, library_dict[lookup]) + if library_instance is not None: + return library_instance logging.debug(f'scoped library contents: {library_dict}') logging.debug(f'override dictionary: {override_dict}') e = f'Cannot find library class [{library_class_name}] for scope [{scope}] when evaluating {self.dsc}' logging.error(e) raise RuntimeError(e) + + def _reduce_lib_instances(self, module: str, library_instance_list: list[str]) -> str: + """For a DSC, multiple library instances for the same library class can exist. + + This is either due to a mistake by the developer, or because the library class + instance only supports certain modules. That is to say a library class instance + defining `MyLib| PEIM` and one defining `MyLib| PEI_CORE` both being defined in + the same LibraryClasses section is acceptable. + + Due to this, we need to filter to the first library class instance that supports + the module type. + """ + for library_instance in library_instance_list: + infp = self.inf(library_instance) + if module.lower() in [phase.lower() for phase in infp.SupportedPhases]: + return library_instance + return None diff --git a/edk2toollib/uefi/edk2/parsers/dsc_parser.py b/edk2toollib/uefi/edk2/parsers/dsc_parser.py index dfb4242a..e7597fa6 100644 --- a/edk2toollib/uefi/edk2/parsers/dsc_parser.py +++ b/edk2toollib/uefi/edk2/parsers/dsc_parser.py @@ -24,8 +24,15 @@ class DscParser(HashFileParser): OtherMods (list): list of other Mods that are not IA32, X64 specific Libs (list): list of Libs (the line in the file) LibsEnhanced (list): Better parsed (in a dict) list of Libs + ScopedLibraryDict (dict): key (library class) value (list of instances) LibraryClassToInstanceDict (dict): Key (Library class) Value (Instance) Pcds (list): List of Pcds + + !!! note + ScopedLibraryDict can have multiple library instances for the same scope because the INF + can also filter the module types it supports. For example, two library instances could be + in the scope of "common", but one filters to only PEI (MyLib| PEI_CORE) and the other + filters to PEIM (MyLib| PEIM). """ SECTION_LIBRARY = "libraryclasses" SECTION_COMPONENT = "components" @@ -359,7 +366,10 @@ def _parse_libraries(self): for scope in current_scope: key = f"{scope.strip()}.{lib.strip()}".lower() value = instance.strip() - self.ScopedLibraryDict[key] = value + if key in self.ScopedLibraryDict and value not in self.ScopedLibraryDict[key]: + self.ScopedLibraryDict[key].insert(0, value) + else: + self.ScopedLibraryDict[key] = [value] return diff --git a/edk2toollib/uefi/edk2/parsers/inf_parser.py b/edk2toollib/uefi/edk2/parsers/inf_parser.py index 353e5118..b324b8f8 100644 --- a/edk2toollib/uefi/edk2/parsers/inf_parser.py +++ b/edk2toollib/uefi/edk2/parsers/inf_parser.py @@ -12,7 +12,7 @@ from edk2toollib.uefi.edk2.parsers.base_parser import HashFileParser AllPhases = ["SEC", "PEIM", "PEI_CORE", "DXE_DRIVER", "DXE_CORE", "DXE_RUNTIME_DRIVER", "UEFI_DRIVER", - "SMM_CORE", "DXE_SMM_DRIVER", "UEFI_APPLICATION"] + "SMM_CORE", "DXE_SMM_DRIVER", "UEFI_APPLICATION", "MM_STANDALONE", "MM_CORE_STANDALONE"] class InfParser(HashFileParser): diff --git a/tests.unit/database/common.py b/tests.unit/database/common.py index 202a4959..7b32c57a 100644 --- a/tests.unit/database/common.py +++ b/tests.unit/database/common.py @@ -156,12 +156,13 @@ def __init__(self, ws: Path): def create_library(self, name: str, lib_cls: str, **kwargs): """Creates a Library INF in the empty tree.""" path = self.library_folder / f'{name}.inf' - kwargs["defines"] = { + default = { "FILE_GUID": str(uuid.uuid4()), "MODULE_TYPE": "BASE", "BASE_NAME": name, "LIBRARY_CLASS": lib_cls, } + kwargs["defines"] = {**default, **kwargs.get("defines", {})} create_inf_file(path, **kwargs) self.library_list.append(str(path)) return str(path.relative_to(self.ws)) diff --git a/tests.unit/database/test_instanced_inf_table.py b/tests.unit/database/test_instanced_inf_table.py index aafe60ad..4f91b0e5 100644 --- a/tests.unit/database/test_instanced_inf_table.py +++ b/tests.unit/database/test_instanced_inf_table.py @@ -11,8 +11,8 @@ from pathlib import Path import pytest -from common import Tree, empty_tree # noqa: F401 -from edk2toollib.database import Edk2DB +from common import Tree, create_inf_file, empty_tree # noqa: F401 +from edk2toollib.database import Edk2DB, Query from edk2toollib.database.tables import InstancedInfTable from edk2toollib.uefi.edk2.path_utilities import Edk2Path @@ -229,3 +229,65 @@ def test_missing_library(empty_tree: Tree): }) with pytest.raises(RuntimeError): inf_table.parse(db) + +def test_skip_library_with_unsupported_module_type(empty_tree: Tree): + """Library class INFs can specify what module types they support. + + In situations where a library class is in the [LibraryClasses] section, it may not necessarily + support all module types as the LIBRARY_CLASS section of the INF may limit its supported + module types. This test ensures that a library instance is ignored if the library instance + itself states it does not support a module type. + + i.e. LIBRARY_CLASS = TestCls| PEIM only supports PEIM's, even if it is in the [LibraryClasses] section. + """ + edk2path = Edk2Path(str(empty_tree.ws), []) + db = Edk2DB(Edk2DB.MEM_RW, pathobj=edk2path) + + testlib1 = empty_tree.create_library("TestLib1", "TestCls", + defines = { + "LIBRARY_CLASS": "TestCls| PEIM" + } + ) + testlib2 = empty_tree.create_library("TestLib2", "TestCls") + comp1 = empty_tree.create_component("TestDriver1", "DXE_DRIVER", libraryclasses = ["TestCls"]) + + # Generate the DSC with testlib1 first + dsc1 = empty_tree.create_dsc( + libraryclasses = [ + f'TestCls|{testlib1}', + f'TestCls|{testlib2}', + + ], + components = [comp1], + ) + + # Generate the DSC with testlib2 first + dsc2 = empty_tree.create_dsc( + libraryclasses = [ + f'TestCls|{testlib2}', + f'TestCls|{testlib1}', + + ], + components = [comp1], + ) + + inf_table = InstancedInfTable(env = { + "ACTIVE_PLATFORM": dsc1, + "TARGET_ARCH": "X64", + "TARGET": "DEBUG", + }) + + inf_table.parse(db) + + inf_table = InstancedInfTable(env = { + "ACTIVE_PLATFORM": dsc2, + "TARGET_ARCH": "X64", + "TARGET": "DEBUG", + }) + + inf_table.parse(db) + + component_list = db.table("instanced_inf").search(~Query().COMPONENT.exists()) + assert len(component_list) == 2 + for component in component_list: + assert component["LIBRARIES_USED"][0] == Path(testlib2).as_posix()