Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database: multiple bugfixes #393

Merged
merged 13 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Pipfile
Javagedes marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]

[dev-packages]

[requires]
python_version = "3.11"
2 changes: 1 addition & 1 deletion edk2toollib/database/queries/unused_component_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,4 @@ def _recurse_inf(self, inf, table, library_list):
library_list.append(inf)

for inf in result["LIBRARIES_USED"]:
self._recurse_inf(inf, table, library_list)
self._recurse_inf(inf[1], table, library_list)
95 changes: 76 additions & 19 deletions edk2toollib/database/tables/instanced_inf_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -106,34 +119,49 @@ def _parse_inf_recursively(

Will immediately return if the INF has already been visited.
"""
if inf is None:
return []

logging.debug(f" Parsing Library: [{inf}]")
visited.append(inf)
library_instances = []
library_instance_list = []
library_class_list = []


#
# 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
# any overrides for this component
#
for lib in infp.get_libraries(self.arch):
lib = lib.split(" ")[0].lower()
library_instances.append(self._lib_to_instance(lib, scope, library_dict, override_dict))
lib = lib.split(" ")[0]
library_instance_list.append(self._lib_to_instance(lib.lower(), scope, library_dict, override_dict))
library_class_list.append(lib)
# Append all NULL library instances
for null_lib in override_dict["NULL"]:
library_instances.append(null_lib)
library_instance_list.append(null_lib)
library_class_list.append("NULL")

to_parse = list(filter(lambda lib: lib is not None, library_instance_list))

# Time to visit in libraries that we have not visited yet.
to_return = []
for library in filter(lambda lib: lib not in visited, library_instances):
for library in filter(lambda lib: lib not in visited, to_parse):
to_return += self._parse_inf_recursively(library, component,
library_dict, override_dict, scope, visited)

# Transform path to edk2 relative form (POSIX)
def to_posix(path):
if path is None:
return None
return Path(path).as_posix()
library_instance_list = list(map(to_posix, library_instance_list))

# Return Paths as posix paths, which is Edk2 standard.
to_return.append({
"DSC": Path(self.dsc).name,
Expand All @@ -143,7 +171,7 @@ def _parse_inf_recursively(
"MODULE_TYPE": infp.Dict["MODULE_TYPE"],
"ARCH": scope.split(".")[0].upper(),
"SOURCES_USED": list(map(lambda p: Path(p).as_posix(), infp.Sources)),
"LIBRARIES_USED": list(map(lambda p: Path(p).as_posix(), library_instances)),
"LIBRARIES_USED": list(zip(library_class_list, library_instance_list)),
"PROTOCOLS_USED": [], # TODO
"GUIDS_USED": [], # TODO
"PPIS_USED": [], # TODO
Expand All @@ -158,39 +186,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)
logging.warn(e)
return None

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
12 changes: 11 additions & 1 deletion edk2toollib/uefi/edk2/parsers/dsc_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion edk2toollib/uefi/edk2/parsers/inf_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Javagedes marked this conversation as resolved.
Show resolved Hide resolved


class InfParser(HashFileParser):
Expand Down
3 changes: 2 additions & 1 deletion tests.unit/database/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion tests.unit/database/test_component_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_simple_component(empty_tree: Tree):
result = db.search(ComponentQuery(component = "TestDriver1"))
assert len(result) == 1

assert sorted(result[0]['LIBRARIES_USED']) == sorted(['TestPkg/Library/TestLib2.inf', 'TestPkg/Library/TestLib3.inf'])
assert sorted(result[0]['LIBRARIES_USED']) == sorted([('TestCls','TestPkg/Library/TestLib2.inf'), ('NULL','TestPkg/Library/TestLib3.inf')])

result = db.search(ComponentQuery(component = "NonExistantDriver"))
assert len(result) == 0
82 changes: 74 additions & 8 deletions tests.unit/database/test_instanced_inf_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -128,8 +128,8 @@ def test_library_override(empty_tree: Tree):
# Ensure the Test Driver is using TestLib2 from the override and the NULL library was added
for row in db.table("instanced_inf").all():
if (row["NAME"] == Path(comp1).stem
and Path(lib2).as_posix() in row["LIBRARIES_USED"]
and Path(lib3).as_posix() in row["LIBRARIES_USED"]):
and ("TestCls", Path(lib2).as_posix()) in row["LIBRARIES_USED"]
and ("NULL", Path(lib3).as_posix()) in row["LIBRARIES_USED"]):
break
else:
assert False
Expand Down Expand Up @@ -172,7 +172,7 @@ def test_scoped_libraries1(empty_tree: Tree):
# For each driver, verify that the the driver number (1, 2, 3) uses the corresponding lib number (1, 2, 3)
for row in db.table("instanced_inf").all():
if "COMPONENT" not in row: # Only care about looking at drivers, which do not have a component
assert row["NAME"].replace("Driver", "Lib") in row["LIBRARIES_USED"][0]
assert row["NAME"].replace("Driver", "Lib") in row["LIBRARIES_USED"][0][1]

def test_scoped_libraries2(empty_tree: Tree):
"""Ensure that the correct libraries in regards to scoping.
Expand Down Expand Up @@ -207,9 +207,9 @@ def test_scoped_libraries2(empty_tree: Tree):

for row in db.table("instanced_inf").all():
if "COMPONENT" not in row:
assert row["NAME"].replace("Driver", "Lib") in row["LIBRARIES_USED"][0]
assert row["NAME"].replace("Driver", "Lib") in row["LIBRARIES_USED"][0][1]

def test_missing_library(empty_tree: Tree):
def test_missing_library(empty_tree: Tree, caplog):
"""Test when a library is missing."""
edk2path = Edk2Path(str(empty_tree.ws), [])
db = Edk2DB(Edk2DB.MEM_RW, pathobj=edk2path)
Expand All @@ -227,5 +227,71 @@ def test_missing_library(empty_tree: Tree):
"TARGET_ARCH": "IA32 X64",
"TARGET": "DEBUG",
})
with pytest.raises(RuntimeError):

with caplog.at_level(logging.WARNING):
inf_table.parse(db)

assert len(caplog.records) == 1
assert 'testcls' in caplog.records[0].message

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] == ("TestCls", Path(testlib2).as_posix())