ciq-cherry-pick.py: Cherry pick commit only if the Fixes: references are commited

If the commit that needs to be cherry picked has "Fixes:" references in the
commit body, there is now a check in pace that verify if those commits
are present in the current branch. At the moment, the scrips returns an
Exception because the developer must check why the commit has to be cherry
picked for a bug fix or cve fix if the actual commit that introduced
the bug/cve was not commited.

If the commit does not reference any Fixes:, an warning is shown to make
the developer aware that they have to double check if it makes sense to
cherry pick this commit. The script continues as this can be reviewed after.
This is common in the linux kernel community. Not all fixes have a Fixes:
reference.

Checking if a commit is part of the branch has now improved. It checks if
either the commit was backported by our team, or if the commit came from upstream.

Note: The implementation reuses some of the logic in the
check_kernel_commits.py. Those have been moved to ciq_helper.py. This commit
address the small refactor in check_kernel_commits.py as well.

Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>

check_kernel_commits.py: Use common implementation from ciq_helper.py

Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>

Author: roxanan1996

check_kernel_commits.py

@@ -8,38 +8,33 @@
 import textwrap
 from typing import Optional
 
-
-def run_git(repo, args):
-    """Run a git command in the given repository and return its output as a string."""
-    result = subprocess.run(["git", "-C", repo] + args, text=True, capture_output=True, check=False)
-    if result.returncode != 0:
-        raise RuntimeError(f"Git command failed: {' '.join(args)}\n{result.stderr}")
-    return result.stdout
+from ciq_helpers import (
+    CIQ_commit_exists_in_branch,
+    CIQ_extract_fixes_references_from_commit_body_lines,
+    CIQ_get_commit_body,
+    CIQ_hash_exists_in_ref,
+    CIQ_run_git,
+)
 
 
 def ref_exists(repo, ref):
     """Return True if the given ref exists in the repository, False otherwise."""
     try:
-        run_git(repo, ["rev-parse", "--verify", "--quiet", ref])
+        CIQ_run_git(repo, ["rev-parse", "--verify", "--quiet", ref])
         return True
     except RuntimeError:
         return False
 
 
 def get_pr_commits(repo, pr_branch, base_branch):
     """Get a list of commit SHAs that are in the PR branch but not in the base branch."""
-    output = run_git(repo, ["rev-list", f"{base_branch}..{pr_branch}"])
+    output = CIQ_run_git(repo, ["rev-list", f"{base_branch}..{pr_branch}"])
     return output.strip().splitlines()
 
 
-def get_commit_message(repo, sha):
-    """Get the commit message for a given commit SHA."""
-    return run_git(repo, ["log", "-n", "1", "--format=%B", sha])
-
-
 def get_short_hash_and_subject(repo, sha):
     """Get the abbreviated commit hash and subject for a given commit SHA."""
-    output = run_git(repo, ["log", "-n", "1", "--format=%h%x00%s", sha]).strip()
+    output = CIQ_run_git(repo, ["log", "-n", "1", "--format=%h%x00%s", sha]).strip()
     short_hash, subject = output.split("\x00", 1)
     return short_hash, subject
 
@@ -48,61 +43,56 @@ def hash_exists_in_mainline(repo, upstream_ref, hash_):
     """
     Return True if hash_ is reachable from upstream_ref (i.e., is an ancestor of it).
     """
-    try:
-        run_git(repo, ["merge-base", "--is-ancestor", hash_, upstream_ref])
-        return True
-    except RuntimeError:
-        return False
+
+    return CIQ_hash_exists_in_ref(repo, upstream_ref, hash_)
 
 
 def find_fixes_in_mainline(repo, pr_branch, upstream_ref, hash_):
     """
-    Return unique commits in upstream_ref that have Fixes: <N chars of hash_> in their message, case-insensitive.
+    Return unique commits in upstream_ref that have Fixes: <N chars of hash_> in their message, case-insensitive,
+    if they have not been commited in the pr_branch.
     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)
     """
     results = []
+
+    # Prepare hash prefixes from 12 down to 6
+    hash_prefixes = [hash_[:index] for index in range(12, 5, -1)]
+
     # Get all commits with 'Fixes:' in the message
-    output = run_git(repo, ["log", upstream_ref, "--grep", "Fixes:", "-i", "--format=%H %h %s (%an)%x0a%B%x00"]).strip()
+    output = CIQ_run_git(
+        repo,
+        [
+            "log",
+            upstream_ref,
+            "--grep",
+            "Fixes:",
+            "-i",
+            "--format=%H %h %s (%an)%x0a%B%x00",
+        ],
+    ).strip()
     if not output:
         return []
+
     # Each commit is separated by a NUL character and a newline
     commits = output.split("\x00\x0a")
-    # Prepare hash prefixes from 12 down to 6
-    hash_prefixes = [hash_[:index] for index in range(12, 5, -1)]
     for commit in commits:
         if not commit.strip():
             continue
-        # The first line is the summary, the rest is the body
+
         lines = commit.splitlines()
-        if not lines:
-            continue
+        # The first line is the summary, the rest is the body
         header = lines[0]
-        full_hash = header.split()[0]
-        # Search for Fixes: lines in the commit message
-        for line in lines[1:]:
-            m = re.match(r"^\s*Fixes:\s*([0-9a-fA-F]{6,40})", line, re.IGNORECASE)
-            if m:
-                for prefix in hash_prefixes:
-                    if m.group(1).lower().startswith(prefix.lower()):
-                        if not commit_exists_in_branch(repo, pr_branch, full_hash):
-                            results.append((full_hash, " ".join(header.split()[1:])))
-                        break
-            else:
-                continue
-    return results
+        full_hash, display_string = (lambda h: (h[0], " ".join(h[1:])))(header.split())
+        fixes = CIQ_extract_fixes_references_from_commit_body_lines(lines=lines[1:])
+        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))
+                    break
 
-
-def commit_exists_in_branch(repo, pr_branch, upstream_hash_):
-    """
-    Return True if upstream_hash_ has been backported and it exists in the
-    pr branch
-    """
-    output = run_git(repo, ["log", pr_branch, "--grep", "commit " + upstream_hash_])
-    if not output:
-        return False
-
-    return True
+    return results
 
 
 def wrap_paragraph(text, width=80, initial_indent="", subsequent_indent=""):
@@ -176,7 +166,7 @@ def main():
         if os.path.exists(vulns_repo):
             # Repository exists, update it with git pull
             try:
-                run_git(vulns_repo, ["pull"])
+                CIQ_run_git(vulns_repo, ["pull"])
             except RuntimeError as e:
                 print(f"WARNING: Failed to update vulns repo: {e}")
                 print("Continuing with existing repository...")
@@ -222,7 +212,7 @@ def main():
     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})"
-        msg = get_commit_message(args.repo, sha)
+        msg = CIQ_get_commit_body(args.repo, sha)
         upstream_hashes = re.findall(r"^commit\s+([0-9a-fA-F]{40})", msg, re.MULTILINE)
         for uhash in upstream_hashes:
             short_uhash = uhash[:12]

ciq-cherry-pick.py

@@ -1,15 +1,36 @@
 import argparse
+import logging
 import os
 import subprocess
 
 import git
 
-from ciq_helpers import CIQ_cherry_pick_commit_standardization, CIQ_original_commit_author_to_tag_string
-
-# from ciq_helpers import *
+from ciq_helpers import (
+    CIQ_cherry_pick_commit_standardization,
+    CIQ_commit_exists_in_current_branch,
+    CIQ_fixes_references,
+    CIQ_original_commit_author_to_tag_string,
+)
 
 MERGE_MSG = git.Repo(os.getcwd()).git_dir + "/MERGE_MSG"
 
+
+def check_fixes(sha):
+    """
+    Checks if commit has "Fixes:" references and if so, it checks if the
+    commit(s) that it tries to fix are part of the current branch
+    """
+
+    fixes = CIQ_fixes_references(repo_path=".", sha=sha)
+    if len(fixes) == 0:
+        logging.warning("The commit you try to cherry pick has no Fixes: reference; review it carefully")
+        return
+
+    for fix in fixes:
+        if not CIQ_commit_exists_in_current_branch(".", fix):
+            raise RuntimeError(f"The commit you want to cherry pick references a Fixes {fix}: but this is not here")
+
+
 if __name__ == "__main__":
     print("CIQ custom cherry picker")
     parser = argparse.ArgumentParser(formatter_class=argparse.RawTextHelpFormatter)
@@ -39,6 +60,8 @@
     if args.ciq_tag is not None:
         tags = args.ciq_tag.split(",")
 
+    check_fixes(args.sha)
+
     author = CIQ_original_commit_author_to_tag_string(repo_path=os.getcwd(), sha=args.sha)
     if author is None:
         exit(1)

ciq_helpers.py

@@ -169,6 +169,143 @@ def CIQ_original_commit_author_to_tag_string(repo_path, sha):
     return "commit-author " + git_auth_res.stdout.decode("utf-8").replace('"', "").strip()
 
 
+def CIQ_run_git(repo_path, args):
+    """
+    Run a git command in the given repository and return its output as a string.
+    """
+    result = subprocess.run(["git", "-C", repo_path] + args, text=True, capture_output=True, check=False)
+    if result.returncode != 0:
+        raise RuntimeError(f"Git command failed: {' '.join(args)}\n{result.stderr}")
+
+    return result.stdout
+
+
+def CIQ_get_commit_body(repo_path, sha):
+    return CIQ_run_git(repo_path, ["show", "-s", sha, "--format=%B"])
+
+
+def CIQ_extract_fixes_references_from_commit_body_lines(lines):
+    fixes = []
+    for line in lines:
+        m = re.match(r"^\s*Fixes:\s*([0-9a-fA-F]{6,40})", line, re.IGNORECASE)
+        if not m:
+            continue
+
+        fixes.append(m.group(1))
+
+    return fixes
+
+
+def CIQ_fixes_references(repo_path, sha):
+    """
+    If commit message of sha contains lines like
+    Fixes: <short_fixed>, this returns a list of <short_fixed>, otherwise an empty list
+    """
+
+    commit_body = CIQ_get_commit_body(repo_path, sha)
+    return CIQ_extract_fixes_references_from_commit_body_lines(lines=commit_body.splitlines())
+
+
+def CIQ_get_full_hash(repo, short_hash):
+    return CIQ_run_git(repo, ["show", "-s", "--pretty=%H", short_hash]).strip()
+
+
+def CIQ_get_current_branch(repo):
+    return CIQ_run_git(repo, ["branch", "--show-current"]).strip()
+
+
+def CIQ_hash_exists_in_ref(repo, pr_ref, hash_):
+    """
+    Return True is hash_ is reachable from pr_branch
+    """
+
+    try:
+        CIQ_run_git(repo, ["merge-base", "--is-ancestor", hash_, pr_ref])
+        return True
+    except RuntimeError:
+        return False
+
+
+# TODO think of a better name
+def CIQ_commit_exists_in_branch(repo, pr_branch, upstream_hash_):
+    """
+    Return True if upstream_hash_ has been backported and it exists in the pr branch
+    """
+
+    # First check if the commit has been backported by CIQ
+    output = CIQ_run_git(repo, ["log", pr_branch, "--grep", "commit " + upstream_hash_])
+    if output:
+        return True
+
+    # 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_)
+
+
+def CIQ_commit_exists_in_current_branch(repo, upstream_hash_):
+    """
+    Return True if upstream_hash_ has been backported and it exists in the current branch
+    """
+
+    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)
+
+
+def CIQ_find_fixes_in_mainline(repo, pr_branch, upstream_ref, hash_):
+    """
+    Return unique commits in upstream_ref that have Fixes: <N chars of hash_> in their message, case-insensitive,
+    if they have not been commited in the pr_branch.
+    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)
+    """
+    results = []
+
+    # Prepare hash prefixes from 12 down to 6
+    hash_prefixes = [hash_[:index] for index in range(12, 5, -1)]
+
+    # Get all commits with 'Fixes:' in the message
+    output = CIQ_run_git(
+        repo,
+        [
+            "log",
+            upstream_ref,
+            "--grep",
+            "Fixes:",
+            "-i",
+            "--format=%H %h %s (%an)%x0a%B%x00",
+        ],
+    ).strip()
+    if not output:
+        return []
+
+    # Each commit is separated by a NUL character and a newline
+    commits = output.split("\x00\x0a")
+    for commit in commits:
+        if not commit.strip():
+            continue
+
+        lines = commit.splitlines()
+        # The first line is the summary, the rest is the body
+        header = lines[0]
+        full_hash, display_string = (lambda h: (h[0], " ".join(h[1:])))(header.split())
+        fixes = CIQ_extract_fixes_references_from_commit_body_lines(lines=lines[1:])
+        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))
+                    break
+
+    return results
+
+
+def CIQ_find_fixes_in_mainline_current_branch(repo, upstream_ref, hash_):
+    current_branch = CIQ_get_current_branch(repo)
+
+    return CIQ_find_fixes_in_mainline(repo, current_branch, upstream_ref, hash_)
+
+
 def repo_init(repo):
     """Initialize a git repo object.