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

[Feature]: Verify library INF implements specified library class from the DSC #104

Open
Javagedes opened this issue Jul 18, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@Javagedes
Copy link
Contributor

Javagedes commented Jul 18, 2023

What does the feature solve?

This feature will resolve hard to detect linking errors due to libraries not being properly linked to components due to a quiet mismatch of library classes

Describe the solution

A check can take place inside of BaseTools/Source/Workspace/DscBuildData.py that, when parsing library class lines in a DSC's [LibraryClasses.*] section or library override of the [Components.*] section, will verify that the library instance actually implements the library by checking which LIBRARY_CLASS definitions exist in the INF [Defines] section.

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.

The following commit shows a possible implementation that has been verified to catch invalid linking in the two scenarios above:
tianocore/edk2@master...Javagedes:edk2:verify-library-class

Attached is a diff:
diff.txt

@mdkinney
@makubacki
@YuweiChen1110

Have you considered any alternatives?

No response

Additional context

No response

@Javagedes Javagedes added the enhancement New feature or request label Jul 18, 2023
@makubacki
Copy link
Member

@YuweiChen1110, can you please advise on next steps?

@YuweiChen1110
Copy link
Contributor

It is valuable to add this feature. The feature's quality is under testing.
After testing, will reply with the result. And then we can submit the formal PR in edk2-basetools repo and send out the patch with email for edk2 repo.

@Javagedes
Copy link
Contributor Author

Javagedes commented Jul 21, 2023

Hi @YuweiChen1110, I found an issue in the diff I previously sent you. In larger builds the same DSC is parsed multiple times, thus each mismatch is reported multiple times also, which is not preferred. I spent some time getting a fix to only report each mismatch once. Below is the diff. Here is also a link to the two commits: microsoft/mu_basecore@release/202302...Javagedes:mu_basecore:library-check

diff.txt

@YuweiChen1110
Copy link
Contributor

Thanks for the contribution and clarify!

@YuweiChen1110
Copy link
Contributor

When I tested this feature in my local build env, I encountered some errors and currently finding the root cause.
Will update this issue with the error info after collection.

Thanks for your patience.

@Javagedes
Copy link
Contributor Author

What issue are you seeing? I can debug tomorrow

@YuweiChen1110
Copy link
Contributor

Hi, the situation I encountered is shown below. It seems like not this feature's issue but with adding this feature a previously existing but undiscovered bug is exposed.

The situation is: a PCD in INF file is not defined in DEC file.
The expected behavior of build system should be: raise the error "PCD [%s.%s] in [%s] is not found in dependent packages:"

However in some strange case, when we do not add this feature, the error will not be raised, after we add this feature, the error is raised correctly. It is very strange, and I am checking on the root cause.

From my side, we need to fix the bug firstly then merge this feature. Or it will block build system current use.

Will update the PR with debugging status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants