ciq-cherry-pick.sh: Some improvements

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 committed 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=os.getcwd(), 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(os.getcwd(), 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)
@@ -25,6 +46,8 @@
     )
     args = parser.parse_args()
 
+    logging.basicConfig(level=logging.INFO)
+
     # Expand the provided SHA1 to the full SHA1 in case it's either abbreviated or an expression
     git_sha_res = subprocess.run(["git", "show", "--pretty=%H", "-s", args.sha], stdout=subprocess.PIPE)
     if git_sha_res.returncode != 0:
@@ -39,6 +62,12 @@
     if args.ciq_tag is not None:
         tags = args.ciq_tag.split(",")
 
+    try:
+        check_fixes(args.sha)
+    except Exception as e:
+        print(e)
+        exit(1)
+
     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,88 @@ 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 if hash_ is reachable from pr_ref
+    """
+
+    try:
+        CIQ_run_git(repo, ["merge-base", "--is-ancestor", hash_, pr_ref])
+        return True
+    except RuntimeError:
+        return False
+
+
+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 repo_init(repo):
     """Initialize a git repo object.