EccCheck update to later tools. Change EccCheck plugin scope. Add EccCheck #770
Conversation
…h name other than origin/master
… readme file with instructions for regenerating for future reference
github-actions
bot
added
language:python
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/202311 #770 +/- ##
===============================================
Coverage 1.23% 1.23%
===============================================
Files 1302 1302
Lines 332084 332084
Branches 6683 6683
===============================================
Hits 4117 4117
Misses 327891 327891
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| # Run git command to determine repos default branch | ||
| # | ||
| result = StringIO() | ||
| params = "symbolic-ref refs/remotes/origin/HEAD --short" |
There was a problem hiding this comment.
I agree the current way of hardcoding to origin/master is bad for us, but I think this way will also cause issues if we are targeting the non-default branch on CI. Such as cherry-picking changes back or if you have a PR targeting a feature branch.
There was a problem hiding this comment.
I agree that this tool is not optimal. Thankfully, this is not being enabled as part of CI, its scope is just being changed to cibuild-edk2 to enable mu based projects to be able to run the edk2 tool locally.
Comparing against the default branch may seem problematic, but the way the tool works is it uses the lines changed between the default branch and the current branch and filters the ecc check log to only report the errors corresponding to the changed lines. This means that the tool will only ever check a subset of the entire files for errors, and that a lot of things which do not conform to the coding standards will be able to slip through repeatedly until someone changes something in those lines.
As for cherry-picking changes, our most common cherry-pick source is from upstream, which runs this tool as part of their CI process. So cherry-picking most change from upstream should, for the most part, already have passed this check.
There was a problem hiding this comment.
I think Joey's saying that we often work on non-default branches. For example, release/202311 (default) and release/202302. In that case, cherry-picking into release/202302 will have the files compared to their equivalents in release/202311 which might be in a different base state which would result in an invalid delta.
The patchset only diff is okay I think but it needs to be correct.
|
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. |
Description
EccCheck is a tool that verifies additional Efi Code Compatibility for files that have been modified.
EccCheck relies on the basetool/Source/Python/Ecc.
Ecc contained files that were generated for parsing C files, but these were tied antlr4-runtime 4.7.1.
Documented instructions for generating new Ecc parser files/
For each item, place an "x" in between
[and]if true. Example:[x].(you can also check items in the GitHub UI)
flow, or firmware?
validation improvement, ...
in build or boot behavior?
a function in a new library class in a pre-existing module, ...
outside direct code modifications (and comments)?
on an a separate Web page, ...
How This Was Tested
Ran tests locally on repo after adding scope
cibuild-edk2Modified different files within packages to see if CI error would be tripped.
Integration Instructions
If wanting to use the EccCheck plugin, add
cibuild-edk2to the scope of the build files.