diff --git a/check_kernel_commits.py b/check_kernel_commits.py index 5a16864..8aca3fd 100644 --- a/check_kernel_commits.py +++ b/check_kernel_commits.py @@ -7,6 +7,7 @@ import textwrap from kt.ktlib.ciq_helpers import ( + CIQ_filter_unapplied_commits, CIQ_find_fixes_in_mainline, CIQ_find_matching_cve, CIQ_get_commit_body, @@ -126,6 +127,9 @@ def main(): any_findings = False out_lines = [] + # len(pr_commits) >= 1 + oldest_pr_commit = pr_commits[-1] + newest_pr_commit = pr_commits[0] for sha in reversed(pr_commits): # oldest first short_hash, subject = get_short_hash_and_subject(args.repo, sha) pr_commit_desc = f"{short_hash} ({subject})" @@ -155,7 +159,68 @@ def main(): ) out_lines.append("") # blank line continue - fixes = CIQ_find_fixes_in_mainline(args.repo, args.pr_branch, upstream_ref, uhash) + + # Find commits that contains Fixes: in upstream_ref + fixes = CIQ_find_fixes_in_mainline(repo=args.repo, upstream_ref=upstream_ref, hash_=uhash) + + # Filter the above commits if they were unapplied in the pr_branch (whole fetched history is used) + fixes_unapplied_without_limit = CIQ_filter_unapplied_commits( + repo=args.repo, pr_branch=args.pr_branch, commits=fixes + ) + + # Filter the above commits if they were unapplied in the pr_branch (oldest_pr_commit..newest_pr_commit interval is used) + fixes_unapplied_with_limit = CIQ_filter_unapplied_commits( + repo=args.repo, pr_branch=args.pr_branch, commits=fixes, interval=(oldest_pr_commit, newest_pr_commit) + ) + + # Issue a warning if we found that a commit that contains Fixes: has been applied before + # Very specific case, but check this PR where it did happen https://github.com/ctrliq/kernel-src-tree/pull/1212#issuecomment-4421837799 + # It is up to the developer to assess the situation if this happens. In the example case, a commit X was pushed, then it was reverted by commit Y, + # then commit X was pushed again but automation locally did not flag that it was reverted before because its dependency Y appeared to have been applied + # in the local tree. + fixes_applied_before_commit = set(fixes_unapplied_with_limit) - set(fixes_unapplied_without_limit) + if fixes_applied_before_commit: + # Build the fixes display text + fixes_lines = [] + for fix_hash, display_str in fixes_applied_before_commit: + fixes_lines.append(display_str) + + fixes_text = "\n".join(fixes_lines) + any_findings = True + if args.markdown: + fixes_block = " " + fixes_text.replace("\n", "\n ") + out_lines.append( + f"- ❗ PR commit `{pr_commit_desc}` references upstream commit \n" + f" `{short_uhash}` which has been referenced by a `Fixes:` tag in the upstream \n" + f" Linux kernel:\n\n" + f"```text\n{fixes_block}\n```\n" + " was applied after its deps, not before\n" + ) + else: + prefix = "[FIXES-WRONG-ORDER] " + header = ( + f"{prefix}PR commit {pr_commit_desc} references upstream commit " + f"{short_uhash}, which has Fixes tags:" + ) + out_lines.append( + wrap_paragraph( + header, width=80, initial_indent="", subsequent_indent=" " * len(prefix) + ) # spaces for '[FIXES-WRONG-ORDER] ' + ) + out_lines.append("") # blank line after 'Fixes tags:' + for line in fixes_text.splitlines(): + out_lines.append(" " + line) + + out_lines.append( + wrap_paragraph( + text="was applied after its deps, not before\n", + initial_indent=" " * len(prefix), + ) + ) + out_lines.append("") # blank line + + # We continue the usual checks for fixes that were unapplied after the commit of interest + fixes = fixes_unapplied_with_limit if fixes: any_findings = True diff --git a/ciq-cherry-pick.py b/ciq-cherry-pick.py index db670e6..3a7194b 100644 --- a/ciq-cherry-pick.py +++ b/ciq-cherry-pick.py @@ -12,7 +12,7 @@ from kt.ktlib.ciq_helpers import ( CIQ_cherry_pick_commit_standardization, CIQ_commit_exists_in_current_branch, - CIQ_find_fixes_in_mainline_current_branch, + CIQ_find_fixes_in_mainline_current_branch_unapplied, CIQ_find_matching_cve, CIQ_fixes_references, CIQ_get_full_hash, @@ -251,7 +251,9 @@ def cherry_pick_fixes( Fixes: . If any, these will also be cherry picked with the ciq tag = cve-bf. If the tag was cve-pre, it stays the same. """ - fixes_in_mainline = CIQ_find_fixes_in_mainline_current_branch(os.getcwd(), upstream_ref, sha) + fixes_in_mainline = CIQ_find_fixes_in_mainline_current_branch_unapplied( + repo=os.getcwd(), upstream_ref=upstream_ref, hash_=sha + ) # Replace cve with cve-bf # Leave cve-pre and cve-bf as they are diff --git a/kt/ktlib/ciq_helpers.py b/kt/ktlib/ciq_helpers.py index 8791770..7cb942d 100644 --- a/kt/ktlib/ciq_helpers.py +++ b/kt/ktlib/ciq_helpers.py @@ -229,21 +229,33 @@ def CIQ_hash_exists_in_ref(repo, pr_ref, hash_): return False -def CIQ_commit_exists_in_branch(repo, pr_branch, upstream_hash_): +def CIQ_commit_exists_in_branch(repo, pr_branch, upstream_hash_, interval=None): """ - Return True if upstream_hash_ has been backported and it exists in the pr branch + Return True if upstream_hash_ has been backported and it exists in the pr branch. + If interval that consists of a pair of commit hashes is not None, we only + check the interval[0]..interval[1], otherwise the whole history of the pr_branch will be used. """ # First check if the commit has been backported by CIQ - output = CIQ_run_git(repo, ["log", pr_branch, "--grep", "^commit " + upstream_hash_]) + git_command = ["log", pr_branch, "--grep", "^commit " + upstream_hash_] + if interval: + oldest_commit, newest_commit = interval + git_command.append(f"{oldest_commit}..{newest_commit}") + + output = CIQ_run_git(repo_path=repo, args=git_command) if output: return True + # If we are searching in the interval .., + # we do not need to check way back if the commit came from upstream as it is + if interval: + return False + # If it was not backported by CIQ, maybe it came from upstream as it is - return CIQ_hash_exists_in_ref(repo, pr_branch, upstream_hash_) + return CIQ_hash_exists_in_ref(repo=repo, pr_ref=pr_branch, hash_=upstream_hash_) -def CIQ_commit_exists_in_current_branch(repo, upstream_hash_): +def CIQ_commit_exists_in_current_branch(repo, upstream_hash_, interval=None): """ Return True if upstream_hash_ has been backported and it exists in the current branch """ @@ -251,13 +263,14 @@ def CIQ_commit_exists_in_current_branch(repo, upstream_hash_): current_branch = CIQ_get_current_branch(repo) full_upstream_hash = CIQ_get_full_hash(repo, upstream_hash_) - return CIQ_commit_exists_in_branch(repo, current_branch, full_upstream_hash) + return CIQ_commit_exists_in_branch( + repo=repo, pr_branch=current_branch, upstream_hash_=full_upstream_hash, interval=interval + ) -def CIQ_find_fixes_in_mainline(repo, pr_branch, upstream_ref, hash_): +def CIQ_find_fixes_in_mainline(repo, upstream_ref, hash_): """ - Return unique commits in upstream_ref that have Fixes: in their message, case-insensitive, - if they have not been committed in the pr_branch. + Return unique commits in upstream_ref that have Fixes: in their message, case-insensitive. Start from 12 chars and work down to 6, but do not include duplicates if already found at a longer length. Returns a list of tuples: (full_hash, display_string) """ @@ -295,17 +308,51 @@ def CIQ_find_fixes_in_mainline(repo, pr_branch, upstream_ref, hash_): for fix in fixes: for prefix in hash_prefixes: if fix.lower().startswith(prefix.lower()): - if not CIQ_commit_exists_in_branch(repo, pr_branch, full_hash): - results.append((full_hash, display_string)) + results.append((full_hash, display_string)) break return results -def CIQ_find_fixes_in_mainline_current_branch(repo, upstream_ref, hash_): +def CIQ_filter_unapplied_commits(repo, pr_branch, commits, interval=None): + """ + Receives a list of tuples: (full_hash, display_string) + Returns filtered list with commits that were not applied in the pr_branch. + If interval that consists of a pair of commit hashes is not None, we only + check the interval[0]..interval[1], otherwise the whole history of the pr_branch will be used. + + Returns a list of tuples: (full_hash, display_string) + """ + + results = [] + for commit in commits: + full_hash = commit[0] + if not CIQ_commit_exists_in_branch(repo=repo, pr_branch=pr_branch, upstream_hash_=full_hash, interval=interval): + results.append(commit) + + return results + + +def CIQ_find_fixes_in_mainline_unapplied(repo, pr_branch, upstream_ref, hash_, interval=None): + """ + Return unique commits in upstream_ref that have Fixes: in their message, case-insensitive, + if they have not been committed in the pr_branch. + If interval that consists of a pair of commit hashes is not None, we only + check the interval[0]..interval[1], otherwise the whole history of the pr_branch will be used. + Start from 12 chars and work down to 6, but do not include duplicates if already found at a longer length. + Returns a list of tuples: (full_hash, display_string) + """ + + fixes = CIQ_find_fixes_in_mainline(repo=repo, upstream_ref=upstream_ref, hash_=hash_) + return CIQ_filter_unapplied_commits(repo=repo, pr_branch=pr_branch, commits=fixes, interval=interval) + + +def CIQ_find_fixes_in_mainline_current_branch_unapplied(repo, upstream_ref, hash_, interval=None): current_branch = CIQ_get_current_branch(repo) - return CIQ_find_fixes_in_mainline(repo, current_branch, upstream_ref, hash_) + return CIQ_find_fixes_in_mainline_unapplied( + repo=repo, pr_branch=current_branch, upstream_ref=upstream_ref, hash_=hash_, interval=interval + ) def CIQ_reset_HEAD(repo):