BaseTools: Detect library class mismatch

[Javagedes]

Description

Performs a check that will verify that the library instance implements the library specified in the dsc by ensuring a LIBRARY_CLASS definition exists in the INF [Defines] section and the value matches the library it says it is implementing.

That is to say, for the following example: TestLib|Path/To/BaseTestLib.inf, that BaseTestLib.inf has LIBRARY_CLASS = TestLib defined in the [Defines] section.

• [ ] Breaking change?
  • Breaking change - Will this cause a break in build or boot behavior?
  • Examples: Add a new library class or move a module to a different repo.
• [ ] Impacts security?
  • Security - Does the change have a direct security impact?
  • Examples: Crypto algorithm change or buffer overflow fix.
• [ ] Includes tests?
  • Tests - Does the change include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

<Describe the test(s) that were run to verify the changes.>

Integration Instructions

While no Integration is required, a new build warning will be seen in the build log. The warning is in the following format: $(DSC_PATH): warning: $(INF) does not support LIBRARY_CLASS $(LIBRARY_CLASS) where $(DSC_PATH) is the DSC being built, $(INF) is the INF described in the DSC, and $(LIBRARY_CLASS) is the name of the library class that the INF is attempting to represent. (i.e., $(LIBRARY_CLASS)|$(INF) or TestLib|Path\to\BaseTestLib.inf).

To resolve these errors, verify the library class is real (The Library class should have a header file associated with it in the DEC of the defining package). If it is real, then ensure the library instance truly implements the library class and add it to the DEFINE section of the library INF. If not, update the DSC to use the correct library class.

If this warning appears, it will appear at the beginning of the build command: Processing meta-data . Architecture(s) = IA32 X64 Build target = DEBUG Toolchain = VS2019 Active Platform = c:\src\Path\Platform.dsc build... c:\src\Path\Platform.dsc(...): warning: c:\src\Path\BaseTestLib.inf does not support LIBRARY_CLASS TestLib

[mdkinney]

@lgao4 @bcran This PR is in approved state, but maintainer has not set push label. Are there additional comments?

[ghost]

@lgao4 is more familiar with this code so I'll let him set the push label.

[mdkinney]

@lgao4 Please review

[bexcran]

@lgao4 ping. This is still waiting for a review.

[github-actions[bot]]

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[mergify[bot]]

PR can not be merged due to conflict. Please rebase and resubmit

[mdkinney]

@Javagedes do you want to rebase to reopen?

@lgao4 please review

[Javagedes]

@mdkinney rebased as requested.

[github-actions[bot]]

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[github-actions[bot]]

This pull request has been automatically been closed because it did not have any activity in 60 days and no follow up within 7 days after being marked stale. Thank you for your contributions.

[Javagedes]

Hi everyone. This fell off of my radar and ended up getting closed. Sorry about that. Hoping to have this re-evaluated and get this merged in. We've been using it for over a year with no issue. I will resolve any comments.

[lgao4]

I understand this change now. But, I am not sure whether it may impact the downstream platform. What issues were detected in your platform?

[Javagedes]

@lgao4

This does not affect any platform by default, but being as this is a warning message, it has the possibility to affect any platform that uses the -w / --warning-as-error command line argument for the build command.

The scenario that would cause this message to appear is the following:

1. A library class definition is placed in a dsc file (e.g. MyLibraryClass|Path/To/Implementation.inf)
2. The library class specified (MyLibraryClass in the above example) in the dec is not specified in the Defines section of the inf.

In the above example, the INF at Path/To/Implementation.inf does not specify LIBRARY_CLASS = MyLibraryClass.

---

While this change detects any library class mismatch as discussed above, in any scenario the main use-case that brought about this change on our side is that when doing a library class override for a specific component in a DSC. If one overrides a library class with an INF that does not actually implement the interface of said library class, you make it completely through the autogen portion of the build command and fail to compile during make execution with some missing symbol linker warnings, which does not accurately describe the underlying issue.

In the below example output log, I did an override of DebugLib with MdePkg/Library/BasePrintLib/BasePrintLib.inf

---

I think the goal in the distant future would be to make this a build error during autogen, however similar to your concerns, that would almost assuredly break people. By having this be a warning, it gives the opportunity for everyone to fix any mismatches. Even if it never makes it to being a hard error, it is still a beneficial warning.

[lgao4]

@lgao4

This does not affect any platform by default, but being as this is a warning message, it has the possibility to affect any platform that uses the -w / --warning-as-error command line argument for the build command.

The scenario that would cause this message to appear is the following:

1. A library class definition is placed in a dsc file (e.g. MyLibraryClass|Path/To/Implementation.inf)
2. The library class specified (MyLibraryClass in the above example) in the dec is not specified in the Defines section of the inf.

In the above example, the INF at Path/To/Implementation.inf does not specify LIBRARY_CLASS = MyLibraryClass.

While this change detects any library class mismatch as discussed above, in any scenario the main use-case that brought about this change on our side is that when doing a library class override for a specific component in a DSC. If one overrides a library class with an INF that does not actually implement the interface of said library class, you make it completely through the autogen portion of the build command and fail to compile during make execution with some missing symbol linker warnings, which does not accurately describe the underlying issue.

In the below example output log, I did an override of DebugLib with MdePkg/Library/BasePrintLib/BasePrintLib.inf

I think the goal in the **distant** future would be to make this a build error during autogen, however similar to your concerns, that would almost assuredly break people. By having this be a warning, it gives the opportunity for everyone to fix any mismatches. Even if it never makes it to being a hard error, it is still a beneficial warning.

I agree this change. Now, it just reports the warning message in the build process. So, there is no break for the existing platform.

[lgao4]

@YuweiChen1110 , have you any comment for this change? I would like to merge it.

[github-actions[bot]]

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[github-actions[bot]]

This pull request has been automatically been closed because it did not have any activity in 60 days and no follow up within 7 days after being marked stale. Thank you for your contributions.