edk2
edk2 copied to clipboard
tianocore
BaseTools: fix spec keyword parsing
Fixes SPEC keyword parsing to match EDK2 specification
Description
When SPEC keyword is used in the INF file, it does not create the equivalent #define in the autogen.h file of the module. Additionally it creates a line in autogenerated makefile. This generated line will cause builds to break of we follow the specification as defined in the example here, [https://github.com/tianocore-docs/edk2-InfSpecification/blob/master/3_edk_ii_inf_file_format/34_%5Bdefines%5D_section.md] Ex : SPEC USB_SPECIFICATION_VERSION = 2.0 The changes fixes the autogen code to function as mentioned by the documentation.
- [ ] Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- N/A
- [ ] Impacts security?
- Security - Does this PR have a direct security impact?
- N/A
- [ ] Includes tests?
- Tests - Does this PR include any explicit test code?
- N/A
How This Was Tested
Tested locally with builds.
Integration Instructions
N/A
@bcran @lgao4 Reaching out through a comment, Not able to add anyone as reviewers.
This change looks good, but I'd prefer to have a review from @lgao4 before merging it.
SPEC
Can you give one test case?
Test case in the code like in the comments? Or in an INF file? Is there a dummy file that I can drop it in for you? ie: The current fix is to match what the EDK2 documentations says it should be,
INF files, Defines Section, Add this SPEC USB_SPECIFICATION_VERSION = 2.0
And you should see it in the autogen.h file, #DEFINE USB_SPECIFICATION_VERSION 2.0, where it is a double type.
Ideally, I would like for it to be more modern be free form (instead of double, make it a string value so I can use multi decimal input). That would require me to change edk2 specification document so thought to try to fix it first before I proposed that. ex : SPEC USB_SPECIFICATION_VERSION = 2.1.2 autogen.h will have "#DEFINE USB_SPECIFICATION_VERSION "2.1.2"
SPEC
Can you give one test case?
Test case in the code like in the comments? Or in an INF file? Is there a dummy file that I can drop it in for you? ie: The current fix is to match what the EDK2 documentations says it should be,
INF files, Defines Section, Add this SPEC USB_SPECIFICATION_VERSION = 2.0 And you should see it in the autogen.h file,
#DEFINE USB_SPECIFICATION_VERSION 2.0, where it is a double type.Ideally, I would like for it to be more modern be free form (instead of double, make it a string value so I can use multi decimal input). That would require me to change edk2 specification document so thought to try to fix it first before I proposed that. ex : SPEC USB_SPECIFICATION_VERSION = 2.1.2 autogen.h will have "#DEFINE USB_SPECIFICATION_VERSION "2.1.2"
Edk2 doesn't use double type. So, I agree to define them as ascii string. Please submit the spec change first.
SPEC
Can you give one test case?
Test case in the code like in the comments? Or in an INF file? Is there a dummy file that I can drop it in for you? ie: The current fix is to match what the EDK2 documentations says it should be, INF files, Defines Section, Add this SPEC USB_SPECIFICATION_VERSION = 2.0 And you should see it in the autogen.h file,
#DEFINE USB_SPECIFICATION_VERSION 2.0, where it is a double type. Ideally, I would like for it to be more modern be free form (instead of double, make it a string value so I can use multi decimal input). That would require me to change edk2 specification document so thought to try to fix it first before I proposed that. ex : SPEC USB_SPECIFICATION_VERSION = 2.1.2 autogen.h will have "#DEFINE USB_SPECIFICATION_VERSION "2.1.2"Edk2 doesn't use double type. So, I agree to define them as ascii string. Please submit the spec change first.
Please advise how/steps to follow to request the change to specification please? Do I make a PR against the document (*.MD) files? Do I raise it in the CI/Basetools meetings as a topic?
SPEC
Can you give one test case?
Test case in the code like in the comments? Or in an INF file? Is there a dummy file that I can drop it in for you? ie: The current fix is to match what the EDK2 documentations says it should be, INF files, Defines Section, Add this SPEC USB_SPECIFICATION_VERSION = 2.0 And you should see it in the autogen.h file,
#DEFINE USB_SPECIFICATION_VERSION 2.0, where it is a double type. Ideally, I would like for it to be more modern be free form (instead of double, make it a string value so I can use multi decimal input). That would require me to change edk2 specification document so thought to try to fix it first before I proposed that. ex : SPEC USB_SPECIFICATION_VERSION = 2.1.2 autogen.h will have "#DEFINE USB_SPECIFICATION_VERSION "2.1.2"Edk2 doesn't use double type. So, I agree to define them as ascii string. Please submit the spec change first.
Please advise how/steps to follow to request the change to specification please? Do I make a PR against the document (*.MD) files? Do I raise it in the CI/Basetools meetings as a topic?
For the change in spec, please send the patch to edk2 mail list [email protected]. I will review and merge it.
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] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Commenting here to keep this PR active. @Ashwin-Veeraiah Are you still working on this?
Commenting here to keep this PR active. @Ashwin-Veeraiah Are you still working on this?
@bexcran Yes. Other priorities in the way. Expect to work in during Thanksgiving. Thank you.
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.
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.
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.
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.
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.
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.