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

Conversation

Javagedes
Copy link
Contributor

@Javagedes Javagedes commented Aug 21, 2023

This PR contains the following database changes:

[bug] Account for library instances that do not exist in the DSC

The instanced_inf_table generator did not account for the fact that
library class instances can choose to only support 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.

[bug] Account for library instances that support only a subset of phases.

Due to platforms performing complex includes in their DSCs, There is a
scenario in which a library instance references a library class that
does not exist. This is not an error as the library class instance may
not be consumed by a component in the platform, so the missing
dependency is acceptable.

This commit adds the ability to handle this scenario, and will not raise
an error for a missing library instance.

[enhancement] Include library class and library instance in "LIBRARIES_USED" column of the instanced_inf table

The LIBRARIES_USED column in the instanced_inf table only contained
the the edk2 relative path to the library instance. This made it
impossible to determine (with 100% accuracy) which library class the
particular library instance was representing.

This commit changes the "LIBRARIES_USED" column to contain a list of
(lib_class, lib_instance) tuples rather than a list of lib_instances.
Please review commit messages in each for a more detailed description.

[enhancement] Include "LIBRARY_CLASS" column in instanced_inf table

Previously, the library class name for a library INF was not included
in the table. This commit adds that information to the table as a row
to make it easier to find the library class name for a library INF.

[bug] Better handling of RuleOverride in instanced_fv_table

When parsing an FDF file, an INF can be prepending with a RuleOverride
directive. The previous functionality to ignore this directive and
retrive the actual INF value could not handle any spaces in the
directive (such as RuleOverride = ACPITABLE). This commit expands the
ability to handle spaces in the directive.

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Patch coverage: 92.85% and project coverage change: +0.09% 🎉

Comparison is base (02f00cc) 80.97% compared to head (85f2356) 81.06%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
+ Coverage   80.97%   81.06%   +0.09%     
==========================================
  Files          60       60              
  Lines        7268     7304      +36     
==========================================
+ Hits         5885     5921      +36     
  Misses       1383     1383              
Files Changed Coverage Δ
edk2toollib/uefi/edk2/parsers/inf_parser.py 91.94% <ø> (+1.34%) ⬆️
edk2toollib/uefi/fmp_capsule_header.py 34.26% <0.00%> (ø)
edk2toollib/database/tables/instanced_inf_table.py 98.38% <95.55%> (-1.62%) ⬇️
...toollib/database/queries/unused_component_query.py 100.00% <100.00%> (ø)
edk2toollib/database/tables/instanced_fv_table.py 100.00% <100.00%> (ø)
edk2toollib/uefi/edk2/parsers/dsc_parser.py 87.65% <100.00%> (+0.07%) ⬆️
edk2toollib/uefi/edk2/parsers/override_parser.py 88.05% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Javagedes Javagedes force-pushed the bugfix-instanced_inf_table branch from ae725ad to 954e553 Compare August 21, 2023 21:33
@Javagedes Javagedes changed the title instanced_inf_table: Account for supported phases in INF instanced_inf_table: multiple bugfixes Aug 21, 2023
@Javagedes Javagedes changed the title instanced_inf_table: multiple bugfixes [REBASE&FF] instanced_inf_table: multiple bugfixes Aug 21, 2023
@Javagedes Javagedes force-pushed the bugfix-instanced_inf_table branch 3 times, most recently from b67b71f to 3e1e83b Compare August 21, 2023 22:45
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.
Due to platforms performing complex includes in their DSCs, There is a
scenario in which a library instance references a library class that
does not exist. This is not an error as the library class instance may
not be consumed by a component in the platform, so the missing
dependency is acceptable.

This commit adds the ability to handle this scenario, and will not raise
an error for a missing library instance.
@Javagedes Javagedes force-pushed the bugfix-instanced_inf_table branch 3 times, most recently from 8c72eb3 to 9ddffb0 Compare August 22, 2023 21:29
The LIBRARIES_USED column in the instanced_inf table only contained
the the edk2 relative path to the library instance. This made it
impossible to determine (with 100% accuracy) which library class the
particular library instance was representing.

This commit changes the "LIBRARIES_USED" column to contain a list of
(lib_class, lib_instance) tuples rather than a list of lib_instances.

Tests
Previously, the library class name for a library INF was not included
in the table. This commit adds that information to the table as a row
to make it easier to find the library class name for a library INF.
@Javagedes Javagedes force-pushed the bugfix-instanced_inf_table branch from 9ddffb0 to f165c95 Compare August 22, 2023 21:33
@Javagedes Javagedes changed the title [REBASE&FF] instanced_inf_table: multiple bugfixes database: multiple bugfixes Aug 29, 2023
@Javagedes Javagedes force-pushed the bugfix-instanced_inf_table branch 5 times, most recently from de594b0 to 6fa229c Compare August 29, 2023 22:39
Javagedes and others added 6 commits August 29, 2023 15:49
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.
Due to platforms performing complex includes in their DSCs, There is a
scenario in which a library instance references a library class that
does not exist. This is not an error as the library class instance may
not be consumed by a component in the platform, so the missing
dependency is acceptable.

This commit adds the ability to handle this scenario, and will not raise
an error for a missing library instance.
The LIBRARIES_USED column in the instanced_inf table only contained
the the edk2 relative path to the library instance. This made it
impossible to determine (with 100% accuracy) which library class the
particular library instance was representing.

This commit changes the "LIBRARIES_USED" column to contain a list of
(lib_class, lib_instance) tuples rather than a list of lib_instances.

Tests
Previously, the library class name for a library INF was not included
in the table. This commit adds that information to the table as a row
to make it easier to find the library class name for a library INF.
When parsing an FDF file, an INF can be prepending with a RuleOverride
directive. The previous functionality to ignore this directive and
retrive the actual INF value could not handle any spaces in the
directive (such as RuleOverride = ACPITABLE). This commit expands the
ability to handle spaces in the directive.
@Javagedes Javagedes force-pushed the bugfix-instanced_inf_table branch from 6fa229c to f14ab1e Compare August 29, 2023 22:49
@Javagedes Javagedes added this to the v0.17.1 milestone Aug 30, 2023
@Javagedes Javagedes self-assigned this Aug 30, 2023
@Javagedes Javagedes added bug Something isn't working enhancement New feature or request labels Aug 30, 2023
edk2toollib/uefi/edk2/parsers/inf_parser.py Outdated Show resolved Hide resolved
Pipfile Outdated Show resolved Hide resolved
@Javagedes Javagedes merged commit 17a2dac into tianocore:master Sep 6, 2023
12 checks passed
@Javagedes Javagedes modified the milestones: v0.17.1, v0.18.0 Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants