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 5 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)
6 changes: 5 additions & 1 deletion edk2toollib/database/tables/instanced_fv_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
"""A module to generate a table containing fv information."""
import re
from pathlib import Path

from tinyrecord import transaction
Expand All @@ -27,6 +28,9 @@ class InstancedFvTable(TableGenerator):
|------------------------------------------------------|
```
""" # noqa: E501

RULEOVERRIDE = re.compile(r'RuleOverride\s*=.+\s+(.*\.inf)', re.IGNORECASE)
Javagedes marked this conversation as resolved.
Show resolved Hide resolved

def __init__(self, *args, **kwargs):
"""Initialize the query with the specific settings."""
self.env = kwargs.pop("env")
Expand Down Expand Up @@ -54,7 +58,7 @@ def parse(self, db: Edk2DB) -> None:
inf_list = [] # Some INF's start with RuleOverride. We only need the INF
for inf in fdfp.FVs[fv]["Infs"]:
if inf.lower().startswith("ruleoverride"):
inf = inf.split(" ", 1)[-1]
inf = InstancedFvTable.RULEOVERRIDE.findall(inf)[0]
if Path(inf).is_absolute():
inf = str(Path(self.pathobj.GetEdk2RelativePathFromAbsolutePath(inf)))
inf_list.append(Path(inf).as_posix())
Expand Down
102 changes: 80 additions & 22 deletions edk2toollib/database/tables/instanced_inf_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ class InstancedInfTable(TableGenerator):

``` py
table_name = "instanced_inf"
|----------------------------------------------------------------------------------------------------------------------------------|
| DSC | GUID | LIBRARY_CLASS | PATH | PHASES | SOURCES_USED | LIBRARIES_USED | PROTOCOLS_USED | GUIDS_USED | PPIS_USED | PCDS_USED |
|----------------------------------------------------------------------------------------------------------------------------------|
|----------------------------------------------------------------------------------------------------------------------------------------------|
| DSC | PATH | NAME | LIBRARY_CLASS | COMPONENT | MODULE_TYPE | ARCH | SOURCES_USED | LIBRARIES_USED | PROTOCOLS_USED | GUIDS_USED | PCDS_USED |
|----------------------------------------------------------------------------------------------------------------------------------------------|
```
""" # noqa: E501
SECTION_LIBRARY = "LibraryClasses"
Expand All @@ -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,44 +119,60 @@ 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,
"PATH": Path(inf).as_posix(),
"NAME": infp.Dict["BASE_NAME"],
"LIBRARY_CLASS": infp.LibraryClass,
"COMPONENT": Path(component).as_posix(),
"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 +187,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
5 changes: 4 additions & 1 deletion tests.unit/database/test_instanced_fv_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def test_valid_fdf(empty_tree: Tree): # noqa: F811
comp2 = empty_tree.create_component("TestDriver2", "DXE_DRIVER")
comp3 = empty_tree.create_component("TestDriver3", "DXE_DRIVER")
comp4 = str(Path('TestPkg','Extra Drivers','TestDriver4.inf'))
comp5 = empty_tree.create_component("TestDriver5", "DXE_DRIVER")

dsc = empty_tree.create_dsc()

Expand All @@ -39,7 +40,8 @@ def test_valid_fdf(empty_tree: Tree): # noqa: F811
f"INF {comp1}", # PP relative
f'INF {str(empty_tree.ws / comp2)}', # Absolute
f'INF RuleOverride=RESET_VECTOR {comp3}', # RuleOverride
f'INF {comp4}' # Space in path
f'INF {comp4}', # Space in path
f'INF ruleoverride = RESET_VECTOR {comp5}', # RuleOverride lowercase & spaces
]
)

Expand All @@ -62,5 +64,6 @@ def test_valid_fdf(empty_tree: Tree): # noqa: F811
Path(comp1).as_posix(),
Path(comp2).as_posix(),
Path(comp3).as_posix(),
Path(comp5).as_posix(),
Path(comp4).as_posix(),
])
Loading