scripts: ci: check_compliance: fix misleading error message when Signed-off-by check fails #96871
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When checking for emails/sign-off, if parsing of the commit information using 'git show' fails, the 'auth_name', 'auth_email' and 'body' variables are unbound but accessed right afterwards, which would cause an error. Skip straight to next iteration of the loop when such an error occurs to ensure access to the unbound variables is never performed. This error was reported by the VS Code Pylance analyzer. Signed-off-by: Mathieu Choplain <[email protected]>
Commit e4650bc modified check_compliance to avoid false negatives when a commit has multiple signoff lines and the author's signoff line isn't the first one. To avoid this, a signoff line which matches exactly the author's name and email is searched instead of matching any line with the proper syntax and checking the fields. While this solved the original issue, this also introduced a new issue: whereas the old script would distinguish a properly formed signoff line with the wrong author from a malformed signoff line, the new method now checks both the syntax and the author name/email at the same time. As such, the script will report "signoff line does not follow the syntax" for both situations, but the signoff line DOES follow the syntax in the latter case: the error message is then only a misleading red herring. Fix this issue - and improve the check - by verifying every signoff line in a commit's message body. Invalid syntax on any line is reported as a "does not follow syntax" failure, whereas inability to find the commit author is reported as a separate "no signoff entry matches commit author" failure as it should always have... Signed-off-by: Mathieu Choplain <[email protected]>
zephyrbot
requested review from
fabiobaltieri,
kartben,
keith-zephyr,
nashif,
pdgendt and
stephanosio
|
pdgendt
approved these changes
Oct 2, 2025
stephanosio
approved these changes
Oct 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit e4650bc modified
check_complianceto avoid false negatives when a commit has multiple signoff lines and the author's signoff line isn't the first one. To avoid this, a signoff line which matches exactly the author's name and email is searched instead of matching any line with the proper syntax and checking the fields.While this solved the original issue, this also introduced a new issue: the old script distinguished a properly formed signoff line with the wrong author from a malformed signoff line, but the new method perfoms both the syntax and the author name/email checks at the same time. As such, if any of these fail, the script will report
signoff line does not follow the syntaxeven though the signoff line DOES follow the syntax in the latter case. This makes the error message nothing but a misleading red herring, which is quite unfortunate considering this situation is much more likely to occur than the "malformed syntax" one.Example here: https://github.com/zephyrproject-rtos/zephyr/actions/runs/16752997174/job/47427937294?pr=93699#step:10:47
Fix this issue - and improve the check - by actually verifying every signoff line in a commit's message body. Invalid syntax on any line is reported as a
does not follow syntaxfailure, whereas inability to find the commit author is reported as a separateno signoff entry matches commit authorfailure as it should always have...+ tiny commit to fix a potential issue reported by VS Code's linter that would occur if
git showfailed (this has presumably never happened since the bug is still here 🙂)